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

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

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.

@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

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
Copy link

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
Copy link

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

@gopherbot
Copy link

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
…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
…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
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
…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.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants