LibWeb: Partition Blob URL fetches by Storage Key #3303
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shannonbooth
force-pushed
the
blob-partioning
branch
2 times, most recently
from
bb21920 to
36c4a08
Compare
tcl3
reviewed
Jan 20, 2025
| bool is_top_level_navigation = request->destination() == Infrastructure::Request::Destination::Document; | ||
|
|
||
| // 5. If isTopLevelNavigation is false and requestEnvironment is null, then return a network error. | ||
| if (is_top_level_navigation && !request_environment) |
There was a problem hiding this comment.
Should this be !is_top_level_navigation?
There was a problem hiding this comment.
oof, yes it should, good spot, thanks! Now I need to figure out how to write a test for this one, doesn't seem trivial with my current understanding of fetch 😅
There was a problem hiding this comment.
I can't seem to find any method of trigging this, every caller of fetch that I can see from a search sets the client of the request. So I've just pushed a fix for now.
Did happen to find a bug with not supporting blob URLs when trying to find a repro for this, will raise a separate PR for that.
Instead of just putting in members directly, wrap them up in structs which represent what a URL blob entry is meant to hold per the spec. This makes more obvious what this is meant to represent, such as the ByteBuffer being used to represent the bytes behind a Blob. This also allows us to use a stronger type for a function that needs to return a Blob URL entry's object.
When we serialize blob URL entries we do not serialize the entire environment settings object and only use it's origin. To allow a Blob URL entry to access its relevant storage key, add a getter that simply takes an origin.
This was a security mechanism introduced in the fetch spec, with supporting AOs added to the FileAPI spec.
shannonbooth
force-pushed
the
blob-partioning
branch
from
36c4a08 to
f0dad7f
Compare
tcl3
approved these changes
Jan 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was a security mechanism introduced in the fetch spec, with
supporting AOs added to the FileAPI spec.