blob: URLs and range requests
Categories
(Core :: DOM: File, enhancement)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox118 | --- | fixed |
People
(Reporter: dlrobertson, Assigned: twisniewski)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
From the spec issue whatwg/fetch#1070, it looks like range requests of blob urls will be the equivalent of a fetch of a sliced blob. The allowed range will be a subset of the range spec. Multiple ranges should return an error. A basic wpt tests PR has been posted here
|
Reporter | |
Updated•2 years ago
|
|
|
Reporter | |
Comment 1•2 years ago
|
||
I'm going to make an attempt at implementing this, but this is just a project I'm working on in my free time, so progress will likely be slow. If the priority increases and the issue needs to be completed quicker, please let me know and I'll be happy to hand it off to someone else.
|
||
Comment 2•2 years ago
|
||
See https://github.com/whatwg/fetch/pull/1621 for some further tweaks to this algorithm and additional tests.
|
|
Reporter | |
Comment 3•1 year ago
|
||
I think this is at least related to bug 1733981, since the algorithm parse a single range header value is used in the both blob range request algorithm (here) and the algorithm CORS-safelisted request-header.
|
|
Reporter | |
Comment 4•1 year ago
|
||
Now that bug 1733981 is fixed, I'll move on to this bug
|
Assignee | |
Comment 5•1 year ago
|
||
:dlrobertson, actually I have a patch in the works here which passes the Fetch web-platform-tests. I haven't yet added support for XMLHttpRequest, but I can upload what I have so far if you'd like to work with that?
|
|
Reporter | |
Comment 6•1 year ago
|
||
Oh nice! That is awesome! Feel free to steal the bug as well... I actually haven't had the time to start on this yet
|
|
Assignee | |
Comment 7•1 year ago
|
||
|
|
Assignee | |
Comment 8•1 year ago
|
||
I will land this after the soft freeze.
|
||
Comment 11•1 year ago
|
||
Backed out for causing wpt failures in parsedepth.html
- Backout link
- Push with failures
- Failure Log
- Failure line: TEST-UNEXPECTED-FAIL | /_mozilla/xml/parsedepth.html | Parsing XML fails when the nesting depth is 5000 - assert_equals: expected 5000 but got 4900
|
|
Assignee | |
Comment 12•1 year ago
|
||
:dlrobinson, I'm not sure how this would cause testing/web-platform/mozilla/tests/xml/parsedepth.html to fail, going from a 5000 to 4900 limit. Do you have any ideas?
|
|
Reporter | |
Comment 13•1 year ago
|
||
(In reply to Thomas Wisniewski [:twisniewski] from comment #12)
:dlrobinson, I'm not sure how this would cause testing/web-platform/mozilla/tests/xml/parsedepth.html to fail, going from a 5000 to 4900 limit. Do you have any ideas?
The same thing happened for early attempts at fixing bug 1771423. I should have noticed that this would cause the same failure. The failure is related to this comment. IIRC ensuring that the created blobs for the test have text/xml or application/xml as a content type fixes this (this is the problematic line).
|
|
Assignee | |
Comment 14•1 year ago
|
||
I see! Then what do you think is the right fix here? Should we just change the content type to text/xml, and fail any related WPTs for now?
And do we know if Chrome and Safari also pass parsedepth.html, meaning they pass both sets of tests somehow?
|
|
Reporter | |
Comment 15•1 year ago
|
||
(In reply to Thomas Wisniewski [:twisniewski] from comment #14)
I see! Then what do you think is the right fix here? Should we just change the content type to text/xml, and fail any related WPTs for now?
I'm a fan of making patches easier, so I'd rather remove the change that preserves a blob's empty content type for an XHR. We can create another issue for that and tackle this issue without delaying getting the rest of this patch landed and into nightly.
And do we know if Chrome and Safari also pass parsedepth.html, meaning they pass both sets of tests somehow?
Yeah, they pass both sets of tests, but Safari does also automatically make the slice with an content-type of application/xml. Chrome passes the test and sets the content-type to "".
|
|
Assignee | |
Comment 16•1 year ago
|
||
:dlroberton, actually, would you be against updating parsedepth.html instead, so that it just sets the slice's content type to application/xml in its slice call here? That seems like a more reasonable fix to me in hindsight, at least until the spec makes a choice to automatically treat blobs as xml for XHRs.
|
|
Reporter | |
Comment 17•1 year ago
|
||
(In reply to Thomas Wisniewski [:twisniewski] from comment #16)
:dlroberton, actually, would you be against updating parsedepth.html instead, so that it just sets the slice's content type to application/xml in its slice call here? That seems like a more reasonable fix to me in hindsight, at least until the spec makes a choice to automatically treat blobs as xml for XHRs.
Yeah works for me! One of the early patches in bug 1771423 did this.
|
||
Comment 18•1 year ago
|
||
|
|
Reporter | |
Comment 19•1 year ago
|
||
Did some digging, and the reason we will not hit the expected block is due to us entering this block, so we will never use expat to parse the blob content (and never hit the relevant check).
The interesting part about this is that responseXML does specify in document-response that we should return early if response-mime-type is "not an HTML MIME type or an XML MIME type". However, "" is not a valid MIME type, so I think response-mime-type is text/xml and we should parse the body. Note that some minimal testing does indicate that Safari and Chrome do this.
|
||
Comment 21•1 year ago
|
||
| bugherder | ||
Description
•