WPT test for reading Blob URLs minted in iframes w/ and w/o sandboxing.

WPT test for reading Blob URLs minted in iframes w/ and w/o sandboxing.

BUG=686516
Review-Url: https://codereview.chromium.org/2678223002
Cr-Commit-Position: refs/heads/master@{#448886}
Committed: https://chromium.googlesource.com/chromium/src/+/9fa1e2b8da2b53ab1fbdfe85fc46227d3920913c

[pwnall, 2017-02-07 01:14:40]

The CQ bit was checked by pwnall@chromium.org to run a CQ dry run

[pwnall, 2017-02-07 01:15:15]

pwnall@chromium.org changed reviewers:
+ mek@chromium.org

[pwnall, 2017-02-07 01:15:16]

PTAL?

[commit-bot: I haz the power, 2017-02-07 01:15:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-07 01:19:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-07 01:19:57]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[pwnall, 2017-02-07 01:35:56]

The CQ bit was checked by pwnall@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-07 01:36:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pwnall, 2017-02-07 02:13:15]

On 2017/02/07 01:15:16, pwnall wrote:
> PTAL?

Also, FWIW, this passes in Firefox and Safari.

[commit-bot: I haz the power, 2017-02-07 02:54:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-07 02:54:42]

Dry run: This issue passed the CQ dry run.

[Marijn Kruisselbrink, 2017-02-07 19:02:41]

Did you also try this in Edge? (if only this was a wpt pull request, it would be
much easier to try this in multiple browsers...) Possibly not very helpful to
try in edge as is though, as their blob URL serialization isn't spec compliant.
Also from my limited testing weirdness seems to be going on with trying to use
fetch() to read blobs, while xhr generally works fine.

https://codereview.chromium.org/2678223002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/FileAPI/url/blob-url-in-sandboxed-iframe.html
(right):

https://codereview.chromium.org/2678223002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/FileAPI/url/blob-url-in-sandboxed-iframe.html:32:
// The script tag is closed in readBlobURL().
I think you meant readBlobFromUrl?

https://codereview.chromium.org/2678223002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/FileAPI/url/blob-url-in-sandboxed-iframe.html:65:
"The serialized origin in the Blob's URL should be null");
Note that the spec explicitly does not have any requirements for the serialized
origin in the Blob URL for an opaque origin. In this case the user agent can use
whatever it wants (not sure if any user agents do anything other than just
keeping null; other than edge, which currently doesn't serialize the origin in
the url in any situation...)

[pwnall, 2017-02-07 20:11:21]

The CQ bit was checked by pwnall@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-07 20:11:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pwnall, 2017-02-07 20:14:40]

On 2017/02/07 19:02:41, Marijn Kruisselbrink wrote:
> Did you also try this in Edge? (if only this was a wpt pull request, it would
be
> much easier to try this in multiple browsers...) Possibly not very helpful to
> try in edge as is though, as their blob URL serialization isn't spec
compliant.
> Also from my limited testing weirdness seems to be going on with trying to use
> fetch() to read blobs, while xhr generally works fine.

I just tried the test in Edge, and it times out because the browser doesn't
support <iframe srcdoc> [1]. FWIW, I voted for it [2].

Would you like me to pull out the iframe contents in a separate file, or would
you be OK with me landing this as-is?

[1] http://caniuse.com/#feat=iframe-srcdoc
[2]
https://wpdev.uservoice.com/forums/257854-microsoft-edge-developer/suggestion...

[pwnall, 2017-02-07 20:14:59]

Thank you very much for the quick feedback!

https://codereview.chromium.org/2678223002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/FileAPI/url/blob-url-in-sandboxed-iframe.html
(right):

https://codereview.chromium.org/2678223002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/FileAPI/url/blob-url-in-sandboxed-iframe.html:32:
// The script tag is closed in readBlobURL().
On 2017/02/07 19:02:40, Marijn Kruisselbrink wrote:
> I think you meant readBlobFromUrl?

Yes, thank you for catching this!

https://codereview.chromium.org/2678223002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/FileAPI/url/blob-url-in-sandboxed-iframe.html:65:
"The serialized origin in the Blob's URL should be null");
On 2017/02/07 19:02:40, Marijn Kruisselbrink wrote:
> Note that the spec explicitly does not have any requirements for the
serialized
> origin in the Blob URL for an opaque origin. In this case the user agent can
use
> whatever it wants (not sure if any user agents do anything other than just
> keeping null; other than edge, which currently doesn't serialize the origin in
> the url in any situation...)

IIUC, https://html.spec.whatwg.org/multipage/browsers.html#concept-origin says
that opaque origins serialize to "null", and that documents with sandboxed
origins get unique opaque origins. Am I misunderstanding something there?

[Marijn Kruisselbrink, 2017-02-07 20:22:22]

On 2017/02/07 at 20:14:40, pwnall wrote:
> On 2017/02/07 19:02:41, Marijn Kruisselbrink wrote:
> > Did you also try this in Edge? (if only this was a wpt pull request, it
would be
> > much easier to try this in multiple browsers...) Possibly not very helpful
to
> > try in edge as is though, as their blob URL serialization isn't spec
compliant.
> > Also from my limited testing weirdness seems to be going on with trying to
use
> > fetch() to read blobs, while xhr generally works fine.
> 
> I just tried the test in Edge, and it times out because the browser doesn't
support <iframe srcdoc> [1]. FWIW, I voted for it [2].

