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

Support apps responding to Expect: 100-continue #3188

Open
dentarg opened this issue Jun 29, 2023 · 10 comments
Open

Support apps responding to Expect: 100-continue #3188

dentarg opened this issue Jun 29, 2023 · 10 comments

Comments

@dentarg
Copy link
Member

dentarg commented Jun 29, 2023

From #519 (comment)

Future work includes deferring sending the 100 Continue until the app begins reading the request body (this is how Go handles this).

There's a TODO in the code about this:

puma/lib/puma/client.rb

Lines 344 to 349 in b266b49

if @env[HTTP_EXPECT] == CONTINUE
# TODO allow a hook here to check the headers before
# going forward
@io << HTTP_11_100
@io.flush
end

@dentarg
Copy link
Member Author

dentarg commented Jun 29, 2023

Corresponding issue on Falcon: socketry/falcon#204

@dentarg
Copy link
Member Author

dentarg commented Jun 29, 2023

Example use-case, respond with a redirect where an upload should go: https://stackoverflow.com/questions/68172230/nginx-behavior-of-expect-100-continue-with-http-redirect

@dhavalsingh
Copy link
Contributor

#3200
Guys, a little help to make sure i am on the right path here 😅

@dhavalsingh
Copy link
Contributor

dhavalsingh commented Aug 8, 2023

I've gone through some other server implementations and some docs on how the Expect: 100-Continue header should work. Here is what I understood.

The Expect: 100-continue header has a special status in HTTP. It allows the client to send an Expect: 100-continue header with the request headers and then pause request sending (i.e. hold back sending the request entity). The server reads the request headers, determines whether it wants to accept the request, and responds with

  • 417 Expectation Failed, if it doesn't support the 100-continue expectation (or if the Expect header contains other, unsupported expectations).
  • a 100 Continue response, if it is ready to accept the request entity and the client should go ahead with sending it
    a final response (like a 4xx to signal some client-side error (e.g. if the request entity length is beyond the configured limit) or a 3xx redirect)
  • Only if the client receives a 100 Continue response from the server is it allowed to continue sending the request entity. In this case, it will receive another response after having completed the request sending.
    So this special feature breaks the normal "one request - one response" logic of HTTP!
    It, therefore, requires special handling in all HTTP stacks (client- and server-side).

For us, this means:

on puma:

  • After having read an Expect: 100-continue header with the request we package up an HTTP Request with just the header and send it through to the application. Only when (and if) the application then requests data from the entity stream do we send out a 100 Continue response and continue reading the request entity. The application will therefore be able to determine whether it wants the client to send the request entity by deciding whether to look at the request entity data stream or not.

Now, this header is handled in different ways by different clients. For eg: in Curl, If the server doesn't send a '100 Continue' response within a certain time (1 second by default), curl will go ahead and send the body anyway.

Current Behaviour on Puma:

  • If a request contains the 100-continue header, the puma server without letting the app intervene sends a 100-continue response back, reads the body, and sends it to the app to process.

Expected Behaviour:

  • Add a config var, lets say app_responds_100_continue. When set to true, it will send the req to the app, let the app respond however it wants (if the app sends 100-continue then It will get the request body or the app can send 4xx or 307 etc and end the cycle there).
  • If the app responds with 100, then read the body and fwd it to the app again.

Now my main issue is understanding the last part

If the app responds with 100, then read the body and fwd it to the app again.

How can we implement this in puma. I've gone through the codebase but I'm not 100% sure if this is doable.

@dentarg @nateberkopec can you guys help me guide me a bit here?

@dentarg
Copy link
Member Author

dentarg commented Aug 8, 2023

I don't know either if it is doable.

Maybe the best we can do is offer a hook via the Puma config, similar to the "low level error handler", where the user can define a response to be used instead of having the actual rack app handle the request. If the user returns false from this handler, everything continues like today (Puma sends the 100-continue header, app handles the request).

@dhavalsingh
Copy link
Contributor

Makes sense, will go through the implementation of "low level error handler". Got caught up in some work stuff.

@nateberkopec
Copy link
Member

Idea:

You can configure a Proc callback, say, called continue_callback.

This continue_callback receives 1 argument, the Rack env. It's basically the normal Rack env, minus any request body (because that obviously hasn't been sent yet).

continue_callback can return one of two things:

  1. If it returns true, then Puma sends a 100-continue response and yields control back to the normal flow (eventually to the regularly configured Rack application)
  2. Otherwise, it may return any valid Rack response with a 3xx or 4xx status code (e.g. [307, {"HTTP_LOCATION": "whatever.com/url"}, [""]])

Whaddyathink?

@dentarg
Copy link
Member Author

dentarg commented Aug 22, 2023

Yeah that's exactly what I had in mind (but I choose "false" instead of "true" to indicate the NOOP path – maybe good because "true" and array are both considered truthy?)

@nateberkopec
Copy link
Member

"false" instead of "true" to indicate the NOOP path – maybe good because "true" and array are both considered truthy?

Fair enough 👍

@dhavalsingh
Copy link
Contributor

Sounds good. I'm on a small trip atm. Will pick it up once I'm back this weekend!

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

3 participants