The race reported in issue #41167 was detected only because the
ReadWriter used in test code happened to be a bytes.Buffer whose
Read and Write operate (unsafely) on shared state. This is not the
case in any realistic scenario where the FastCGI protocol is spoken
over sockets or pairs of pipes.
Since tests that use nopWriteCloser don't care about any output
generate by child.Serve(), we change nopWriteCloser to provide
a dummy Write method.
Remove the locking added in CL 252417, since it causes a deadlock
during write as reported in #43901. The race in tests no longer
happens thanks to the aforementioned change to nopWriteCloser.
Fixes#43901.
Updates #41167.
Change-Id: I8cf31088a71253c34056698f8e2ad0bee9fcf6c6
GitHub-Last-Rev: b06d8377fd
GitHub-Pull-Request: golang/go#43027
Reviewed-on: https://go-review.googlesource.com/c/go/+/275692
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
The old ioutil references are still valid, but update our code
to reflect best practices and get used to the new locations.
Code compiled with the bootstrap toolchain
(cmd/asm, cmd/dist, cmd/compile, debug/elf)
must remain Go 1.4-compatible and is excluded.
Also excluded vendored code.
For #41190.
Change-Id: I6d86f2bf7bc37a9d904b6cee3fe0c7af6d94d5b1
Reviewed-on: https://go-review.googlesource.com/c/go/+/263142
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This CL ensures that responses served via CGI and FastCGI
have a Content-Type header based on the content of the
response if not explicitly set by handlers.
If the implementers of the handler did not explicitly
specify a Content-Type both CGI implementations would default
to "text/html", potentially causing cross-site scripting.
Thanks to RedTeam Pentesting GmbH for reporting this.
Fixes#40928
Fixes CVE-2020-24553
Change-Id: I82cfc396309b5ab2e8d6e9a87eda8ea7e3799473
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/823217
Reviewed-by: Russ Cox <rsc@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/252179
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
The current interface can't access all environment
variables directly or via cgi.RequestFromMap, which
only reads variables on its "white list" to be set on
the http.Request it returns. If an fcgi variable is
not on the "white list" - e.g. REMOTE_USER - the old
code has no access to its value.
This passes variables in the Request context that aren't
used to add data to the Request itself and adds a method
that parses those env vars from the Request's context.
Fixes#16546
Change-Id: Ibf933a768b677ece1bb93d7bf99a14cef36ec671
Reviewed-on: https://go-review.googlesource.com/40012
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
As stated in FastCGI specifications:
FastCGI transmits a name-value pair as the length of the name,
followed by the length of the value, followed by the name,
followed by the value.
The current implementation trusts the name and value length
provided in the record, leading to a panic if the record
is malformed.
Added an explicit check on the lengths.
Test case and fix suggested by diogin@gmail.com (Jingcheng Zhang)
Fixes#11824
Change-Id: I883a1982ea46465e1fb02e0e02b6a4df9e529ae4
Reviewed-on: https://go-review.googlesource.com/15015
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently the test fails if run more than once:
$ go test -v -run=TestChildServeCleansUp -cpu=1,1 net/http/fcgi
=== RUN TestChildServeCleansUp
--- PASS: TestChildServeCleansUp (0.00s)
=== RUN TestChildServeCleansUp
fatal error: all goroutines are asleep - deadlock!
The problem is that the writer mutates test input data,
so it is wrong on the second execution.
Change-Id: I4ca54dd2926c6986b2908023ac65e5e65630ed26
Reviewed-on: https://go-review.googlesource.com/6383
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Close the pipe for the body of a request when it is aborted and close
all pipes when child.serve terminates.
Fixes#6934
Change-Id: I1c5e7d2116e1ff106f11a1ef8e99bf70cf04162a
Reviewed-on: https://go-review.googlesource.com/1923
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Hilko Bengen
Russ Cox
Roberto Clapis
Meir Fischer
Didier Spezia
Dmitry Vyukov
Evan Kroske