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

Add Transfer-Encoding: chunked + Trailer #72

Open
Prinzhorn opened this issue Jan 31, 2022 · 3 comments
Open

Add Transfer-Encoding: chunked + Trailer #72

Prinzhorn opened this issue Jan 31, 2022 · 3 comments

Comments

@Prinzhorn
Copy link

Prinzhorn commented Jan 31, 2022

I'm planning on using go-httpbin for integration tests of an HTTP intercepting proxy. Thanks for maintaining it!

I will come up with some more "exotic" feature requests and have to see how easy it is to add those myself.

I'd love to see support for chunked encoding with trailers.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Transfer-Encoding
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Trailer
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Server-Timing

Here's a rough idea:

/chunked/:n Generates a chunked response with n random chunks of variable size, a Transfer-Encoding: chunked header and an optional Server-Timing trailer

Parameters:

  • minsize=1: Min number of random bytes
  • maxsize=1024: Max number of random bytes
  • trailer=1: Include the Trailer: Server-Timing header and Server-Timing trailer

Does that sound like something you would accept? Do you see any problems? I don't really care about Server-Timing directly, but I need any trailer. Foo would do but I thought we might as well use something that's actually used IRL and has some browser support.

To support max-body-size we could just check the worst case maxsize * n before doing anything.

@Prinzhorn
Copy link
Author

I've realized how little trailers are used (aside from more specific uses such as in gRPC, which runs over HTTP/2). Also some of the tooling I'm using doesn't even support them, so that part is way less important than the chunked encoding. But I think the trailers are basically free to implement.

@mccutchen
Copy link
Owner

So, at a high level, I'm in favor of some combination of a) adding a new, more explicit and hopefully discoverable /chunked endpoint b) making the Transfer-Encoding: chunked behavior of the /stream, /stream-bytes, and /drip1 endpoints more obvious/discoverable/explicit and c) adding the ability to specify trailers to any or all of the above.

Quick demo of /stream using chunked transfer encoding:

$ curl --verbose --no-buffer --http1.1 https://httpbingo.org/stream/5 2>&1 | grep -e 'HTTP/1.1' -e 'transfer-encoding'
> GET /stream/5 HTTP/1.1
< HTTP/1.1 200 OK
< transfer-encoding: chunked

I suspect this will require a bit of fighting w/ the Golang stdlib net/http server's implementation, which automagically handles2 chunked transfer encoding in some(?) cases. We get that behavior implicitly in the endpoints above via our use of the http.Flusher interface (docs, example usage in the handler for /stream).

I've never dealt with trailers myself, so I have no idea what it would take to add that support to go-httpbin. As you point out the primary use case I've seen is for gRPC, but it should at least be possible.

Footnotes

  1. In poking around at this issue, I noticed that /drip does not currently send back Transfer-Encoding: chunked, at least as currently served by https://httpbingo.org/, though I expected it to. No idea if this is due to my host, or some difference in the way I'm using the Flusher interface in that handler (e.g. maybe writing a status code breaks something here). But this is kinda what I mean by having to fight w/ the stdlib to support the kind of functionality you have in mind!

  2. The automagical handling of Transfer-Encoding also makes it tough to write the kinds of test cases I'd like to write for these endpoints, as noted in this comment. Maybe there's a better way to do all of this, though, so any improvements here are welcome!

@Prinzhorn
Copy link
Author

Thanks for looking into this. All the things you've listed sound reasonable. I didn't know that some endpoints already support chunked.

I'll probably get to the integration tests within the next three month. From what I can tell the exiting endpoints might already get me almost 100% of the way.

mccutchen added a commit that referenced this issue Jun 29, 2023
This started with me wading into the `/drip` endpoint to add
`Server-Timing` trailers as suggested in
#72.

In making those changes, I discovered that the standard golang net/http
server we're building on only supports sending trailers when the
response is written using `Content-Encoding: chunked`. Initially, I
thought it would be reasonable for this endpoint, which is designed to
stream writes to the client, to switch over to a chunked response, but
upon further consideration I don't think that's a good idea. So I'll
defer adding trailers to some later work.

But, in thinking this through (and fighting with the unit tests already
in place for drip), I realized that the `/drip` endpoint had room for
improvement, if we think about it as an endpoint _designed to let
clients test their behavior when an HTTP server is responding very
slowly_.

So, we ended up with these changes:

Initial delay semantics:
- Originally, `/drip` would immediately write the desired status code
and then wait for the initial delay (if any).
- Here we update that so that it waits for the initial delay before
writing anything to the response, to better simulate a server that is
just very slow to respond at all.

Incremental write timings:
- Switch over to a ticker to more accurately pace the incremental writes
across the requested duration
- Stop sleeping after the final byte is written (should not affect
clients, but avoids wasting server resources)

Worth noting that I now believe it's important that this endpoint
**not** switch over to chunked content encoding, because it is useful to
maintain and endpoint that simulates a server very slowly writing a
"normal" HTTP response.
mccutchen added a commit that referenced this issue Sep 16, 2024
This adds a new `/trailers` endpoint which allows clients to specify
trailer key/value pairs in the query parameters, similar to the existing
`/cookies/set` and `/response-headers` endpoints.

Per discussion on #72,
we'll likely add more useful trailers to some of the existing endpoints
as a follow-up.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants