dt.columns(':visible').widths().toArray() always returns array of zeros

sebastianbarth · September 24

Hi Allan,

I found a weird situation with the fixedColumns() plugin failing to set correct style.left pixel values (they are all 0).
I couldn't reproduce this in an isolated online example to pass to you.

What I managed to do is to boil it down to a problem with the following snipped in the fixed columns plugin:

    _addStyles() {
        let dt = this.s.dt;
        let that = this;
        let colCount = this.s.dt.columns(':visible').count();
        let headerStruct = dt.table().header.structure(':visible');
        let footerStruct = dt.table().footer.structure(':visible');
        let widths = dt.columns(':visible').widths().toArray();
        ....
     }

There the following call returns an array of 0 values:

let widths = dt.columns(':visible').widths().toArray(); // Returns [0, 0, 0, 0, 0, 0, 0]

This internally calls the following function:

registerPlural('columns().widths()', 'column().width()', function () {
    // Injects a fake row into the table for just a moment so the widths can
    // be read, regardless of colspan in the header and rows being present
    // in the body
    var columns = this.columns(':visible');
    var row = Dom.c('tr').html('<td>' + Array(columns.count()).join('</td><td>') + '</td>');
    Dom.s(this.table().body()).append(row);
    var widths = [];
    var indexes = columns.indexes();
    row.children().each((el, idx) => {
        widths[indexes[idx]] = Dom.s(el).width('outer');
    });
    row.remove();
    return this.iterator('column', (settings, column) => {
        return widths[column] || 0;
    }, true);
});

There the following line produces an <tr></tr> element with now <td> elements, although columns.count() returns 8:

This of course leads to [0, 0, 0, 0, 0, 0, 0].

Any idea what that could be?

I am also a bit confused that we need to filter to :visible in _addStyles() and in the columns().widths() function as well. Is that correct/redundant?

I also noticed that we already have rows. Since you do not allow to have cells with colspan > 1, we could fallback to using existing cells, right? I am looking toward performance optimization via avoiding one forced layout. What do you think?

Thank you for looking into this. I am looking forward to your answer!

Greetings
Sebastian

sebastianbarth · September 24

The code is using:

  "datatables.net-dt": "3.1.1"
  "datatables.net-fixedcolumns": "6.1.0"
  "datatables.net-scroller-dt": "3.1.0"

allan · September 24

Is the element the table is in hidden at that point? That's the only thing off the top of my head...

Allan

sebastianbarth · September 25

Hi Allan,

we traced the problem down to the interaction between Dom.html() and our default Trusted Types policy, which uses DOMPurify to sanitize HTML strings.

In columns().widths(), DataTables creates a temporary row with:

var row = Dom.c('tr').html(
    '<td>' + Array(columns.count()).join('</td><td>') + '</td>'
);

Dom.html() assigns the string to the element’s innerHTML property. For three columns, this is effectively:

tr.innerHTML = '<td></td><td></td><td></td>';

That assignment works normally because the browser parses the fragment in the context of a <tr>. With our Trusted Types configuration, however, the string first passes through our default policy and DOMPurify.

DOMPurify parses the string as an HTML document without the destination element’s context. The HTML parser discards these standalone <td> tags when DOMPurify adds them to a temporarily created <body></body> element , so sanitization returns an empty string. Consequently, the temporary row has no cells to measure.

Would you consider creating these structural elements through DOM APIs instead? For example:

var row = Dom.c('tr');

for (var i = 0, count = columns.count(); i < count; i++) {
    Dom.c('td').appendTo(row);
}

This avoids the HTML parsing sink entirely for this operation.

More generally, using textContent where content is intended to be plain text, and DOM creation methods for fixed structural markup, would improve compatibility with Trusted Types enforcement: I noticed quite some usages of .html(v) where .text(v) would suffice. Places that intentionally accept bigger HTML (e.g. performance reasons) would still need appropriate handling, of course.

A dedicated Trusted Types policy might also be an option for strictly controlled, library-generated markup, provided the application’s CSP permits it. It should not automatically trust arbitrary strings or application-supplied content, though. Otherwise, this would introduce a risk.

Other applications using a DOMPurify-backed default policy could encounter the same issue. DOMPurify as sanitizer is the quasi the industry standard.

What is your opinion on this?

allan · September 25

Thank you for tracking that down. Very happy to make that change - done here.

Overall, it is good practice to use just DOM methods rather than writing into innerHTML. That needs to be balanced with backwards compatibility and cases where there is already XSS protection (such as using the built in renderers in DataTables). The more I can move to just DOM methods, the better, without breaking things .

Allan

dt.columns(':visible').widths().toArray() always returns array of zeros

dt.columns(':visible').widths().toArray() always returns array of zeros

Answers