Skip to content
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

Make the return value of "Parse metadata" consistent #86

Merged
merged 1 commit into from
Mar 2, 2015

Conversation

fmarier
Copy link
Member

@fmarier fmarier commented Nov 7, 2014

The output of this algorithm should be the same in all three
cases:

  • the empty string
  • a list of one or more metadata with unrecognized hash functions
  • a list of one or more metadata with invalid ni URI

This change makes the last two cases match the output of the first.

@fmarier
Copy link
Member Author

fmarier commented Dec 23, 2014

This is the opposite of #120.

The output of this algorithm should be the same in all three
cases:

- the empty string
- a list of one or more metadata with unrecognized hash functions
- a list of one or more metadata with invalid ni URI

This change makes the last two cases match the output of the first.
@devd
Copy link
Contributor

devd commented Feb 25, 2015

I am not sure if the right thing is to return empty value from algorithm in all three cases or to treat all three as empty in the spec. The different values could allow for flexibility in the future, maybe? And I imagine UAs might want to implement that anyhow to differentiate warnings in the console. WDYT @mozfreddyb @metromoxie

@joelweinberger
Copy link
Contributor

I don't feel too strongly either way. I think the point about warnings is good, but UAs could, of course, juts do that mid-algorithm anyway.

@fmarier
Copy link
Member Author

fmarier commented Feb 25, 2015

@devd Speaking for the Firefox implementation, the algorithm in the spec is not really what I've implemented anyways. I do check for various ways in which things fail to be valid.

However, I think these are implementation details and that the spec algorithm can omit these details and focus on being simple to understand and reason about.

@mozfreddyb
Copy link
Contributor

I hope that users and authors in our spec audience will outnumber us user agent people. So maybe we should optimize for clarity, not for implementation?

(I was going to say priority of constituencies, but forgot the exact term.)

@mikewest
Copy link
Member

I'd agree that you should optimize for clarity, but I would tentatively suggest that the prose in your specification is for authors and users, and it should explain enough for them to understand both the intent and impact of the feature. The algorithms are probably best written with implementers as the target audience.

@devd
Copy link
Contributor

devd commented Mar 2, 2015

lets merge this for now and then take another edit pass at it later. I imagine we will need to edit the use of ni:// URIs too.

devd added a commit that referenced this pull request Mar 2, 2015
Make the return value of "Parse metadata" consistent
@devd devd merged commit a2ff4da into w3c:master Mar 2, 2015
fmarier pushed a commit to fmarier/webappsec that referenced this pull request Aug 14, 2015
This commit reintroduces a distinction that used to exist in the spec
prior to 930ec20 (see w3c#86 and w3c#119).

As [discussed on the list](https://lists.w3.org/Archives/Public/public-webappsec/2015Aug/0006.html),
it helps developers catch mistakes by failing closed on CORS errors
when the `integrity` attribute is non-empty (an indication that the
developer meant to use SRI).

Prior to this commit, `integrity` attributes which consists of only
invalid metadata would fail open (silently load).
fmarier pushed a commit to fmarier/webappsec that referenced this pull request Sep 23, 2015
This commit reintroduces a distinction that used to exist in the spec
prior to 930ec20 (see w3c#86 and w3c#119).

As [discussed on the list](https://lists.w3.org/Archives/Public/public-webappsec/2015Aug/0006.html),
it helps developers catch mistakes by failing closed on CORS errors
when the `integrity` attribute is non-empty (an indication that the
developer meant to use SRI).

Prior to this commit, `integrity` attributes which consists of only
invalid metadata would fail open (silently load).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants