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

SRI: towards minimum viable SRI #74

Merged
merged 10 commits into from
Nov 6, 2014

Conversation

mozfreddyb
Copy link
Contributor

This is my stab to get the spec aligned with the consensus we discussed at last week's discussions.

More commits incoming, this first one is only up to section 3.4 (excluded).
I've also filed some minor issues on the tracker (#71, #73) which are untouched by this pull request.

@mozfreddyb
Copy link
Contributor Author

@mikewest, @metromoxie @devd:

Gentlemen, I hope you'll enjoy this fine read :-)
I tried to make the commits small enough to review. individually and can squash them before merge, if desired.

Note:

  • I left the fallback/reporting untouched, as I think we should just go for it.
  • I sneakily left the proposed restriction out, to only allow SRI on authenticated origins. Hoping that we get to fight a lot more about this on the mailing list. (There are no Bikeshed emoticons on Github, so I'll go with 🚲 🐚 )

@devd
Copy link
Contributor

devd commented Nov 3, 2014

a minor nit -- I was wondering if we should keep the original full spec in a separate file or branch?

@devd
Copy link
Contributor

devd commented Nov 3, 2014

(btw, I am taking a look too)

@mikewest
Copy link
Member

mikewest commented Nov 3, 2014

Good idea, @devd. Branched https://github.com/w3c/webappsec/tree/original-sri so we keep it around in a named branch.

I don't have a ton of time to review this right now, but I totally trust you three to do the right thing with the document! I hope I'll have some time to get back to this once MIX and CSP3 are further along.

@@ -575,14 +574,11 @@ A variety of HTML elements result in requests for resources that are to be
embedded into the document, or executed in its context. To support integrity
metadata for each of these, and new elements that are added in the future,
a new `integrity` attribute is added to the list of content attributes for
the `a`, `audio`, `embed`, `iframe`, `img`, `link`, `object`, `script`, `source`,
`track`, and `video` elements.
the `a`, `link`, and `script` elements.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably have a comment stating that this is still up in the air. Downloads, in I recall correctly at TPAC, did not come to consensus.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure about downloads and thought this just needs more discussion. I'm also OK with leaving it out if everyone agrees.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Joel's suggesting just adding a comment that this is still up in the air to clarify that this is not finalized. Lets leave it in, but just add a comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did that in 656d2f8 :)

@devd
Copy link
Contributor

devd commented Nov 4, 2014

lgtm. @metromoxie are you ok with merging? I agree we should have a comment about a download

@mozfreddyb
Copy link
Contributor Author

I did not hear any objections, so I will merge.

mozfreddyb added a commit that referenced this pull request Nov 6, 2014
@mozfreddyb mozfreddyb merged commit a10a47a into w3c:master Nov 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants