It was generating the wrong error message, always defaulting to "500
Internal Server Error", since the err variable used was always nil.
Fixes#12991
Change-Id: I94b0e516409c131ff3b878bcb91e65f0259ff077
Reviewed-on: https://go-review.googlesource.com/16060
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Current http client doesn't support Expect: 100-continue request
header(RFC2616-8/RFC7231-5.1.1). So even if the client have the header,
the head of the request body is consumed prematurely.
Those are my intentions to avoid premature consuming body in this change.
- If http.Request header contains body and Expect: 100-continue
header, it blocks sending body until it gets the first response.
- If the first status code to the request were 100, the request
starts sending body. Otherwise, sending body will be cancelled.
- Tranport.ExpectContinueTimeout specifies the amount of the time to
wait for the first response.
Fixes#3665
Change-Id: I4c04f7d88573b08cabd146c4e822061764a7cd1f
Reviewed-on: https://go-review.googlesource.com/10091
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fixes#7782Fixes#9554
Updates #7237 (original metabug, before we switched to specific bugs)
Updates #11932 (plan9 still doesn't have net I/O deadline support)
Change-Id: I96f311b88b1501d884ebc008fd31ad2cf1e16d75
Reviewed-on: https://go-review.googlesource.com/15941
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In net/parse.go we reimplement bytes.IndexByte and strings.IndexByte,
However those are implemented in runtime/$GOARCH_asm.s.
Using versions from runtime should provide performance advantage,
and keep the same code together.
Change-Id: I6212184bdf6aa1f2c03ce26d4b63f5b379d8ed0c
Reviewed-on: https://go-review.googlesource.com/15953
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The clues to this were already there, but as a user I was still unsure.
Make this more explicit.
Change-Id: I68564f3498dcd4897772a303588f03a6b65f111d
Reviewed-on: https://go-review.googlesource.com/15172
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This enables HTTP/2 by default (for https only) if the user didn't
configure anything in their NPN/ALPN map. If they're using SPDY or an
alternate http2 or a newer http2 from x/net/http2, we do nothing
and don't use the standard library's vendored copy of x/net/http2.
Upstream remains golang.org/x/net/http2.
Update #6891
Change-Id: I69a8957a021a00ac353f9d7fdb9a40a5b69f2199
Reviewed-on: https://go-review.googlesource.com/15828
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The PROXY protocol is supported by several proxy servers such as haproxy
and Amazon ELB. This protocol allows services running behind a proxy to
learn the remote address of the actual client connecting to the proxy,
by including a single textual line at the beginning of the TCP
connection.
http://www.haproxy.org/download/1.5/doc/proxy-protocol.txt
There are several Go libraries for this protocol (such as
https://github.com/armon/go-proxyproto), which operate by wrapping a
net.Conn with an implementation whose RemoteAddr method reads the
protocol line before returning. This means that RemoteAddr is a blocking
call.
Before this change, http.Serve called RemoteAddr from the main Accepting
goroutine, not from the per-connection goroutine. This meant that it
would not Accept another connection until RemoteAddr returned, which is
not appropriate if RemoteAddr needs to do a blocking read from the
socket first.
Fixes#12943.
Change-Id: I1a242169e6e4aafd118b794e7c8ac45d0d573421
Reviewed-on: https://go-review.googlesource.com/15835
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Allow all CGI environment settings from the inherited set and default
inherited set to be overridden including PATH by Env.
Change-Id: Ief8d33247b879fa87a8bfd6416d4813116db98de
Reviewed-on: https://go-review.googlesource.com/14959
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
These were proposed in the RFC over three years ago, then proposed to
be added to Go in https://codereview.appspot.com/7678043/ 2 years and
7 months ago, and the spec hasn't been updated or retracted the whole
time.
Time to export them.
Of note, HTTP/2 uses code 431 (Request Header Fields Too Large).
Updates #12843
Change-Id: I78c2fed5fab9540a98e845ace73f21c430a48809
Reviewed-on: https://go-review.googlesource.com/15732
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
A MIME header can include values defined on several lines.
Only the first line of each value was trimmed.
Make sure all the lines are trimmed before being aggregated.
Fixes#11204
Change-Id: Id92f384044bc6c4ca836e5dba2081fe82c82dc85
Reviewed-on: https://go-review.googlesource.com/15683
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#12866
net/http.Client returns some errors wrapped in a *url.Error. To avoid
the requirement to unwrap these errors to determine if the cause was
temporary or a timeout, make *url.Error implement net.Error directly.
Change-Id: I1ba84ecc7ad5147a40f056ff1254e60290152408
Reviewed-on: https://go-review.googlesource.com/15672
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
The net package already has support for limited uses of the strconv
package. Despite this, a few uses of strconv have crept in over time.
Remove them and use the existing net support instead.
Change-Id: Icdb4bdaa8e1197f1119a96cddcf548ed4a551b74
Reviewed-on: https://go-review.googlesource.com/15400
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The existing serve() method returns a zero-length response body when
it encounters an error, which results in a blank page and no visible
error in browsers.
This change sends a response body explaining the error for display in browsers.
Fixes#12745
Change-Id: I9dc3b95ad88cb92c18ced51f6b52bd3b2c1b974c
Reviewed-on: https://go-review.googlesource.com/15018
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The native Go host resolver was behaving differently than libc
and the entries in the /etc/hosts were handled in a case sensitive
way. In order to be compatible with libc's resolver, /etc/hosts
lookups must be case-insensitive.
Fixes#12806.
Change-Id: I3c14001abffadf7458fd1a027c91e6438a87f285
Reviewed-on: https://go-review.googlesource.com/15321
Run-TryBot: Burcu Dogan <jbd@google.com>
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>
In the present code, there is no way for ok to ever return false, but
it still a good idea to check it.
Change-Id: I8f360018b33a5d85dabbbbec0f89ffc81f77ecbb
Reviewed-on: https://go-review.googlesource.com/13956
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
They added no value.
Change-Id: I9e690379d2dfd983266de0ea5231f2b57c8b1517
Reviewed-on: https://go-review.googlesource.com/14568
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When running an experimental kernel with IPv4 disabled, Listen(":port")
currently tries to create an AF_INET socket, and fails. Instead, it
should see !supportsIPv4, and use an AF_INET6 socket.
This sort of environment is quite esoteric at the moment, but I can
force the tests to fail on regular Linux using the following tweaks:
- net/net.go: supportsIPv4, supportsIPv6, supportsIPv4map = false, true, false
- net/sockopt_linux.go: ipv6only=true
- net/ipsock_posix.go: Revert this fix
- ./make.bash && ../bin/go test net
Also, make the arrows in server_test.go point to the left, because
server<-client is easier to read.
Fixes#12510
Change-Id: I0cc3b6b08d5e6908d2fbf8594f652ba19815aa4b
Reviewed-on: https://go-review.googlesource.com/14334
Run-TryBot: Paul Marks <pmarks@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Optimize two calls of io.Copy which cannot make use of neither
io.ReaderFrom nor io.WriterTo optimization tricks by replacing them with
io.CopyBuffer with reusable buffers.
First is fallback call to io.Copy when server misses the optimized case
of using sendfile to copy from a regular file to net.TCPConn; second is
use of io.Copy on piped reader/writer when handler implementation uses
http.CloseNotifier interface. One of the notable users of
http.CloseNotifier is httputil.ReverseProxy.
benchmark old ns/op new ns/op delta
BenchmarkCloseNotifier-4 309591 303388 -2.00%
benchmark old allocs new allocs delta
BenchmarkCloseNotifier-4 50 49 -2.00%
benchmark old bytes new bytes delta
BenchmarkCloseNotifier-4 36168 3140 -91.32%
Fixes#12455
Change-Id: I512e6aa2f1aeed2ed00246afb3350c819b65b87e
Reviewed-on: https://go-review.googlesource.com/14177
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The default implementation of Accept, which spins up a new server
for every new connection, calls log.Fatal if the listener is closed,
stopping any outstanding work. Change that to a non-fatal log
call so work can continue.
There is no programmatic signaling of the problem, just the log,
but that should be enough.
Fixes#11221.
Change-Id: I7c7f6164a0a0143236729eb778d7638c51c34ed1
Reviewed-on: https://go-review.googlesource.com/14185
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This may fix the flakiness on Windows/x64, assuming that it's actually
due to a variance in the connection time which slightly exceeds 100ms.
150ms + 95ms = 245ms, which is still low enough to avoid triggering
Happy Eyeballs (300ms) on non-Windows platforms.
Updates #12309
Change-Id: I816a36fbc0a3e5c90e3cf1b75a134faf0d91557c
Reviewed-on: https://go-review.googlesource.com/14120
Run-TryBot: Paul Marks <pmarks@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This worked in Go 1.4 but was lost in the "pure Go" lookup
routines substituted late in the Go 1.5 cycle.
Fixes#12263.
Change-Id: I77ec9d97cd8e67ace99d6ac965e5bc16c151ba83
Reviewed-on: https://go-review.googlesource.com/13915
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Accepting a request with a nil body was never explicitly supported but
happened to work in the past.
This doesn't happen in most cases because usually people pass
a Server's incoming Request to the ReverseProxy's ServeHTTP method,
and incoming server requests are guaranteed to have non-nil bodies.
Still, it's a regression, so fix.
Fixes#12344
Change-Id: Id9a5a47aea3f2875d195b66c9a5f8581c4ca2aed
Reviewed-on: https://go-review.googlesource.com/13935
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
byte is unsigned so the comparison against zero is always true.
Change-Id: I8fa60245972be362ae920507a291f92c0f9831ad
Reviewed-on: https://go-review.googlesource.com/13941
Run-TryBot: Todd Neal <todd@tneal.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It is already validated by isDoaminName.
Change-Id: I7a955b632a5143e16b012641cf12bad452900753
Reviewed-on: https://go-review.googlesource.com/13789
Reviewed-by: Andrew Gerrand <adg@golang.org>
Could go in 1.5, although not critical.
See also #12107
Change-Id: I7f1608b58581d21df4db58f0db654fef79e33a90
Reviewed-on: https://go-review.googlesource.com/13481
Reviewed-by: Dave Cheney <dave@cheney.net>
This is especially important for LookupAddr, which used to be pure Go
(lightweight, one goroutine per call) and without this CL is now
unconditionally cgo (heavy, one thread per call).
Fixes#12190.
Change-Id: I43436a942bc1838b024225893e156f280a1e80cf
Reviewed-on: https://go-review.googlesource.com/13698
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Go 1.4 and before have always returned DNS names with a trailing dot
for reverse lookups, as they do for basically all other routines returning
DNS names. Go 1.4 and before always implemented LookupAddr using
pure Go (not C library calls).
Go 1.5 added the ability to make a C library call to implement LookupAddr.
Unfortunately the C library call returns a DNS name without a trailing dot
(an unrooted name), meaning that if turn off cgo during make.bash then
you still get the rooted name but with cgo on you get an unrooted name.
The unrooted name is inconsistent with the pure Go implementation
and with all previous Go releases, so change it to a rooted name.
Fixes#12189.
Change-Id: I3d6b72277c121fe085ea6af30e5fe8019fc490ad
Reviewed-on: https://go-review.googlesource.com/13697
Reviewed-by: Rob Pike <r@golang.org>
Found in a Google program running under the race detector.
No test, but verified that this fixes the race with go run -race of:
package main
import (
"crypto/tls"
"fmt"
"net"
"net/http"
"net/http/httptest"
)
func main() {
for {
ts := httptest.NewTLSServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {}))
conf := &tls.Config{} // non-nil
a, b := net.Pipe()
go func() {
sconn := tls.Server(a, conf)
sconn.Handshake()
}()
tr := &http.Transport{
TLSClientConfig: conf,
}
req, _ := http.NewRequest("GET", ts.URL, nil)
_, err := tr.RoundTrip(req)
println(fmt.Sprint(err))
a.Close()
b.Close()
ts.Close()
}
}
Also modified cmd/vet to report the copy-of-mutex bug statically
in CL 13646, and fixed two other instances in the code found by vet.
But vet could not have told us about cloneTLSConfig vs cloneTLSClientConfig.
Confirmed that original report is also fixed by this.
Fixes#12099.
Change-Id: Iba0171549e01852a5ec3438c25a1951c98524dec
Reviewed-on: https://go-review.googlesource.com/13453
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
The old code was only allowing the chars we choose not to escape.
We sometimes prefer to escape chars that do not strictly need it.
Allowing those to be used in RawPath lets people override that
preference, which is in fact the whole point of RawPath (new in Go 1.5).
While we are here, also allow [ ] in RawPath.
This is not strictly spec-compliant, but it is what modern browers
do and what at least some people expect, and the [ ] do not cause
any ambiguity (the usual reason they would be escaped, as they are
part of the RFC gen-delims class).
The argument for allowing them now instead of waiting until Go 1.6
is that this way RawPath has one fixed meaning at the time it is
introduced, that we should not need to change or expand.
Fixes#5684.
Change-Id: If9c82a18f522d7ee1d10310a22821ada9286ee5c
Reviewed-on: https://go-review.googlesource.com/13258
Reviewed-by: Andrew Gerrand <adg@golang.org>
The code in question was added as part of allowing zone identifiers
in IPv6 literals like http://[ipv6%zone]:port/foo, in golang.org/cl/2431.
The old condition makes no sense. It refers to §3.2.1, which is the wrong section
of the RFC, it excludes all the sub-delims, which §3.2.2 (the right section)
makes clear are valid, and it allows ':', which is not actually valid,
without an explanation as to why (because we keep :port in the Host field
of the URL struct).
The new condition allows all the sub-delims, as specified in RFC 3986,
plus the additional characters [ ] : seen in IP address literals and :port suffixes,
which we also keep in the Host field.
This allows mysql://a,b,c/path to continue to parse, as it did in Go 1.4 and earlier.
This CL does not break any existing tests, suggesting the over-conservative
behavior was not intended and perhaps not realized.
It is especially important not to over-escape the host field, because
Go does not unescape the host field during parsing: it rejects any
host field containing % characters.
Fixes#12036.
Change-Id: Iccbe4985957b3dc58b6dfb5dcb5b63a51a6feefb
Reviewed-on: https://go-review.googlesource.com/13254
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Go 1.4 and earlier accepted mysql://x@y(z:123)/foo
and I don't see any compelling reason to break that.
The CL during Go 1.5 that broke this syntax was
trying to fix#11208 and was probably too aggressive.
I added a test case for #11208 to make sure that stays
fixed.
Relaxing the check did not re-break #11208 nor did
it cause any existing test to fail. I added a test for the
mysql://x@y(z:123)/foo syntax being preserved.
Fixes#12023.
Change-Id: I659d39f18c85111697732ad24b757169d69284fc
Reviewed-on: https://go-review.googlesource.com/13253
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Prior to this change, broken trailers would be handled by body.Read, and
an error would be returned to its caller (likely a Handler), but that
error would go completely unnoticed by the rest of the server flow
allowing a broken connection to be reused. This is a possible request
smuggling vector.
Fixes#12027.
Change-Id: I077eb0b8dff35c5d5534ee5f6386127c9954bd58
Reviewed-on: https://go-review.googlesource.com/13148
Reviewed-by: Russ Cox <rsc@golang.org>
It was failing with multiple goroutines a few out of every thousand
runs (with errRequestCanceled) because it was using the same
*http.Request for all 5 RoundTrips, but the RoundTrips' goroutines
(notably the readLoop method) were all still running, sharing that
same pointer. Because the response has no body (which is what
TestZeroLengthPostAndResponse tests), the readLoop was marking the
connection as reusable early (before the caller read until the body's
EOF), but the Transport code was clearing the Request's cancelation
func *AFTER* the caller had already received it from RoundTrip. This
let the test continue looping and do the next request with the same
pointer, fetch a connection, and then between getConn and roundTrip
have an invariant violated: the Request's cancelation func was nil,
tripping this check:
if !pc.t.replaceReqCanceler(req.Request, pc.cancelRequest) {
pc.t.putIdleConn(pc)
return nil, errRequestCanceled
}
The solution is to clear the request cancelation func in the readLoop
goroutine in the no-body case before it's returned to the caller.
This now passes reliably:
$ go test -race -run=TestZeroLengthPostAndResponse -count=3000
I think we've only seen this recently because we now randomize scheduling
of goroutines in race mode (https://golang.org/cl/11795). This race
has existed for a long time but the window was hard to hit.
Change-Id: Idb91c582919f85aef5b9e5ef23706f1ba9126e9a
Reviewed-on: https://go-review.googlesource.com/13070
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Introduced in https://go-review.googlesource.com/12865 (git rev c2db5f4c).
This fix doesn't add any new lock acquistions: it just moves the
existing one taken by the unreadDataSize method and moves it out
wider.
It became flaky at rev c2db5f4c, but now reliably passes again:
$ go test -v -race -run=TestTransportAndServerSharedBodyRace -count=100 net/http
Fixes#11985
Change-Id: I6956d62839fd7c37e2f7441b1d425793f4a0db30
Reviewed-on: https://go-review.googlesource.com/12909
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Brad Fitzpatrick
Yasuharu Goto
Ilya Tocar
Aaron Jacobs
David Glasser
Steven Hartland
Didier Spezia
Dave Cheney
Ian Lance Taylor
Kenny Grant
Burcu Dogan
Tarmigan Casebolt
Rob Pike
Paul Marks
Dan Peterson
Artyom Pervukhin
Nodir Turakulov
Shenghou Ma
Russ Cox
Todd Neal
Matt Layher
Mikio Hara
Andrew Gerrand
David Symonds
Jed Denlea