Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upURL.RevokeBlobURL is not strict enough about blob urls #24290
Comments
|
I'd like to work on this |
|
@SiddharthaMishra are you working on this? Otherwise, i'd like to start with it. |
|
Yeah. Sorry about delaying this, I'll finish up ASAP. |
|
@SiddharthaMishra can you assign it to yourself then? |
|
@highfive: assign me |
|
It looks like this has already been assigned to someone. I'll leave the decision to a core contributor. |
|
@jdm, since our blob URL store has keys which are UUIDs instead of the entire urls as in the standard, does it make sense to serialize the entire URL? Should the behavior of the URL store hashmap be changed to accept the entire urls as keys? |
|
@SiddharthaMishra I think the RevokeBlobURL message should be changed to accept a Url value instead of an id and an origin. The code that processes the message can parse the blob url and ignore it if there's any fragment or query string present. |
|
@SiddharthaMishra Are you working on this? May I work on this? |
Fix revoke blob url <!-- Please describe your changes on the following line: --> fix #24290 --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #24290 (GitHub issue number if applicable) <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because ___ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Fix revoke blob url <!-- Please describe your changes on the following line: --> fix #24290 --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #24290 (GitHub issue number if applicable) <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because ___ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Spec: https://w3c.github.io/FileAPI/#dfn-revokeObjectURL
Code:
components/script/dom/url.rscomponents/net_traits/blob_url_store.rsTest:
./mach test-wpt tests/wpt/web-platform-tests/FileAPI/url/url-with-xhr.any.js./mach test-wpt tests/wpt/web-platform-tests/FileAPI/url/url-with-fetch.any.jsThere are a couple problems:
parse_blob_urlhelper strips off any extra parts of the URL being revoked, whereas https://w3c.github.io/FileAPI/#removeTheEntry serializes the entire url