What was happening on Issue 7010 was handler intentionally took 30
milliseconds and the proxy's client timeout was 35 milliseconds. Then it
slammed the proxy with a bunch of requests.
Sometimes the server would be too slow to respond in its 5 millisecond
window and the client code would cancel the request, force-closing the
persistConn. If this came at the right time, the server's reply was
already in flight, and one of the goroutines would report:
Unsolicited response received on idle HTTP channel starting with "H"; err=<nil>
... rightfully scaring the user.
But the error was already handled and returned to the user, and this
connection knows it's been shut down. So look at the closed flag after
acquiring the same mutex guarding another field we were checking, and
don't complain if it's a known shutdown.
Also move closed down below the mutex which guards it.
Fixes#7010
LGTM=dsymonds
R=golang-codereviews, dsymonds
CC=adg, golang-codereviews, rsc
https://golang.org/cl/86740044
The test fails now with -race, so it's disabled.
The intention is that the fix for issue 7264
will also modify this test the same way and enable it.
Reporduce with 'go test -race -issue7264 -cpu=4'.
Update #7264
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/86770043
Add test for multipart form requests with an invalid content-type to ensure
ErrNotMultipart is returned.
Change ParseMultipartForm to return ErrNotMultipart when it is returned by multipartReader.
Modify test for empty multipart request handling to use POST so that the body is checked.
Fixes#6334.
This is the first changeset working on multipart request handling. Further changesets
could add more tests and clean up the TODO.
LGTM=bradfitz
R=golang-codereviews, gobot, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/44040043
I was implementing rules from RFC 2616. The rules are apparently useless,
ambiguous, and too strict for common software on the Internet. (e.g. curl)
Add more tests, including a test of a chunked request.
Fixes#7625
LGTM=dsymonds
R=golang-codereviews, dsymonds
CC=adg, golang-codereviews, rsc
https://golang.org/cl/84480045
The Go HTTP server doesn't use Response.Write, but others do,
so make it correct. Add a bunch more tests.
This bug is almost a year old. :/
Fixes#5381
LGTM=adg
R=golang-codereviews, adg
CC=dsymonds, golang-codereviews, rsc
https://golang.org/cl/85740046
Go's had pretty decent HTTP Trailer support for a long time, but
the docs have been largely non-existent. Fix that.
In the process, re-learn the Trailer code, clean some stuff
up, add some error checks, remove some TODOs, fix a minor bug
or two, and add tests.
LGTM=adg
R=golang-codereviews, adg
CC=dsymonds, golang-codereviews, rsc
https://golang.org/cl/86660043
In a typical HTTP request, the client writes the request, and
then the server replies. Go's HTTP client code (Transport) has
two goroutines per connection: one writing, and one reading. A
third goroutine (the one initiating the HTTP request)
coordinates with those two.
Because most HTTP requests are done when the server replies,
the Go code has always handled connection reuse purely in the
readLoop goroutine.
But if a client is writing a large request and the server
replies before it's consumed the entire request (e.g. it
replied with a 403 Forbidden and had no use for the body), it
was possible for Go to re-select that connection for a
subsequent request before we were done writing the first. That
wasn't actually a data race; the second HTTP request would
just get enqueued to write its request on the writeLoop. But
because the previous writeLoop didn't finish writing (and
might not ever), that connection is in a weird state. We
really just don't want to get into a state where we're
re-using a connection when the server spoke out of turn.
This CL changes the readLoop goroutine to verify that the
writeLoop finished before returning the connection.
In the process, it also fixes a potential goroutine leak where
a connection could close but the recycling logic could be
blocked forever waiting for the client to read to EOF or
error. Now it also selects on the persistConn's close channel,
and the closer of that is no longer the readLoop (which was
dead locking in some cases before). It's now closed at the
same place the underlying net.Conn is closed. This likely fixes
or helps Issue 7620.
Also addressed some small cosmetic things in the process.
Update #7620Fixes#7569
LGTM=adg
R=golang-codereviews, adg
CC=dsymonds, golang-codereviews, rsc
https://golang.org/cl/86290043
Also makes ErrWriteToConnected more appropriate; it's used
not only UDPConn operations but UnixConn operations.
Update #4856
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/84800044
The prefix was not uniformly applied and is probably better
left off for using with OpError.
Update #4856
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/84660046
Takes advantage of CL 83740044, to optimize map[string] lookup
from []byte key.
Deletes code.
No conditional check for gccgo, since Ian plans to add this
to gccgo before GCC 4.10 (Go 1.3).
benchmark old ns/op new ns/op delta
BenchmarkReadMIMEHeader 6066 5086 -16.16%
benchmark old allocs new allocs delta
BenchmarkReadMIMEHeader 12 12 +0.00%
benchmark old bytes new bytes delta
BenchmarkReadMIMEHeader 1317 1317 +0.00%
Update #3512
LGTM=rsc
R=rsc, dave
CC=golang-codereviews, iant
https://golang.org/cl/84230043
If an error happens on a connection, server goroutine can call b.Logf
after benchmark finishes.
So join both client and server goroutines.
Update #7718
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/84750047
This is just testing the status quo, so that any future attempt
to change it will make the test break and redirect the person
making the change to look at issue 6027.
Fixes#6027.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/83930046
There is no way to call them from outside the net package.
They are used to implement UCPConn.ReadMsgUDP and similar.
LGTM=mikioh.mikioh
R=golang-codereviews, mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/83730044
DragonFlyBSD, FreeBSD 9 and beyond, NetBSD 6 and beyond, and
Solaris (illumos) support AF_UNIX+SOCK_SEQPACKET socket.
LGTM=dave
R=golang-codereviews, dave
CC=golang-codereviews
https://golang.org/cl/83390043
This CL tries to fill the gap between Linux and other Unix-like systems
in the same way UDPConn already did.
Fixes#7677.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/83330045
On DragonFly BSD, we adjust the ephemeral port range because
unlike other BSD systems its default ephemeral port range
doesn't conform to IANA recommendation as described in RFC 6355
and is pretty narrow.
On DragonFly BSD 3.6: default range [1024, 5000], high range [49152, 65535]
On FreeBSD 10: default range [10000, 65535], high range [49152, 65535]
On Linux 3.11: default range [32768, 61000]
Fixes#7541.
LGTM=iant
R=jsing, gobot, iant
CC=golang-codereviews
https://golang.org/cl/80610044
For now we strictly use IPV6_V6ONLY=1 for IPv6-only communications
and IPV6_V6ONLY=0 for both IPv4 and IPv6 communications. So let the
capability test do the same.
LGTM=iant
R=golang-codereviews, gobot, iant
CC=golang-codereviews
https://golang.org/cl/80140044
Some platform that implements inp_localgroup-like shared internet
protocol control block group looks a bit sensitive about transport
layer protocol's address:port reuse. Sometimes it rejects a TCP SYN
packet using TCP RST, and sometimes silence.
For now, until test case refactoring, we admit few Dial failures on
TestTCPConcurrentAccept as a workaround.
Update #7400
Update #7541
LGTM=jsing
R=jsing
CC=golang-codereviews
https://golang.org/cl/75920043
The previous fix CL 69340044 still leaves a possibility of it.
This CL prevents the kernel, especially DragonFly BSD, from
performing unpredictable asynchronous connection establishment
on stream-based transport layer protocol sockets.
Update #7541
Update #7474
LGTM=jsing
R=jsing
CC=golang-codereviews
https://golang.org/cl/75930043
Disable it until it's debugged so it doesn't hide other real
problems on Windows. The test was known to be unreliable
anyway (which is why it only needed 1 of 20 runs to pass), but
apparently it never passes on Windows. Figure out why later.
Update #7634
LGTM=alex.brainman
R=adg, alex.brainman
CC=golang-codereviews
https://golang.org/cl/80110043
This the second part of making persistent HTTPS connections to
certain servers (notably Amazon) robust.
See the story in part 1: https://golang.org/cl/76400046/
This is the http Transport change that notes whether our
net.Conn.Read has ever seen an EOF. If it has, then we use
that as an additional signal to not re-use that connection (in
addition to the HTTP response headers)
Fixes#3514
LGTM=rsc
R=agl, rsc
CC=golang-codereviews
https://golang.org/cl/79240044
See testing.FailNow for further information.
Also avoid nil pointer derefernce in TestTransportMaxPerHostIdleConns.
LGTM=dave
R=golang-codereviews, dave
CC=golang-codereviews
https://golang.org/cl/76470043
Request ID reuse is allowed by the FastCGI spec [1]. In particular nginx uses
the same request ID, 1, for all requests on a given connection. Because
serveRequest does not remove the request from conn.requests, this causes it to
treat the second request as a duplicate and drops the connection immediately
after beginRequest. This manifests with nginx option 'fastcgi_keep_conn on' as
the following message in nginx error log:
2014/03/17 01:39:13 [error] 730#0: *109 recv() failed (104: Connection reset by peer) while reading response header from upstream, client: x.x.x.x, server: example.org, request: "GET / HTTP/1.1", upstream: "fastcgi://127.0.0.1:9001", host: "example.org"
Because handleRecord and serveRequest run in different goroutines, access to
conn.requests must now be synchronized.
[1] http://www.fastcgi.com/drupal/node/6?q=node/22#S3.3
LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://golang.org/cl/76800043
Solaris doesn't have struct ip_mreqn, instead it uses struct ip_mreq
and struct group_req with struct sockaddr_storage.
Also fixes incorrect SockaddrDatalink.
Update #7399
LGTM=aram, iant
R=golang-codereviews, aram, gobot, iant
CC=golang-codereviews
https://golang.org/cl/73920043
gc does not report this as an error, but go/types does.
(I suspect that constructing a closure counts as a reference
to &all in gc's implementation).
This is not a tool bug, since the spec doesn't require
implementations to implement this check, but it does
illustrate that dialect variations are always a nuisance.
LGTM=rsc, bradfitz
R=bradfitz
CC=golang-codereviews, gri, rsc
https://golang.org/cl/73850043
Disable the "udp" to IPv6 unicast address on the loopback interface
test under DragonFly BSD. This currently returns a local address of
0.0.0.1, rather than an IPv6 address with zone identifier.
Update #7473
LGTM=mikioh.mikioh
R=golang-codereviews, mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/71500044
Performing multiple connect system calls on a non-blocking socket
under DragonFly BSD does not necessarily result in errors from earlier
connect calls being returned, particularly if we are connecting to
localhost. Instead, once netpoll tells us that the socket is ready,
get the SO_ERROR socket option to see if the connection succeeded
or failed.
Fixes#7474
LGTM=mikioh.mikioh
R=mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/69340044
the crypto/tls revision d3d43f270632 (CL 67010043, requiring ServerName or InsecureSkipVerify) breaks net/smtp,
since it seems impossible to do SMTP via TLS anymore. i've tried to fix this by simply using a tls.Config with
ServerName, instead of a nil *tls.Config. without this fix, doing SMTP with TLS results in error "tls: either
ServerName or InsecureSkipVerify must be specified in the tls.Config".
testing: the new method TestTlsClient(...) sets up a skeletal smtp server with tls capability, and test client
injects a "fake" certificate allowing tls to work on localhost; thus, the modification to SendMail(...) enabling
this.
Fixes#7437.
LGTM=bradfitz
R=golang-codereviews, josharian, bradfitz
CC=golang-codereviews
https://golang.org/cl/70380043
The network connection dies differently from the server's
perspective on (some) Windows when the client goes away. Match
on the common prefix (common to Unix and Windows) instead of
the network error part.
Fixes#7456
LGTM=josharian
R=golang-codereviews, josharian
CC=alex.brainman, golang-codereviews, iant
https://golang.org/cl/70010050
I have one machine where this 25 test run is flaky
and fails ("21 >= 21"), but 50 works everywhere.
LGTM=josharian
R=josharian
CC=golang-codereviews
https://golang.org/cl/67870053
