-
Notifications
You must be signed in to change notification settings - Fork 35
Update references and clarify section 3.5 in the current version of the spec #112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -36,7 +36,6 @@ spec: ABNF; urlPrefix: https://tools.ietf.org/html/rfc5234 | |||||
text: VCHAR; url: appendix-B.1 | ||||||
text: WSP; url: appendix-B.1 | ||||||
|
||||||
|
||||||
spec: Fetch; urlPrefix: https://fetch.spec.whatwg.org | ||||||
type: dfn | ||||||
text: fetch; url: concept-fetch | ||||||
|
@@ -213,10 +212,7 @@ spec: SHA2; urlPrefix: http://csrc.nist.gov/publications/fips/fips180-4/fips-180 | |||||
specified in RFC5234. [[!ABNF]] | ||||||
|
||||||
<a href="https://tools.ietf.org/html/rfc5234#appendix-B.1">Appendix B.1</a> of | ||||||
[[!ABNF]] defines <a>VCHAR</a> (printing characters). | ||||||
|
||||||
<a>WSP</a> (white space) characters are defined in <a href="http://www.w3.org/TR/html5/infrastructure.html#space-character">Section 2.4.1 Common parser idioms</a> of the HTML 5 specification as | ||||||
<code>White_Space characters</code>. [[!HTML5]] | ||||||
[[!ABNF]] defines <a>VCHAR</a> (printing characters) and <a>WSP</a> (white space). | ||||||
|
||||||
</section> | ||||||
|
||||||
|
@@ -247,8 +243,8 @@ spec: SHA2; urlPrefix: http://csrc.nist.gov/publications/fips/fips180-4/fips-180 | |||||
|
||||||
This metadata MUST be encoded in the same format as the `hash-source` (without | ||||||
the single quotes) in <a | ||||||
href="http://www.w3.org/TR/CSP2/#source-list-syntax">section 4.2 of the Content | ||||||
Security Policy Level 2 specification</a>. | ||||||
href="http://www.w3.org/TR/CSP3/#framework-directive-source-list">section | ||||||
2.3.1 of the Content Security Policy Level 3 specification</a>. [[!CSP3]] | ||||||
|
||||||
For example, given a script resource containing only the string `alert('Hello, | ||||||
world.');`, an author might choose <a>SHA-384</a> as a hash function. | ||||||
|
@@ -451,24 +447,16 @@ spec: SHA2; urlPrefix: http://csrc.nist.gov/publications/fips/fips180-4/fips-180 | |||||
valid metadata as described by the following ABNF grammar: | ||||||
|
||||||
<pre dfn-type="grammar" link-type="grammar"> | ||||||
<dfn>integrity-metadata</dfn> = *<a>WSP</a> <a>hash-with-options</a> *(1*<a>WSP</a> <a>hash-with-options</a> ) *<a>WSP</a> / *<a>WSP</a> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2.2 Grammatical Concepts defines Therefore, in 2.2 Grammatical Concepts, it seems that |
||||||
<dfn>hash-with-options</dfn> = <a>hash-expression</a> *("?" <a>option-expression</a>) | ||||||
<dfn>option-expression</dfn> = *<a>VCHAR</a> | ||||||
<dfn>hash-algo</dfn> = <hash-algo production from [Content Security Policy Level 2, section 4.2]> | ||||||
<dfn>base64-value</dfn> = <base64-value production from [Content Security Policy Level 2, section 4.2]> | ||||||
<dfn>integrity-metadata</dfn> = *<a>WSP</a> <a>hash-expression</a> *(1*<a>WSP</a> <a>hash-expression</a> ) *<a>WSP</a> / *<a>WSP</a> | ||||||
<dfn>hash-algo</dfn> = <hash-algo production from [Content Security Policy Level 3, section 2.3.1]> | ||||||
<dfn>base64-value</dfn> = <base64-value production from [Content Security Policy Level 3, section 2.3.1]> | ||||||
<dfn>hash-expression</dfn> = <a>hash-algo</a> "-" <a>base64-value</a> | ||||||
</pre> | ||||||
|
||||||
`option-expression`s are associated on a per `hash-expression` basis and are | ||||||
applied only to the `hash-expression` that immediately precedes it. | ||||||
|
||||||
In order for user agents to remain fully forwards compatible with future | ||||||
options, the user agent MUST ignore all unrecognized `option-expression`s. | ||||||
|
||||||
Note: Note that while the `option-expression` has been reserved in the syntax, | ||||||
no options have been defined. It is likely that a future version of the spec | ||||||
will define a more specific syntax for options, so it is defined here as broadly | ||||||
as possible. | ||||||
|
||||||
Note: Since no `options` are not defined (see the | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
[[#integrity-metadata-description]]), the above ABNF syntax does not consider | ||||||
them. If `options` are defined in a future version, the ABNF syntax should be | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
modified accordingly. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be ok this way? @annevk There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Modulo nits, I think so. |
||||||
|
||||||
## Handling integrity violations ## {#handling-integrity-violations} | ||||||
|
||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the previous text makes no sense, but I wonder if it was meant to match https://infra.spec.whatwg.org/#ascii-whitespace? That would be
LWSP / %x0C
.This also seems like a problem with the parser. It references https://infra.spec.whatwg.org/#strictly-split but doesn't define spaces. I suspect it meant to reference https://infra.spec.whatwg.org/#split-on-ascii-whitespace?
Are there tests for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you said, ASCII whitespace seems to be equivalent to
LWSP
. And the implementation, Chromium, use ASCIISpace defined as follows.In this situation, as you said, I think it would be better to refer to split-on-ascii-whitespace rather than strictly-split for the parser. And it seems fitting to use
LWSP
, which refers to ASCII whitespace, instead ofWSP
. The ABNF in Section 3.5 will also need to be modified accordingly. Does it make sense?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So Chrome's definition also counts U+000B as whitespace if I'm reading that correctly. That seems wrong.
And note that LWSP excludes U+000C (which is why I added that above).
I think we should use ASCII whitespace here and file a bug on Chrome for them including U+000B. And use
LWSP / %x0C
in ABNF.