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

Integrate streams into HTTP redirect #26686

Open
gterzian opened this issue May 28, 2020 · 2 comments
Open

Integrate streams into HTTP redirect #26686

gterzian opened this issue May 28, 2020 · 2 comments

Comments

@gterzian
Copy link
Member

@gterzian gterzian commented May 28, 2020

Follow-up on #25873 (comment)

The current integration means the request body is "lost" in the case of a re-direct, I think because it has been streamed already and therefore the stream is "done".

So the spec actually says we should "re-extract" a new stream from the original source object, see Step 12 of https://fetch.spec.whatwg.org/#ref-for-concept-http-redirect-fetch

I think this would require storing the original object in a Trusted<OriginalSource> on the TransmitBodyConnectHandler(where OriginalSource is probably an enum for the various sources), and then add another IPC message to BodyChunkRequest, say a ReExtract<(IpcReceiver<BodyChunkRequest>> one, that would be handled in script by:

  1. queuing a task to extract the source,
  2. Use the IpcReceiver<BodyChunkRequest> to essentially do into_net_request_body except that it wouldn't produce a body, and it would use the receiver to setup the route and create a new TransmitBodyConnectHandler to then later transmit the body for the re-direct.

On the net side of things, you would have to create an IPC channel, and then set the receiver to replace the exiting stream of the RequestBody, and then send the receiver over IPC back to script, using the original sender found on stream.

I think this roughly describes what needs to be done, but it might require some additional looking into.

@gterzian
Copy link
Member Author

@gterzian gterzian commented May 28, 2020

I think it's a good way to learn about, or look more into, the whole IPC/event-loop integration and script to net communication stuff.

@gterzian
Copy link
Member Author

@gterzian gterzian commented May 31, 2020

This is partially addressed by the "partially integrate streaming request bodies with http re-direct" commit in #25873, although not by actually re-extracting a stream(it only works with sources that have already been read into memory), and it won't work with file-based blobs.

So the goal of this issue would be to replace the implementation of that commit with something that would actually queue a task and re-extract a stream from the Dom object. The IPC setup is already largely done.

cc @jdm (so we don't have any regressions from that PR anymore)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.