1784040 - [Sanitizer] Implement namespace handling

Status: RESOLVED FIXED

(Core :: DOM: Security, task, P3)

Comment 1

[Tom Schuster (MoCo)]

We are still trying to find an appropriate (WebIDL) syntax for elements and attributes with namespaces. I came up with the following proposal that I want to evaluate by creating a draft implementation. I am hopeful that adapting to whatever syntax we pick is going to be relatively easy.

// Either ["div", "span"] or { "http://www.w3.org/2000/svg" : ["text"] }
typedef (sequence<DOMString> or record<DOMString, sequence<DOMString>>) ElementList;

// {
//   "": {
//     "id": "*" or ["div", "span"] or { "http://www.w3.org/2000/svg" : ["text"] }
//   },
//   "http://www.w3.org/2000/svg": {
//      "path": "*"
//   }
// }
enum Star {
  "*"
};
typedef record<DOMString, record<DOMString, (Star or ElementList)>> AttributeList;

Comment 2

[Tom Schuster (MoCo)]

Attachment: Bug 1784040 - Error handling during construction of the sanitizer. r?emilio

Comment 3

[Tom Schuster (MoCo)]

Attachment: Bug 1784040 - Support experimental ElementList and AttributeList with namespace. r?emilio

Depends on D159146

Comment 4

[Tom Schuster (MoCo)]

As I said this is experimental and it now seems quite likely that we aren't going to adopt this specific "syntax". So I will have to update it before landing after that decision is reached.

Comment 5

[Tom Schuster (MoCo)]

Hey peter! Would you mind taking a look at the WebIDL changes? I came up with most of this myself, and I am far from an expert in designing IDL for web APIs.

• I would like to call one of my properties "namespace", but that causes parser errors.
• I had to add [GenerateConversionToJS] to prevent build failures in the bindings.

In general if you have some feedback on this that would be very valuable. (Also in https://github.com/WICG/sanitizer-api/issues/181)

Comment 6

[Peter Van der Beken [:peterv]]

(In reply to Tom Schuster (MoCo) from comment #5)

Hey peter! Would you mind taking a look at the WebIDL changes? I came up with most of this myself, and I am far from an expert in designing IDL for web APIs.

I'll take a look.

• I would like to call one of my properties "namespace", but that causes parser errors.

You should be able to use _namespace (see the second note in https://webidl.spec.whatwg.org/#idl-names).

Comment 7

[Tom Schuster (MoCo)]

Thank you Peter!

Comment 8

[Tom Schuster (MoCo)]

Attachment: Bug 1784040 - Update Sanitizer tests for namespace handling. r?freddyb

Depends on D159147

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/26becb127787
Error handling during construction of the sanitizer. r=emilio
https://hg.mozilla.org/integration/autoland/rev/52b822cb8a4c
Support experimental ElementList and AttributeList with namespace. r=emilio
https://hg.mozilla.org/integration/autoland/rev/37174bb01720
Update Sanitizer tests for namespace handling. r=freddyb

Comment 10

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/37996 for changes under testing/web-platform/tests

Comment 11

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/26becb127787
https://hg.mozilla.org/mozilla-central/rev/52b822cb8a4c
https://hg.mozilla.org/mozilla-central/rev/37174bb01720

Comment 12

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 13

[Hamish Willee]

MDN docs work for this can be tracked in https://github.com/mdn/content/issues/24400

My understanding is that previously you could only specify HTML elements and attributes as strings in the respective lists passed to Sanitizer().
With this update you can still pass strings, in which case the namespace is assumed to be HTML. However you can alternatively specify an object that includes the name of the element/attribute and a namespace string.

How stable is all this?

• The spec doesn't mention any of this https://wicg.github.io/sanitizer-api/#sanitizer-api
• FF is still behind a pref for sanitizer API but this new functionality does not appear to be gated by another pref
• Chrome indicates full support for existing API, but no obvious evidence that they are adopting this?

Is it is too early to document this or not? IF it is needed then I'd update browser compatibility with a new subfeature namespace_support (or similar), but I'd really like a spec link for that ...

Comment 14

[Frederik Braun [:freddy]]

It's still to early to document this. Chrome's implementation of the Sanitizer API is non-standard and incomplete.
The standard work is ongoing and we're facing some gnarly issues on various fronts (defaults, syntax, security guarantees).

I'm surprised that Chrome indicates full support, to be honest. Their development mailing list (blink-dev) called this an MVP. I'd be really disappointed if people used and relied on that implementation.

Comment 15

[Tom Schuster (MoCo)]

As Freddy said, the spec is still unstable and some of the changes we are going to have to make aren't fully decided on and thus also no in the spec. It would be really nice if we could suggest that users hold off on using this in production. Especially because the implementation in Chrome is non-standard and incompatible with that we are going to specify. Chrome is going to ship the standards changes, but solving the incompatibility problems with their existing implementation is up to them to figure out.

Comment 16

[Hamish Willee]

Thanks very much. I will put off documenting this for now. It would be really helpful if you could give the MDN docs team a heads up when you do think this is coming stable - because otherwise what is likely to happen is that you deliver a proper release and we're way behind on the docs.

W.r.t. Chrome, they removed the preference in 105; as there are no notes in browser compatibility information the assumption would be that their implementation was compatible with the spec at that point - or essentially does that the MDN docs says here https://developer.mozilla.org/en-US/docs/Web/API/Sanitizer/Sanitizer . If you know of specific places where they do not comply we can add notes and such to the browser compatibility information.

MDN marks this API as "experimental". We don't normally do more than that, though if it is really that unstable we might add a "warning" box. I'd rather not though.