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

Remove image/media origins and check origin-cleanness directly #6656

Merged
merged 3 commits into from
May 6, 2021

Conversation

domenic
Copy link
Member

@domenic domenic commented May 5, 2021

That is, check the type of response directly, instead of synthesizing an opaque origin for opaque responses and then comparing that to the entry settings object's origin.

This helps with #1431 by removing various uses of the entry concept, and closes #2761 by removing the origin concept for image and media elements entirely, since it is now unused.

  • At least two implementers are interested (and none opposed):
    • By code inspection this matches Chromium
    • I strongly suspect this also matches other browsers
  • Tests are written and can be reviewed and commented upon at:
    • I suspect this is tested in depth. I'm not 100% sure when the old model differs from the one proposed here. I'd like to ask that we don't make me test it since this is so much clearly more correct and probably these sorts of security-sensitive areas have a lot of tests already.
  • Implementation bugs are filed:
    • N/A

(See WHATWG Working Mode: Changes for more details.)


/canvas.html ( diff )
/imagebitmap-and-animations.html ( diff )
/images.html ( diff )
/origin.html ( diff )

That is, check the type of response directly, instead of synthesizing an opaque origin for opaque responses and then comparing that to the entry settings object's origin.

This helps with #1431 by removing various uses of the entry concept, and closes #2761 by removing the origin concept for image and media elements entirely, since it is now unused.
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

This is amazing. I'm surprised it ended up being such a straightforward patch.

Note that this is also fixes #2813. And given that @jakearchibald might want to look at this or cheer or some such. 😊

<span>CORS-same-origin</span> or <span>CORS-cross-origin</span>; this affects the
<span>origin</span> of the image itself (e.g., when used on a <code>canvas</code>).</p></li>
<span>CORS-same-origin</span> or <span>CORS-cross-origin</span>; this affects the image's
interaction with other APIs (e.g. when used on a <code>canvas</code>).</p></li>
Copy link
Member

Choose a reason for hiding this comment

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

Keep the comma?

@jakearchibald
Copy link
Contributor

Yessssss!

@domenic domenic merged commit 2f69571 into main May 6, 2021
@domenic domenic deleted the image-video-data-origin branch May 6, 2021 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Make definition of "origin" less spread out, and more layered
3 participants