fix(proxy): handle Expect 100-continue without deadlock - #19
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a28c6095b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // the header so the origin reads the subsequently forwarded body normally. | ||
| downCounter := &countingWriter{w: conn} | ||
| if headerHasToken(req.Header.Values("Expect"), "100-continue") { | ||
| req.Header.Del("Expect") |
There was a problem hiding this comment.
When a request contains Expect: 100-continue, foo or separate Expect fields for those values, this branch acknowledges only 100-continue but deletes the entire header. This prevents the origin from seeing and acting on the remaining end-to-end expectation, potentially causing it to process a request it would otherwise reject or handle differently; remove only the consumed 100-continue token and retain the other values.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b381b2ffb4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| keptValues := make([]string, 0, len(header.Values(name))) | ||
| for _, value := range header.Values(name) { | ||
| keptTokens := make([]string, 0) | ||
| for _, rawToken := range strings.Split(value, ",") { |
There was a problem hiding this comment.
For a valid extension such as Expect: foo="a,100-continue,b", splitting on every comma treats the text inside the quoted string as separate list members, falsely detects 100-continue, sends an unsolicited interim response, and rewrites the remaining expectation as foo="a, b". Parse the Expect field's quoted-string syntax so only a top-level 100-continue list member is consumed.
Useful? React with 👍 / 👎.
| if removeHeaderToken(req.Header, "Expect", "100-continue") { | ||
| if _, err := io.WriteString(downCounter, "HTTP/1.1 100 Continue\r\n\r\n"); err != nil { |
There was a problem hiding this comment.
When this locally generated 100 Continue is written successfully but the origin then closes, times out, or returns a malformed response, remoteReader.read returns through line 229 without calling AddDown. The 25 bytes already sent to the client are therefore omitted from traffic statistics; account for downCounter.n on that error path as is already done for the other exits.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0042eb4ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if removeHeaderToken(req.Header, "Expect", "100-continue") { | ||
| if _, err := io.WriteString(downCounter, "HTTP/1.1 100 Continue\r\n\r\n"); err != nil { |
There was a problem hiding this comment.
When an HTTP/1.0 request includes Expect: 100-continue, this branch unconditionally writes an HTTP/1.1 informational response. HTTP/1.0 clients do not support 1xx responses, and servers must not send them to such clients, so the response can be misinterpreted as the final response; only generate the local 100 Continue when req.ProtoAtLeast(1, 1) is true.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a404b8a105
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if req.ProtoAtLeast(1, 1) && removeHeaderToken(req.Header, "Expect", "100-continue") { | ||
| if _, err := io.WriteString(downCounter, "HTTP/1.1 100 Continue\r\n\r\n"); err != nil { |
There was a problem hiding this comment.
When an HTTP/1.1 request has Expect: 100-continue but no body (for example, Content-Length: 0), this condition still emits an interim response. A server must not send 100 Continue unless the request indicates that content will follow, and clients that therefore did not enter a continue-wait state receive an unexpected extra response; require a non-empty/streaming request body before acknowledging and consuming the expectation.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
100-continueas a case-insensitive HTTP tokenExpectheader before forwarding the request upstream100 ContinueWhy
The plain HTTP path previously called
Request.Writebefore reading upstream responses. WithExpect: 100-continue, the client waited for the proxy while the proxy waited for the client body, even if the origin had already emitted the interim response.Validation
The regression test reproduces the waiting client, verifies the request body reaches the origin, and verifies the consumed expectation is not forwarded.