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.

Sign up for GitHub

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

Jump to bottom

x/net/http2: Client doesn't send body until ExpectContinueTimeout expires #49677

Closed
mhr3 opened this issue Nov 19, 2021 · 6 comments
Closed

x/net/http2: Client doesn't send body until ExpectContinueTimeout expires #49677

mhr3 opened this issue Nov 19, 2021 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.18

Comments

@mhr3
Copy link

mhr3 commented Nov 19, 2021

What version of Go are you using (go version)?

$ go version
go version go1.17.1 linux/amd64

Does this issue reproduce with the latest release?

This is a problem with x/net master

What did you do?

Set a non-zero timeout for ExpectContinueTimeout on http.Transport and sent a POST request with Expect: 100-continue over h2.

What did you expect to see?

The request uploads the data and finishes.

What did you see instead?

The data doesn't start uploading until after the specified timeout expires, adding extra latency.
@mhr3 mhr3 mentioned this issue Nov 19, 2021
Closed
@heschi heschi added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 22, 2021
@heschi heschi added this to the Go1.18 milestone Nov 22, 2021
@heschi
Copy link
Contributor

heschi commented Nov 22, 2021

cc @neild

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/363914 mentions this issue: net/http2: Fix handling of expect continue

@neild
Copy link
Contributor

neild commented Dec 1, 2021

@gopherbot Please open backport issues for 1.16 and 1.17. This is a regression.

@gopherbot gopherbot mentioned this issue Dec 1, 2021
Closed
@gopherbot
Copy link
Contributor

Backport issue(s) opened: #49904 (for 1.16), #49905 (for 1.17).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gopherbot gopherbot mentioned this issue Dec 1, 2021
Closed
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/368474 mentions this issue: [internal-branch.go1.16-vendor] net/http2: Fix handling of expect continue

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/368475 mentions this issue: [internal-branch.go1.17-vendor] net/http2: Fix handling of expect continue

gopherbot pushed a commit to golang/net that referenced this issue Dec 1, 2021
@mhr3 @mknyszek
[internal-branch.go1.16-vendor] net/http2: Fix handling of expect con…
d8ae719 
…tinue

Recent refactoring in the http2 package broke handling of ExpectContinueTimeout, where the client was always waiting for the timeout to pass before sending the body.

For golang/go#49677
Fixes golang/go#49904

Change-Id: I565299e48313bb905b2655bd52084ea49ab742f3
GitHub-Last-Rev: 587b484
GitHub-Pull-Request: #118
Reviewed-on: https://go-review.googlesource.com/c/net/+/363914
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
(cherry picked from commit 0a0e4e1)
Reviewed-on: https://go-review.googlesource.com/c/net/+/368474
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
gopherbot pushed a commit to golang/net that referenced this issue Dec 1, 2021
@mhr3 @mknyszek
[internal-branch.go1.17-vendor] net/http2: Fix handling of expect con…
85e122b 
…tinue

Recent refactoring in the http2 package broke handling of ExpectContinueTimeout, where the client was always waiting for the timeout to pass before sending the body.

For golang/go#49677
Fixes golang/go#49905

Change-Id: I565299e48313bb905b2655bd52084ea49ab742f3
GitHub-Last-Rev: 587b484
GitHub-Pull-Request: #118
Reviewed-on: https://go-review.googlesource.com/c/net/+/363914
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
(cherry picked from commit 0a0e4e1)
Reviewed-on: https://go-review.googlesource.com/c/net/+/368475
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
dteh pushed a commit to dteh/fhttp that referenced this issue Jun 22, 2022
@mhr3 @neild
net/http2: Fix handling of expect continue
7fca4b0 
Recent refactoring in the http2 package broke handling of ExpectContinueTimeout, where the client was always waiting for the timeout to pass before sending the body.

Fixes: golang/go#49677

Change-Id: I565299e48313bb905b2655bd52084ea49ab742f3
GitHub-Last-Rev: 587b48407932d44b26399b6c0b4ac228e6889ac8
GitHub-Pull-Request: golang/net#118
Reviewed-on: https://go-review.googlesource.com/c/net/+/363914
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
fedosgad pushed a commit to fedosgad/oohttp that referenced this issue Jun 22, 2022
@mhr3 @mknyszek
[internal-branch.go1.17-vendor] net/http2: Fix handling of expect con…
322f395 
…tinue

Recent refactoring in the http2 package broke handling of ExpectContinueTimeout, where the client was always waiting for the timeout to pass before sending the body.

For golang/go#49677
Fixes golang/go#49905

Change-Id: I565299e48313bb905b2655bd52084ea49ab742f3
GitHub-Last-Rev: 587b48407932d44b26399b6c0b4ac228e6889ac8
GitHub-Pull-Request: golang/net#118
Reviewed-on: https://go-review.googlesource.com/c/net/+/363914
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
(cherry picked from commit 0a0e4e1bb54c236434124a1fa7b0a400de92f2cc)
Reviewed-on: https://go-review.googlesource.com/c/net/+/368475
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Dec 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.18
Development

Successfully merging a pull request may close this issue.

net/http2: Fix handling of expect continue mhr3/net
4 participants
@neild @mhr3 @gopherbot @heschi