| Age | Commit message (Collapse) | Author |
|---|
|
Also fix a (minor) double-Close error in Serve that was exposed by the
test fix.
Serve accepts a net.Listener, which produces net.Conn instances.
The documentation for net.Conn requires its methods to be safe for
concurrent use, so most implementations likely allow Close to be
called multiple times as a side effect of making it safe to call
concurrently with other methods. However, the net.Conn interface is a
superset of the io.Closer interface, io.Closer explicitly leaves the
behavior of multiple Close calls undefined, and net.Conn does not
explicitly document a stricter requirement.
Perhaps more importantly, the test for the fcgi package calls
unexported functions that accept an io.ReadWriteCloser (not a
net.Conn), and at least one of the test-helper ReadWriteCloser
implementations expects Close to be called only once.
The goroutine leaks were exposed by a racy arbitrary timeout reported
in #61271. Fixing the goroutine leak exposed the double-Close error:
one of the leaked goroutines was blocked on reading from an unclosed
pipe. Closing the pipe (to unblock the goroutine) triggered the second
Close call.
Fixes #61271.
Change-Id: I5cfac8870e4bb4f13adeee48910d165dbd4b76fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/508815
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
It has been agreed that we should prefer the US spelling of words like
"canceling" over "cancelling"; for example, see https://go.dev/cl/14526.
Fix a few occurrences of the "canceling" inconsistency, as well as:
* signaling
* tunneling
* marshaling
Change-Id: I99f3ba0a700a9f0292bc6c1b110af31dd05f1ff0
Reviewed-on: https://go-review.googlesource.com/c/go/+/398734
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
There was a race condition that could lead to child.serveRequest
removing the request ID before child.handleRequest had read the empty
FCGI_STDIN message that indicates end-of-stream which in turn could
lead to child.serveRequest blocking while trying to consume the
request body.
Now, we remove the request ID from within child.handleRequest after
the end of stdin has been detected, eliminating the race condition.
Since there are no more concurrent modifications/accesses
to child.requests, we remove the accompanying sync.Mutex.
Change-Id: I80c68e65904a988dfa9e3cceec1829496628ff34
GitHub-Last-Rev: b3976111ae1d3bbbfa36045f99acce7911a18c44
GitHub-Pull-Request: golang/go#42840
Reviewed-on: https://go-review.googlesource.com/c/go/+/273366
Trust: Damien Neil <dneil@google.com>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
|
|
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: b06d8377fdada075775d79a20577d38a7c471b45
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>
|
|
Preparation was in CL 134570043.
This CL contains only the effect of 'hg mv src/pkg/* src'.
For more about the move, see golang.org/s/go14nopkg.
|
