aboutsummaryrefslogtreecommitdiff
path: root/src/net/http/serve_test.go
AgeCommit message (Collapse)Author
5 daysnet/http: run tests for HTTP/3 where it can already passNicholas S. Husin
By default, our test harnesses (run and runSynctest) now use http3Mode, in addition to http1Mode and http2Mode, when no []testMode were explicitly defined for a given test. Tests that cannot currently pass for HTTP/3 have been modified to use http3SkippedMode, which serves as a convenient alias for the old default of []testMode{http1Mode, http2Mode}. We changed the default mode and defined http3SkippedMode so we have a clear list of TODOs in terms of how much changes are still needed before our HTTP/3 implementation reaches basic feature parity with HTTP/1 and HTTP/2. For #70914 Change-Id: I719d5d66399a51f7c3d96180ebed9b606a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/765320 Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Nicholas Husin <husin@google.com>
2026-03-24net/http: add a test for starting a server with no HTTP/2 and no TLS configDamien Neil
Test for the fix in CL 758560. Change-Id: I34edf9f14dc5d6a569f20aa3d55d9d136a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/758661 Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Nicholas Husin <nsh@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Nicholas Husin <husin@google.com>
2026-03-23net/http: make ResponseWriter.ReadFrom respect declared Content-LengthNicholas S. Husin
Unlike ResponseWriter.Write, ResponseWriter.ReadFrom does not currently respect declared Content-Length header. As a result, it is possible for a server handler to inadvertently write more bytes for their response body than has been declared via Content-Length. These excess bytes are written to the wire, and could potentially be misinterpreted by a client as a separate response. That said, this is not a concern security-wise. This edge case can only be exercised by someone who has a relatively complete control of a server handler—by which point, worse things can be done with less effort. Regardless, this is still a bug. Therefore, make sure that ResponseWriter.ReadFrom respects declared Content-Length too for consistency. Fixes #78179 Change-Id: I469b064e43e49e467b907d23fc1ee879066569f0 Reviewed-on: https://go-review.googlesource.com/c/go/+/755701 Reviewed-by: Nicholas Husin <husin@google.com> Auto-Submit: Nicholas Husin <nsh@golang.org> Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-03-12net/http/internal/http2: remove ExportSetH2GoawayTimeoutDamien Neil
This was used by one test. Rewrite that test to use synctest (faster!) and not care about the timeout. For #67810 Change-Id: I61496e575ae8a16ff778470f3f9d711e6a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/751303 Reviewed-by: Nicholas Husin <husin@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Nicholas Husin <nsh@golang.org>
2025-11-21net/http: preserve original path encoding in redirectsSean Liao
Fixes #70758 Change-Id: I9fc6fe98c194351557c6219513918b7593899bc1 Reviewed-on: https://go-review.googlesource.com/c/go/+/720821 Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Mark Freeman <markfreeman@google.com>
2025-11-21net/http: use HTTP 307 redirects in ServeMuxSean Liao
Clients receiving an HTTP 301 Moved Permanently may conservatively change the method of a POST request to GET. The newer HTTP 307 Temporary Redirect and 308 Permanent Redirect explicitly allows retrying POST requests after the redirect. These should be safe for ServeMux as this internal redirect is generated before user provided handlers are called. As ServeMux is making the redirect for the user without explicit direction, and clients may cache Permanent Redirects indefinitely, Temporary Redirect is used in case the user adds a handler for a path, that was previously redirected but no longer should. Fixes #50243 Fixes #60769 Change-Id: I6c0b735bab03bb7b50f05457b3b8a8ba813badb2 Reviewed-on: https://go-review.googlesource.com/c/go/+/720820 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Mark Freeman <markfreeman@google.com>
2025-11-18Revert "net/http: do not discard body content when closing it within request ↵Nicholas S. Husin
handlers" This reverts commit cb0d9980f5721715ebb73dd2e580eaa11c2ddee2. Reason for revert: the old behavior seems to be relied on by current users, e.g. https://github.com/connectrpc/connect-go/blob/cb2e11fb88c9a61804043355a619c12d4a30a1a5/protocol_connect.go#L837. For #75933 Change-Id: I996280238e5c70a8d760a0b31e3a13c6a44b8616 Reviewed-on: https://go-review.googlesource.com/c/go/+/721761 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com> Auto-Submit: Nicholas Husin <nsh@golang.org> Reviewed-by: Nicholas Husin <husin@google.com>
2025-11-14net/http: do not discard body content when closing it within request handlersNicholas S. Husin
(*body).Close() internally tries to discard the content of a request body up to 256 KB. We rely on this behavior to allow connection re-use, by calling (*body).Close() when our request handler exits. Unfortunately, this causes an unfortunate side-effect where we would prematurely try to discard a body content when (*body).Close() is called from within a request handler. There should not be a good reason for (*body).Close() to do this when called from within a request handler. As such, this CL modifies (*body).Close() to not discard body contents when called from within a request handler. Note that when a request handler exits, it will still try to discard the body content for connection re-use. For #75933 Change-Id: I71d2431a540579184066dd35d3da49d6c85c3daf Reviewed-on: https://go-review.googlesource.com/c/go/+/720380 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Nicholas Husin <husin@google.com> Reviewed-by: Damien Neil <dneil@google.com>
2025-10-17all: remove unnecessary loop variable copies in testsTobias Klauser
Copying the loop variable is no longer necessary since Go 1.22. Change-Id: Iebb21dac44a20ec200567f1d786f105a4ee4999d Reviewed-on: https://go-review.googlesource.com/c/go/+/711640 Reviewed-by: Florian Lehner <lehner.florian86@gmail.com> Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Damien Neil <dneil@google.com> Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2025-09-06net/http: pool transport gzip readersAlexander Yastrebov
goos: linux goarch: amd64 pkg: net/http │ HEAD~1 │ HEAD │ │ sec/op │ sec/op vs base │ ClientGzip-8 621.0µ ± 2% 616.3µ ± 10% ~ (p=0.971 n=10) │ HEAD~1 │ HEAD │ │ B/op │ B/op vs base │ ClientGzip-8 49.765Ki ± 0% 9.514Ki ± 2% -80.88% (p=0.000 n=10) │ HEAD~1 │ HEAD │ │ allocs/op │ allocs/op vs base │ ClientGzip-8 57.00 ± 0% 52.00 ± 0% -8.77% (p=0.000 n=10) Allocation saving comes from absent compress/flate.(*dictDecoder).init This change also improves concurrent body read detection by returning an explicit error. Updates #61353 Change-Id: I380acfca912dc009b3b9c8283e27b3526cedd546 GitHub-Last-Rev: df12f6a48af4854ba686fe431a9aeb6d9ba3c303 GitHub-Pull-Request: golang/go#61390 Reviewed-on: https://go-review.googlesource.com/c/go/+/510255 Reviewed-by: Sean Liao <sean@liao.dev> Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Michael Pratt <mpratt@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
2025-05-21net/http: use synctest.Test rather than RunDamien Neil
Use the non-experimental Test function. As a bonus, this lets us drop the hacks we were doing to support t.Cleanup inside bubbles. Change-Id: I070624e1384494e9d5fcfee594cfbb7680c1beda Reviewed-on: https://go-review.googlesource.com/c/go/+/675315 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
2025-05-19net/http: fix ServeMux.Handler on trailing-slash redirectJonathan Amsterdam
When a match involves a trailing-slash redirect, ServeMux.Handler now returns the pattern that matched. Fixes #73688. Change-Id: I682d9cc9a3628bed8bf21139b98369ffa6c53792 Reviewed-on: https://go-review.googlesource.com/c/go/+/673815 Reviewed-by: Filippo Valsorda <filippo@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com>
2025-04-16net/http: set Request.TLS when net.Conn implements ConnectionStateWeidi Deng
Fixes #56104 Change-Id: I8fbbb00379e51323e2782144070cbcad650eb6f1 GitHub-Last-Rev: 62d7a8064e4f2173f0d8e02ed91a7e8de7f13fca GitHub-Pull-Request: golang/go#56110 Reviewed-on: https://go-review.googlesource.com/c/go/+/440795 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Sean Liao <sean@liao.dev> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
2025-04-09net/http: reduce memory usage when hijackingJakob Ackermann
Previously, Hijack allocated a new write buffer and the existing connection write buffer used an extra 4KiB of memory until the handler finished and the "conn" was garbage collected. Now, hijack re-uses the existing write buffer and re-attaches it to the raw connection to avoid referencing the net/http "conn" after returning. After a handler that hijacked exited, the "conn" reference in "connReader" will now be unset. This allows all of the "conn", "response" and "Request" to get garbage collected. Overall, this is reducing the memory usage by 43% or 6.7KiB per hijacked connection (see BenchmarkServerHijackMemoryUsage in an earlier revision of the CL). CloseNotify will continue to work _before_ the handler has exited (i.e. while the "conn" is still referenced in "connReader"). This aligns with the documentation of CloseNotifier: > After the Handler has returned, there is no guarantee that the channel > receives a value. goos: linux goarch: amd64 pkg: net/http cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz │ before │ after │ │ sec/op │ sec/op vs base │ ServerHijack-8 42.59µ ± 8% 39.47µ ± 16% ~ (p=0.481 n=10) │ before │ after │ │ B/op │ B/op vs base │ ServerHijack-8 16.12Ki ± 0% 12.06Ki ± 0% -25.16% (p=0.000 n=10) │ before │ after │ │ allocs/op │ allocs/op vs base │ ServerHijack-8 51.00 ± 0% 49.00 ± 0% -3.92% (p=0.000 n=10) Change-Id: I20a37ee314ed0d47463a4657d712154e78e48138 GitHub-Last-Rev: 80f09dfa273035f53cdd72845e5c5fb129c3e230 GitHub-Pull-Request: golang/go#70756 Reviewed-on: https://go-review.googlesource.com/c/go/+/634855 Reviewed-by: Sean Liao <sean@liao.dev> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com> Auto-Submit: Sean Liao <sean@liao.dev>
2025-03-04net/http: don't modify caller's tls.Config.NextProtosDamien Neil
Clone the input slice before adjusting NextProtos to add or remove "http/1.1" and "h2" entries, so as not to modify a slice that the caller might be using. (We clone the tls.Config that contains the slice, but that's a shallow clone.) Fixes #72100 Change-Id: I9f228b8fb6f6f2ca5023179ec114929c002dbda9 Reviewed-on: https://go-review.googlesource.com/c/go/+/654875 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Jonathan Amsterdam <jba@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2025-02-27net/http: reject newlines in chunk-size linesDamien Neil
Unlike request headers, where we are allowed to leniently accept a bare LF in place of a CRLF, chunked bodies must always use CRLF line terminators. We were already enforcing this for chunk-data lines; do so for chunk-size lines as well. Also reject bare CRs anywhere other than as part of the CRLF terminator. Fixes CVE-2025-22871 Fixes #71988 Change-Id: Ib0e21af5a8ba28c2a1ca52b72af8e2265ec79e4a Reviewed-on: https://go-review.googlesource.com/c/go/+/652998 Reviewed-by: Jonathan Amsterdam <jba@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2025-02-14net/http: unskip TestDisableContentLength/h2Dmitri Shuralyov
h2_bundle.go has been updated. Change-Id: I055b8db9aab964621c980e4731011c89f7694405 Reviewed-on: https://go-review.googlesource.com/c/go/+/649496 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
2025-02-10net/http: use standard time formatting methodsTom Thorogood
time.Time has had an AppendFormat method since go1.5 so there's no need to carry around a custom implementation. Change-Id: I8e7e5a9ac34e8bf251f5d70555405777ce4e22a8 Reviewed-on: https://go-review.googlesource.com/c/go/+/647955 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Damien Neil <dneil@google.com> Auto-Submit: Ian Lance Taylor <iant@golang.org>
2024-11-25net/http: run TestServerShutdownStateNew in a synctest bubbleDamien Neil
Took ~12s previously, ~0s now. Change-Id: I72580fbde73482a40142cf84cd3d78a50afb9f44 Reviewed-on: https://go-review.googlesource.com/c/go/+/630382 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
2024-07-29net/http: don't write HEAD response body in ResponseWriter.ReadFromDamien Neil
Responses to HEAD requests don't have a body. The ResponseWriter automatically discards writes to the response body when responding to a HEAD request. ResponseWriter.ReadFrom was failing to discard writes under some circumstances; fix it to do so. Fixes #68609 Change-Id: I912f6b2b2a535df28ae37b875fcf15b10da1af2b Reviewed-on: https://go-review.googlesource.com/c/go/+/601475 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Auto-Submit: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
2024-07-25net: use slices and maps to clean up testsapocelipes
Replace reflect.DeepEqual with slices.Equal/maps.Equal, which is much faster. Change-Id: I54600fb63a56460c11d3d5af9072da585e31b1a2 GitHub-Last-Rev: 08c1445ad5be94d071e8ceb4b060b8f4ab0d77ba GitHub-Pull-Request: golang/go#67606 Reviewed-on: https://go-review.googlesource.com/c/go/+/587816 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Ian Lance Taylor <iant@google.com>
2024-06-28net/http: avoid appending an existing trailing slash to path againJes Cok
This CL is similar to CL 562557, and it takes over CL 594175. While here, unrelatedly remove mapKeys function, use slices.Sorted(maps.Keys(ms)) to simplify code. Fixes #67657 Change-Id: Id8b99216f87a6dcfd6d5fa61407b515324c79112 Reviewed-on: https://go-review.googlesource.com/c/go/+/594737 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com> Reviewed-by: Joedian Reid <joedian@google.com>
2024-06-18net/http: keep Content-Encoding in Error, add GODEBUG for ServeContentDamien Neil
This reverts the changes to Error from CL 571995, and adds a GODEBUG controlling the changes to ServeContent/ServeFile/ServeFS. The change to remove the Content-Encoding header when serving an error breaks middleware which sets Content-Encoding: gzip and wraps a ResponseWriter in one which compresses the response body. This middleware already breaks when ServeContent handles a Range request. Correct uses of ServeContent which serve pre-compressed content with a Content-Encoding: gzip header break if we don't remove that header when serving errors. Therefore, we keep the change to ServeContent/ ServeFile/ServeFS, but we add the ability to disable the new behavior by setting GODEBUG=httpservecontentkeepheaders=1. We revert the change to Error, because users who don't want to include a Content-Encoding header in errors can simply remove the header themselves, or not add it in the first place. Fixes #66343 Change-Id: Ic19a24b73624a5ac1a258ed7a8fe7d9bf86c6a38 Reviewed-on: https://go-review.googlesource.com/c/go/+/593157 Reviewed-by: Russ Cox <rsc@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2024-06-07net/http: check GetConfigForClient in server.ServeTLSChance Zibolski
Just like for tls.Config.GetCertificate the http.Server.ServeTLS method should be checking tls.Config.GetConfigForClient before trying top open the specified certFile/keyFile. This was previously fixed for crypto/tls when using tls.Listen in CL205059, but the same change for net/http was missed. I've added a comment src/crypto/tls/tls.go in the relevant section in the hope that any future changes of a similar nature consider will consider updating net/http as needed as well. Change-Id: I312303bc497d92aa2f4627fe2620c70779cbcc99 GitHub-Last-Rev: 6ed29a900816a13690a9f3e26476d9bc1055a6f7 GitHub-Pull-Request: golang/go#66795 Reviewed-on: https://go-review.googlesource.com/c/go/+/578396 Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
2024-05-23std: fix calls to Printf(s) with non-constant sAlan Donovan
In all cases the intent was not to interpret s as a format string. In one case (go/types), this was a latent bug in production. (These were uncovered by a new check in vet's printf analyzer.) Updates #60529 Change-Id: I3e17af7e589be9aec1580783a1b1011c52ec494b Reviewed-on: https://go-review.googlesource.com/c/go/+/587855 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Russ Cox <rsc@golang.org>
2024-05-21net/http: disable flaky 100-continue testsDamien Neil
Disable three 100-continue tests that aren't exercising the intended behavior because they don't set ExpectContinueTimeout. The tests are flaky right now; setting ExpectContinueTimeout makes them consistently fail. Set ExpectContinueTimeout and t.Skip the tests for now. Fixes #67382 For #67555 Change-Id: I459a19a927e14af03881e89c73d20c93cf0da43e Reviewed-on: https://go-review.googlesource.com/c/go/+/587155 Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2024-05-14net/http: avoid panic when writing 100-continue after handler doneDamien Neil
When a request contains an "Expect: 100-continue" header, the first read from the request body causes the server to write a 100-continue status. This write caused a panic when performed after the server handler has exited. Disable the write when cleaning up after a handler exits. This also fixes a bug where an implicit 100-continue could be sent after a call to WriteHeader has sent a non-1xx header. This change drops tracking of whether we've written a 100-continue or not in response.wroteContinue. This tracking was used to determine whether we should consume the remaining request body in chunkWriter.writeHeader, but the discard-the-body path was only taken when the body was already consumed. (If the body is not consumed, we set closeAfterReply, and we don't consume the remaining body when closeAfterReply is set. If the body is consumed, then we may attempt to discard the remaining body, but there is obviously no body remaining.) Fixes #53808 Change-Id: I3542df26ad6cdfe93b50a45ae2d6e7ef031e46fa Reviewed-on: https://go-review.googlesource.com/c/go/+/585395 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
2024-05-09net/http: remove misleading response headers on errorRuss Cox
This is a reapply of CL 544019 and CL 569815, but with less aggressive semantics as discussed in proposal #66343. Error deletes Content-Encoding, since it is writing the response and any preset encoding may not be correct. On the error-serving path in ServeContent/ServeFile/ServeFS, these functions delete additional headers: Etag, Last-Modified, and Cache-Control. The caller may have set these intending them for the success response, and they may well not be correct for error responses. Fixes #50905. Fixes #66343. Change-Id: I873d33edde1805990ca16d85ea8d7735b7448626 Reviewed-on: https://go-review.googlesource.com/c/go/+/571995 Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2024-03-25net/http: eliminate the needless idle timeout for TestServerNoReadTimeoutAndy Pan
Change-Id: I1339749bfeac99848beca780cebb9c87564da656 Reviewed-on: https://go-review.googlesource.com/c/go/+/573335 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
2024-03-22net/http: ensure server handler is done in TestServerNoWriteTimeoutDamien Neil
Surprisingly, newClientServerTest doesn't ensure that server handlers are done in its t.Cleanup function. This test's handler can outlive the test and attempt to log after the test has completed, causing race detector failures. Add an explicit call to Server.Shutdown to ensure the handler has completed. We should also probably add a Shutdown to clientServerTest.close, but that's a larger change; this fixes the immediate problem. Change-Id: Ibe81b4b382c9c8a920b0ff5f76dea6afe69b10f5 Reviewed-on: https://go-review.googlesource.com/c/go/+/573895 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Auto-Submit: Damien Neil <dneil@google.com>
2024-03-21net/http: add tests with zero and negative read/write timeoutsAndy Pan
Change-Id: I38ebd280c200b30692eb35640327034a5e898bd0 Reviewed-on: https://go-review.googlesource.com/c/go/+/570376 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: David Chase <drchase@google.com> Auto-Submit: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2024-03-14Revert "net/http: remove superfluous newline on redirects"Russ Cox
This reverts commit 2b58355ef624239dbe32185dc8dfc9d1074615c6. Reason for revert: This breaks tons of tests for no real reason. Change-Id: I89773f48cf983c0b6346e46c37a0ebbe2620e3b4 Reviewed-on: https://go-review.googlesource.com/c/go/+/571675 Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2024-02-29net/http: remove Content-Length header in http.ErrorDamien Neil
Error replies to a request with an error message and HTTP code. Delete any preexisting Content-Length header before writing the header; if a Content-Length is present, it's probably for content that the caller has given up on writing. For #50905 Change-Id: Ia3d4ca008be46fa5d41afadf29ca5cacb1c47660 Reviewed-on: https://go-review.googlesource.com/c/go/+/554216 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
2024-02-26net/http: reject client-side retries in server timeout testsBryan C. Mills
This breaks an unbounded client-side retry loop if the server's timeout happens to fire during its final read of the TLS handshake. The retry loop was observed on wasm platforms at CL 557437. I was also able to reproduce chains of dozens of retries on my linux/amd64 workstation by adjusting some timeouts and adding a couple of sleeps, as in this patch: https://gist.github.com/bcmills/d0a0a57e5f64eebc24e8211d8ea502b3 However, on linux/amd64 on my workstation the test always eventually breaks out of the retry loop due to timing jitter. I couldn't find a retry-specific hook in the http.Client, http.Transport, or tls.Config structs, so I have instead abused the Transport.Proxy hook for this purpose. Separately, we may want to consider adding a retry-specific hook, or changing the net/http implementation to avoid transparently retrying in this case. Fixes #65410. Updates #65178. Change-Id: I0e43c039615fe815f0a4ba99a8813c48b1fdc7e6 Reviewed-on: https://go-review.googlesource.com/c/go/+/559835 Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
2024-02-14net/http: reject requests with invalid Content-Length headersAndy Pan
According to RFC 9110 and RFC 9112, invalid "Content-Length" headers might involve request smuggling or response splitting, which could also cause security failures. Currently, `net/http` ignores all "Content-Length" headers when there is a "Transfer-Encoding" header and forward the message anyway while other mainstream HTTP implementations such as Apache Tomcat, Nginx, HAProxy, Node.js, Deno, Tornado, etc. reject invalid Content-Length headers regardless of the presence of a "Transfer-Encoding" header and only forward chunked-encoding messages with either valid "Content-Length" headers or no "Content-Length" headers. Fixes #65505 Change-Id: I73af2ee0785137e56c7546a4cce4a5c5c348dbc5 Reviewed-on: https://go-review.googlesource.com/c/go/+/561075 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Damien Neil <dneil@google.com>
2024-02-13net/http: refine trailing-slash redirect logicJonathan Amsterdam
Do not add a trailing slash and redirect if the path already ends in a slash. Also, and unrelatedly, add a test for cleanPath. Fixes #65624. Change-Id: Ifcf9edc929d2eb6db88132c09d2bade85c5dda3d Reviewed-on: https://go-review.googlesource.com/c/go/+/562557 Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2024-02-12net/http: remove superfluous newline on redirectscodesoap
Change-Id: I30d3ae9d540f9cc85ea5a6875ee8884d3e646d6f GitHub-Last-Rev: 29cabdcb3a8746ef51953617f4ec47deac3608da GitHub-Pull-Request: golang/go#65623 Reviewed-on: https://go-review.googlesource.com/c/go/+/562356 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
2024-01-30net/textproto: reject HTTP requests with empty header keysAndy Pan
According to RFC 7230, empty field names in HTTP header are invalid. However, there are no specific instructions for developers to deal with that kind of case in the specification. CL 11242 chose to skip it and do nothing about it, which now seems like a bad idea because it has led `net/http` to behave inconsistently with the most widely-used HTTP implementations: Apache, Nginx, Node with llhttp, H2O, Lighttpd, etc. in the case of empty header keys. There is a very small chance that this CL will break a few existing HTTP clients. Fixes #65244 Change-Id: Ie01e9a6693d27caea4d81d1539345cf42b225535 Reviewed-on: https://go-review.googlesource.com/c/go/+/558095 Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2023-11-02net/http: remove arbitrary timeouts in tests of Server.ErrorLogBryan C. Mills
This also allows us to remove the chanWriter helper from the test, using a simpler strings.Builder instead, relying on clientServerTest.close for synchronization. (I don't think this runs afoul of #38370, because the handler functions themselves in these tests should never be executed, let alone result in an asynchronous write to the error log.) Fixes #57599. Change-Id: I45c6cefca0bb218f6f9a9659de6bde454547f704 Reviewed-on: https://go-review.googlesource.com/c/go/+/539436 Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2023-09-27net/http: add extra synchronization for a Logf call in ↵Bryan C. Mills
TestTransportAndServerSharedBodyRace This race was reported in https://build.golang.org/log/6f043170946b665edb85b50804a62db68348c52f. As best as I can tell, it is another instance of #38370. The deferred call to backend.close() ought to be enough to ensure that the t.Logf happens before the end of the test, but in practice it is not, and with enough scheduling delay we can manage to trip the race detector on a call to Logf after the test function has returned. Updates #38370. Change-Id: I5ee45df45c6bfad3239d665df65a138f1c4573a3 Reviewed-on: https://go-review.googlesource.com/c/go/+/531195 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Damien Neil <dneil@google.com>
2023-09-20net/http: eliminate more clientServerTest leaks in tests that use ↵Bryan C. Mills
runTimeSensitiveTest Change-Id: I77684a095af03d5c4e50da8e7af210b10639ff23 Reviewed-on: https://go-review.googlesource.com/c/go/+/529756 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Damien Neil <dneil@google.com>
2023-09-19net/http: buffer the testConn close channel in ↵Bryan C. Mills
TestHandlerFinishSkipBigContentLengthRead Previously the test used an unbuffered channel, but testConn.Close sends to it with a select-with-default, so the send would be dropped if the test goroutine happened not to have parked on the receive yet. To make this kind of bug less likely in future tests, use a newTestConn helper function instead of constructing testConn channel literals in each test individually. Fixes #62622. Change-Id: I016cd0a89cf8a2a748ed57a4cdbd01a178f04dab Reviewed-on: https://go-review.googlesource.com/c/go/+/529475 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Damien Neil <dneil@google.com>
2023-09-15net/http: avoid leaking goroutines when TestServerGracefulClose retriesBryan C. Mills
If the call to ReadString returns an error, the closure in testServerGracefulClose will return an error and retry the test with a longer timeout. If that happens, we need to wait for the conn.Write goroutine to complete so that we don't leak connections across tests. Updates #57084. Fixes #62643. Change-Id: Ia86c1bbd0a5e5d0aeccf4dfeb994c19d1fb10b00 Reviewed-on: https://go-review.googlesource.com/c/go/+/528398 Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Than McIntosh <thanm@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Damien Neil <dneil@google.com>
2023-09-14net/http: synchronize tests that use reqNum countersBryan C. Mills
This suppresses the race reported in #62638. I am not 100% certain how that race happens, but here is my theory. The increment of reqNum happens before the server writes the response headers, and the server necessarily writes the headers before the client receives them. However, that write/read pair occurs through I/O syscalls rather than Go synchronization primitives, so it doesn't necessarily create a “happens before” relationship as defined by the Go memory model: although we can establish a sequence of events, that sequence is not visible to the race detector, nor to the compiler. Fixes #62638. Change-Id: I90d66ec3fc32b9b8e1f9bbf0bc2eb289b964b99b Reviewed-on: https://go-review.googlesource.com/c/go/+/528475 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com> Auto-Submit: Bryan Mills <bcmills@google.com>
2023-09-13net/http: scale rstAvoidanceDelay to reduce test flakinessBryan C. Mills
As far as I can tell, some flakiness is unavoidable in tests that race a large client request write against a server's response when the server doesn't read the full request. It does not appear to be possible to simultaneously ensure that well-behaved clients see EOF instead of ECONNRESET and also prevent misbehaving clients from consuming arbitrary server resources. (See RFC 7230 §6.6 for more detail.) Since there doesn't appear to be a way to cleanly eliminate this source of flakiness, we can instead work around it: we can allow the test to adjust the hard-coded delay if it sees a plausibly-related failure, so that the test can retry with a longer delay. As a nice side benefit, this also allows the tests to run more quickly in the typical case: since the test will retry in case of spurious failures, we can start with an aggressively short delay, and only back off to a longer one if it is really needed on the specific machine running the test. Fixes #57084. Fixes #51104. For #58398. Change-Id: Ia4050679f0777e5eeba7670307a77d93cfce856f Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest-race,gotip-linux-amd64-race,gotip-windows-amd64-race Reviewed-on: https://go-review.googlesource.com/c/go/+/527196 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com> Auto-Submit: Bryan Mills <bcmills@google.com>
2023-09-05all: use ^$ instead of XXXX, NoSuchTestExists to match no testsDmitri Shuralyov
It's shorter and can't accidentally match unlikely test names. Change-Id: I96dd9da018cad1acf604f266819470278f54c128 Reviewed-on: https://go-review.googlesource.com/c/go/+/524949 Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2023-09-05all: use ^TestName$ regular pattern for invoking a single testDmitri Shuralyov
Use ^ and $ in the -run flag regular expression value when the intention is to invoke a single named test. This removes the reliance on there not being another similarly named test to achieve the intended result. In particular, package syscall has tests named TestUnshareMountNameSpace and TestUnshareMountNameSpaceChroot that both trigger themselves setting GO_WANT_HELPER_PROCESS=1 to run alternate code in a helper process. As a consequence of overlap in their test names, the former was inadvertently triggering one too many helpers. Spotted while reviewing CL 525196. Apply the same change in other places to make it easier for code readers to see that said tests aren't running extraneous tests. The unlikely cases of -run=TestSomething intentionally being used to run all tests that have the TestSomething substring in the name can be better written as -run=^.*TestSomething.*$ or with a comment so it is clear it wasn't an oversight. Change-Id: Iba208aba3998acdbf8c6708e5d23ab88938bfc1e Reviewed-on: https://go-review.googlesource.com/c/go/+/524948 Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Kirill Kolyshkin <kolyshkin@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2023-08-25net/http: deflake TestRequestBodyLimitDamien Neil
This test can return with a Transport still processing an in-flight request, resulting in a test failure due to the leaked Transport. Avoid this by waiting for the Transport to close the request body before returning. Fixes #60264 Change-Id: I8d8b54f633c2e28da2b1bf1bc01ce09dd77769de Reviewed-on: https://go-review.googlesource.com/c/go/+/522695 Reviewed-by: Bryan Mills <bcmills@google.com> Auto-Submit: Damien Neil <dneil@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Damien Neil <dneil@google.com>
2023-08-22net/http: use testenv.Command instead of exec.Command in testsBryan C. Mills
On Unix platforms, testenv.Command sends SIGQUIT to stuck commands before the test times out. For subprocesses that are written in Go, that causes the runtime to dump running goroutines, and in other languages it triggers similar behavior (such as a core dump). If the subprocess is stuck due to a bug (such as #57999), that may help to diagnose it. For #57999. Change-Id: Ia2e9d14718a26001e030e162c69892497a8ebb21 Reviewed-on: https://go-review.googlesource.com/c/go/+/521816 Reviewed-by: Damien Neil <dneil@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com>
2023-06-07net/http: close response body in TestRequestBodyLimitDamien Neil
Failing to close the response body before returning leaks the in-progress request past the test lifetime. Fixes #60264 Change-Id: Ic327d9f8e02e87ed656324aaa042f833d9ea18ca Reviewed-on: https://go-review.googlesource.com/c/go/+/501309 Run-TryBot: Damien Neil <dneil@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>