Ah, in that case probably fine to leave as is (since the test would fail anyway
due to their spec-incompliant blob url format).

https://codereview.chromium.org/2678223002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/FileAPI/url/blob-url-in-sandboxed-iframe.html
(right):

https://codereview.chromium.org/2678223002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/FileAPI/url/blob-url-in-sandboxed-iframe.html:65:
"The serialized origin in the Blob's URL should be null");
On 2017/02/07 at 20:14:59, pwnall wrote:
> On 2017/02/07 19:02:40, Marijn Kruisselbrink wrote:
> > Note that the spec explicitly does not have any requirements for the
serialized
> > origin in the Blob URL for an opaque origin. In this case the user agent can
use
> > whatever it wants (not sure if any user agents do anything other than just
> > keeping null; other than edge, which currently doesn't serialize the origin
in
> > the url in any situation...)
> 
> IIUC, https://html.spec.whatwg.org/multipage/browsers.html#concept-origin says
that opaque origins serialize to "null", and that documents with sandboxed
origins get unique opaque origins. Am I misunderstanding something there?

See https://w3c.github.io/FileAPI/#unicodeSerializationOfBlobURL, and in
particular step 6: "If serialized is "null", set it to an implementation-defined
value." So yes, the origin gets serialized to "null", but what actually gets
appended to the blob URL in that case could be anything...

[commit-bot: I haz the power, 2017-02-07 21:19:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-07 21:19:59]

Dry run: This issue passed the CQ dry run.

[pwnall, 2017-02-08 01:35:21]

The CQ bit was checked by pwnall@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-08 01:35:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pwnall, 2017-02-08 01:35:55]

Thanks for the patient explanation! PTAL?

https://codereview.chromium.org/2678223002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/FileAPI/url/blob-url-in-sandboxed-iframe.html
(right):

https://codereview.chromium.org/2678223002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/FileAPI/url/blob-url-in-sandboxed-iframe.html:65:
"The serialized origin in the Blob's URL should be null");
On 2017/02/07 20:22:22, Marijn Kruisselbrink wrote:
> On 2017/02/07 at 20:14:59, pwnall wrote:
> > On 2017/02/07 19:02:40, Marijn Kruisselbrink wrote:
> > > Note that the spec explicitly does not have any requirements for the
> serialized
> > > origin in the Blob URL for an opaque origin. In this case the user agent
can
> use
> > > whatever it wants (not sure if any user agents do anything other than just
> > > keeping null; other than edge, which currently doesn't serialize the
origin
> in
> > > the url in any situation...)
> > 
> > IIUC, https://html.spec.whatwg.org/multipage/browsers.html#concept-origin
says
> that opaque origins serialize to "null", and that documents with sandboxed
> origins get unique opaque origins. Am I misunderstanding something there?
> 
> See https://w3c.github.io/FileAPI/#unicodeSerializationOfBlobURL, and in
> particular step 6: "If serialized is "null", set it to an
implementation-defined
> value." So yes, the origin gets serialized to "null", but what actually gets
> appended to the blob URL in that case could be anything...

Ah, thanks!

This bit is tricky! Do you think we could get it removed from the spec, and
standardize on "null"? It seems like 3/4 engines are already on board.

In any case, I removed the assert from this test, as this seems like a
longer-term discussion.

[Marijn Kruisselbrink, 2017-02-08 01:51:35]

lgtm

[Marijn Kruisselbrink, 2017-02-08 02:16:22]

Oh, and feel free to file a spec bug to require "null" in the serialized origin
if the origin is opaque :) It would certainly make sense I think. I'm also
working on better defining how the origin of a blob URL should be determined (to
make sure that an opaque origin blob URL is actually considered same origin with
its opaque origin creator; the file api spec currently sort of says that that
should be the case, and this test relies on that being the case, but as
specified in the url and fetch specs they would currently not be considered same
origin).

[commit-bot: I haz the power, 2017-02-08 03:02:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-08 03:02:57]

Dry run: This issue passed the CQ dry run.

[pwnall, 2017-02-08 03:06:02]

The CQ bit was checked by pwnall@chromium.org

[commit-bot: I haz the power, 2017-02-08 03:06:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 03:13:16]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1486523162521150,
"parent_rev": "4e2788493f4bef375b52e78e5a9ee38f2d92b191", "commit_rev":
"9fa1e2b8da2b53ab1fbdfe85fc46227d3920913c"}

[commit-bot: I haz the power, 2017-02-08 03:13:51]

Description was changed from

==========
WPT test for reading Blob URLs minted in iframes w/ and w/o sandboxing.

BUG=686516
==========

to

==========
WPT test for reading Blob URLs minted in iframes w/ and w/o sandboxing.

BUG=686516

Review-Url: https://codereview.chromium.org/2678223002
Cr-Commit-Position: refs/heads/master@{#448886}
Committed:
https://chromium.googlesource.com/chromium/src/+/9fa1e2b8da2b53ab1fbdfe85fc46...
==========

[commit-bot: I haz the power, 2017-02-08 03:13:53]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/9fa1e2b8da2b53ab1fbdfe85fc46...