Closed Bug 1389421 Opened 7 years ago Closed 4 years ago

Support nonce IDL property

Categories

(Core :: DOM: Core & HTML, enhancement, P2)

enhancement

Tracking

()

RESOLVED DUPLICATE of bug 1374612

People

(Reporter: ayg, Unassigned)

References

Details

Attachments

(1 file)

The HTML standard defines a "nonce" attribute on HTMLLinkElement, HTMLScriptElement, and HTMLStyleElement:

https://html.spec.whatwg.org/#dom-link-nonce
https://html.spec.whatwg.org/#dom-script-nonce
https://html.spec.whatwg.org/#dom-style-nonce

They don't show up in tests:

http://w3c-test.org/html/dom/interfaces.html

Do we not implement the nonce property?  Or we do but we hide it?  Is this specced somewhere?  I couldn't find docs or discussion on this point after a bit of searching.
Flags: needinfo?(francois)
Flags: needinfo?(dveditz)
Christoph might know...
Flags: needinfo?(francois) → needinfo?(ckerschb)
A nonce provides a mechanism which allows inline scripts, styles to comply with the page's Content Security Policy and hence provides a mechanism to safely whitelist such scripts, styles.

As far as CSP goes, we support nonces and have test coverage within dom/security/csp as well as testing/web-platform/tests/content-security-policy/

See:
  https://dxr.mozilla.org/mozilla-central/search?q=nonce%3D%22&redirect=false

Nonces were introduced within CSP2 and are still speced within CSP3:
  https://www.w3.org/TR/CSP/

Even though we haven't explicitly decided to hide the nonce, it's probably something we want to consider:
  https://bugzilla.mozilla.org/show_bug.cgi?id=1374612
Flags: needinfo?(ckerschb)
It does seem that even if we hide the content attribute from the DOM as per that proposal (whatever that ends up being, there doesn't appear to be consensus on a direction), the IDL attribute (which this bug is about) is missing for no good reason. So we should probably add that?
(In reply to Anne (:annevk) from comment #3)
> It does seem that even if we hide the content attribute from the DOM as per
> that proposal (whatever that ends up being, there doesn't appear to be
> consensus on a direction), the IDL attribute (which this bug is about) is
> missing for no good reason. So we should probably add that?

Agreed!
Flags: needinfo?(dveditz)
Priority: -- → P2
The problem is just that nobody updated the IDL files.
Assignee: nobody → ayg
Status: NEW → ASSIGNED
Summary: nonce attribute does not show up in tests → Support nonce IDL property
Comment on attachment 8899429 [details]
Bug 1389421 - Support nonce IDL property;

https://reviewboard.mozilla.org/r/170706/#review175902

It is a tad unclear to me what will happen to the content attributes, but I guess this implementation will then just be changed to use something else than content attributes.
Attachment #8899429 - Flags: review?(bugs) → review+
Yeah, if at some point there's consensus on hiding the content attributes we'll change to change things around, but much more will need to change anyway at that point so that shouldn't block this.
Comment on attachment 8899429 [details]
Bug 1389421 - Support nonce IDL property;

Re-requesting review for the test changes, since it's not absolutely obvious to me that they're what we want.
Attachment #8899429 - Flags: review+ → review?(bugs)
Comment on attachment 8899429 [details]
Bug 1389421 - Support nonce IDL property;

https://reviewboard.mozilla.org/r/170706/#review176016

I'd prefer if also ckerschb would take a look at this.
Attachment #8899429 - Flags: review?(bugs) → review+
Comment on attachment 8899429 [details]
Bug 1389421 - Support nonce IDL property;

https://reviewboard.mozilla.org/r/170706/#review176020

thanks for adding, r=ckerschb
Attachment #8899429 - Flags: review+
Pushed by ayg@aryeh.name:
https://hg.mozilla.org/integration/autoland/rev/2e5306010ce1
Support nonce IDL property; r=ckerschb,smaug
https://hg.mozilla.org/mozilla-central/rev/2e5306010ce1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
The changes to the .tentative.html tests in this patch in the nonce-hiding are wrong.  Those are testing a spec change proposal (hence "tentative") that decouples .nonce from the nonce attribute; see <https://github.com/whatwg/html/pull/2373>.  Please make sure those changes get backed out ASAP; we want to make sure we don't upstream them.
Flags: needinfo?(ckerschb)
Flags: needinfo?(bugs)
Why are there tentative wpt tests. Makes no sense.
I guess I'll back the whole thing out.
(In reply to Olli Pettay [:smaug] from comment #17)
> I guess I'll back the whole thing out.
I agree, we should back out the whole changeset.
Flags: needinfo?(ckerschb)
Chatted with RyanVM on IRC, he will back out that change for us once the trees reopen.
Flags: needinfo?(ryanvm)
Backed out at Christoph's request.
https://hg.mozilla.org/integration/mozilla-inbound/rev/05944b9f04aa5f4a35a8c331170ffc343ba122f2
Assignee: ayg → nobody
Status: RESOLVED → REOPENED
Flags: needinfo?(ryanvm)
Flags: needinfo?(bugs)
Resolution: FIXED → ---
Target Milestone: mozilla57 → ---
Status: REOPENED → NEW
> Why are there tentative wpt tests.

Because people want to be able to share tests for things that are still being discussed.
Why are such tests in wpt? Sure such tests could be shared in authors' own branches of wpt.
It is similar if one would push tentative code changes to production code.
The difference is that WPT has an explicit "this is not passing, and that's ok" mechanism and that the tests are clearly marked as "ok if this fails".

They're in wpt so they can be shared across browsers and used to test implementations of the proposals involved.
And to be clear, the whole changeset did not need to be backed out; just the changes to the tentative tests. Though if the proposal in https://github.com/whatwg/html/pull/2373 were implemented the implementation of .nonce would need to be changed quite a bit, of course.
I'd suggest we dupe this into bug 1374612? I doubt the DOM Core team has the bandwidth to take this on anytime soon, unless the priority is suddenly different than it was a year ago, and it doesn't seem like we need two bugs to track the same thing.

This issue affects jQuery as we now need to use both the property & the attribute to set nonce properly:
https://github.com/jquery/jquery/blob/29a9544a4fb743491a42f827a6cf8627b7b99e0f/src/core/DOMEval.js#L23-L33
While the code is needed for Edge as well, jQuery 4.0 will most likely not support EdgeHTML so Firefox support will be the only blocker.

Status: NEW → RESOLVED
Closed: 7 years ago4 years ago
Resolution: --- → DUPLICATE

Anne, it might make sense to implement the property even if we don't do the hiding thing, right? See comment 28 for the reasons we might want to do that.

Flags: needinfo?(annevk)

Sure, but the other bug has a patch, is being worked on, and at this point is really what we should do if we decide to spend time on this.

Flags: needinfo?(annevk)
You need to log in before you can comment on or make changes to this bug.