-
Notifications
You must be signed in to change notification settings - Fork 18k
net/http: disable 100 continue status after handler finished #66594
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
Conversation
This PR (HEAD: aac8503) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/575196. Important tips:
|
Message from Alexander Yastrebov: Patch Set 1: (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
Message from Alexander Yastrebov: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
aac8503
to
4a48705
Compare
This PR (HEAD: 4a48705) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/575196. Important tips:
|
Message from Alexander Yastrebov: Patch Set 2: (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
4a48705
to
dfcd406
Compare
This PR (HEAD: dfcd406) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/575196. Important tips:
|
Message from Damien Neil: Patch Set 3: Commit-Queue+1 (5 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
Message from Go LUCI: Patch Set 3: Dry run: CV is trying the patch. Bot data: {"action":"start","triggered_at":"2024-04-01T17:48:53Z","revision":"5fc80acd5889257d70d63a661a1dc954e9418864"} Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
Message from Damien Neil: Patch Set 3: -Commit-Queue Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
Message from Go LUCI: Patch Set 3: This CL has passed the run Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
Message from Go LUCI: Patch Set 3: LUCI-TryBot-Result+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
dfcd406
to
06c5d8b
Compare
This PR (HEAD: 06c5d8b) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/575196. Important tips:
|
Message from Alexander Yastrebov: Patch Set 4: (5 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
Message from Alexander Yastrebov: Patch Set 4: (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
152f674
to
42e2fb9
Compare
This PR (HEAD: 42e2fb9) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/575196. Important tips:
|
42e2fb9
to
92f8074
Compare
Message from Alexander Yastrebov: Patch Set 5: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
This PR (HEAD: 92f8074) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/575196. Important tips:
|
Message from Alexander Yastrebov: Patch Set 6: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
When client supplies "Expect: 100-continue" header, server wraps request body into expectContinueReader that writes 100 Continue status on the first body read. When handler acts as a reverse proxy and passes incoming request (or body) to the client (or transport) it may happen that request body is read after handler exists which may cause nil pointer panic on connection write if server already closed the connection. This change disables write of 100 Continue status by expectContinueReader after handler finished and before connection is closed. It also fixes racy access to w.wroteContinue. Fixes golang#53808 Updates golang#46866
92f8074
to
8b75f9f
Compare
This PR (HEAD: 8b75f9f) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/575196. Important tips:
|
Message from Damien Neil: Patch Set 7: (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
Message from Alexander Yastrebov: Patch Set 7: (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
Message from Damien Neil: Patch Set 7: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/575196. |
The issue was fixed by https://go.dev/cl/585395 |
When client supplies "Expect: 100-continue" header,
server wraps request body into expectContinueReader
that writes 100 Continue status on the first body read.
When handler acts as a reverse proxy and passes incoming
request (or body) to the client (or transport) it may happen
that request body is read after handler exists which may
cause nil pointer panic on connection write if server
already closed the connection.
This change disables write of 100 Continue status by expectContinueReader
after handler finished and before connection is closed.
It also fixes racy access to w.wroteContinue.
Fixes #53808
Updates #46866