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

Who is responsible for implementing Chunked-Transfer-Encoding #95

Closed
stuxcrystal opened this issue Jun 22, 2019 · 12 comments
Closed

Who is responsible for implementing Chunked-Transfer-Encoding #95

stuxcrystal opened this issue Jun 22, 2019 · 12 comments

Comments

@stuxcrystal
Copy link

I read the ASGI-Spec and it was not clear who is responsible for implementing Chunked-Transfer-Encoding for the event http.request.

Is the framework ultimately responsible for parsing whatever is sent over the raw socket or is the server responsible for supporting Transfer-Encoding?

@andrewgodwin
Copy link
Member

Do you mean inbound transfer encoding, or outbound? I presume you mean inbound, given you mentioned request.

My stance on this is that it's up to the framework to handle it, as the ASGI spec does not have high-level-enough messages in the HTTP extension to handle this - but it will give the framework the request body as it arrives, allowing it to be done there. This also reflects how WSGI handled it (which is to say, it didn't).

@davidism
Copy link

davidism commented Jun 23, 2019

WSGI servers ended up being the ones that handled decoding. In ASGI the distinction is clearer and it possibly makes sense for the app to handle it. Would be nice if there was a sans-io library for this, maybe there is already.

@tomchristie
Copy link
Member

Uvicorn automatically uses Transfer-Encoding: chunked For streaming response with no Content-Length set. I’ve not double checked Daphne or Hypercorn’s behaviour. I’d generally assume(?) that it makes sense that way, since it’s a required level of baseline functionality for streaming responses, and there’s no real value in forcing that onto framework-level implementations.

@stuxcrystal
Copy link
Author

stuxcrystal commented Jun 23, 2019

I think it'd be good if the ASGI-spec could explicitly define who is responsible for handling Transfer-Encoding. Otherwise, I think, nobody will implement it or worse: Both will handle it, mangling the incoming message.

@andrewgodwin
Copy link
Member

Outbound/response chunked encoding should be handled by servers, as it is in WSGI - I believe this is what @tomchristie is referring to. For inbound chunked encoding, I think it should be applications, if they wish. I will make a PR that clarifies this.

@andrewgodwin
Copy link
Member

OK, so I have whipped up a PR for the current situation (#96), but I'm not super thrilled about having things be so asymmetric - application-handled inbound, and server-handled outbound. Are there any advantages to doing it this way? Or should we have the servers handle the transfer-encoding overhead? (Especially as it's only for single hops, so there's no guarantee a reverse proxy isn't getting rid of it)

@davidism
Copy link

Transfer encoding doesn't really strike me as an application-level concern, so if it's reasonable for the server to handle decoding and encoding, that seems preferable.

@tomchristie
Copy link
Member

I think that chunked transfer encoding (which can be used on either requests or responses) should probably be a server concern. It’s not actually really an encoding, rather a message framing.

Other transfer encodings, such as gzip, compress, and brotli ought to be an application-level concern. All of those can can be applied in addition to chunking, but can only ever be set on the response, since there’s no HTTP mechanism for negotiating supported encodings for the request body.

@andrewgodwin
Copy link
Member

OK, let me revise the PR then.

@andrewgodwin
Copy link
Member

PR (#96) updated - I have asserted that Transfer-Encoding is always the server's job, while applications can apply gzip, deflate, etc as Content-Encoding should they wish.

@andrewgodwin
Copy link
Member

Last call - I'm going to merge the new PR that has Transfer-Encoding as the server's job both ways if nobody objects!

@andrewgodwin
Copy link
Member

Finally merged.

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

No branches or pull requests

4 participants