This change adds support for the splice system call on Linux,
for the purpose of optimizing (*TCPConn).ReadFrom by reducing
copies of data from and to userspace. It does so by creating a
temporary pipe and splicing data from the source connection to the
pipe, then from the pipe to the destination connection. The pipe
serves as an in-kernel buffer for the data transfer.
No new API is added to package net, but a new Splice function is
added to package internal/poll, because using splice requires help
from the network poller. Users of the net package should benefit
from the change transparently.
This change only enables the optimization if the Reader in ReadFrom
is a TCP connection. Since splice is a more general interface, it
could, in theory, also be enabled if the Reader were a unix socket,
or the read half of a pipe.
However, benchmarks show that enabling it for unix sockets is most
likely not a net performance gain. The tcp <- unix case is also
fairly unlikely to be used very much by users of package net.
Enabling the optimization for pipes is also problematic from an
implementation perspective, since package net cannot easily get at
the *poll.FD of an *os.File. A possible solution to this would be
to dup the pipe file descriptor, register the duped descriptor with
the network poller, and work on that *poll.FD instead of the original.
However, this seems too intrusive, so it has not been done. If there
was a clean way to do it, it would probably be worth doing, since
splicing from a pipe to a socket can be done directly.
Therefore, this patch only enables the optimization for what is likely
the most common use case: tcp <- tcp.
The following benchmark compares the performance of the previous
userspace genericReadFrom code path to the new optimized code path.
The sub-benchmarks represent chunk sizes used by the writer on the
other end of the Reader passed to ReadFrom.
benchmark old ns/op new ns/op delta
BenchmarkTCPReadFrom/1024-4 4727 4954 +4.80%
BenchmarkTCPReadFrom/2048-4 4389 4301 -2.01%
BenchmarkTCPReadFrom/4096-4 4606 4534 -1.56%
BenchmarkTCPReadFrom/8192-4 5219 4779 -8.43%
BenchmarkTCPReadFrom/16384-4 8708 8008 -8.04%
BenchmarkTCPReadFrom/32768-4 16349 14973 -8.42%
BenchmarkTCPReadFrom/65536-4 35246 27406 -22.24%
BenchmarkTCPReadFrom/131072-4 72920 52382 -28.17%
BenchmarkTCPReadFrom/262144-4 149311 95094 -36.31%
BenchmarkTCPReadFrom/524288-4 306704 181856 -40.71%
BenchmarkTCPReadFrom/1048576-4 674174 357406 -46.99%
benchmark old MB/s new MB/s speedup
BenchmarkTCPReadFrom/1024-4 216.62 206.69 0.95x
BenchmarkTCPReadFrom/2048-4 466.61 476.08 1.02x
BenchmarkTCPReadFrom/4096-4 889.09 903.31 1.02x
BenchmarkTCPReadFrom/8192-4 1569.40 1714.06 1.09x
BenchmarkTCPReadFrom/16384-4 1881.42 2045.84 1.09x
BenchmarkTCPReadFrom/32768-4 2004.18 2188.41 1.09x
BenchmarkTCPReadFrom/65536-4 1859.38 2391.25 1.29x
BenchmarkTCPReadFrom/131072-4 1797.46 2502.21 1.39x
BenchmarkTCPReadFrom/262144-4 1755.69 2756.68 1.57x
BenchmarkTCPReadFrom/524288-4 1709.42 2882.98 1.69x
BenchmarkTCPReadFrom/1048576-4 1555.35 2933.84 1.89x
Fixes#10948
Change-Id: I3ce27f21f7adda8b696afdc48a91149998ae16a5
Reviewed-on: https://go-review.googlesource.com/107715
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Change-Id: I6e1fa67dc9d4d151c90eb19a6f736e4daa7d4fb3
Reviewed-on: https://go-review.googlesource.com/107615
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
On Unix systems, the underlying socket is no longer forced into blocking
mode.
Fixes#24942
Change-Id: I3e0c503c72df0844e30a63af298691dedacd1f46
Reviewed-on: https://go-review.googlesource.com/108297
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Use the vendored ValidTrailerHeader function from x/net/http/httpguts to
check Trailer headers according to RFC 7230. The previous implementation
only omitted illegal Trailer headers defined in RFC 2616.
This CL adds x/net/http/httpguts from CL 104042 (git rev a35a21de97)
Fixes#23908
Change-Id: Ib2329a384040494093c18e209db9b62aaf86e921
Reviewed-on: https://go-review.googlesource.com/104075
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This changes rawResponse to install a logger before
Serve()ing and makes the log output available to
tests.
Updates #24831
Updates CL 89275
Change-Id: I0fb636a35b05959ca9978d5d8552f38b7cf8e8b5
Reviewed-on: https://go-review.googlesource.com/106756
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There are two flakes present on the dashboard for this test.
Change-Id: I4abec972586314fbafe7db5760b91afd7ae47fd3
Reviewed-on: https://go-review.googlesource.com/106980
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
RawRead assumes the callback will perform either (a) a blocking read
and always return true, (b) a blocking read with a SO_RCVTIMEO set
returning false on WSAETIMEDOUT, or (c) a non-blocking read
returning false on WSAEWOULDBLOCK. In the latter two cases, it uses
a 0-byte overlapped read for notifications from the IOCP runtime
when the socket becomes readable before trying again.
RawWrite assumes the callback will perform blocking write and will
always return true, and makes no effort to tie into the runtime loop.
Change-Id: Ib10074e9d502c040294f41a260e561e84208652f
Reviewed-on: https://go-review.googlesource.com/76391
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Fixes#24580
Change-Id: I7536aca1e90717283bd6a3bb4b1bab059b0cf720
Reviewed-on: https://go-review.googlesource.com/104677
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Move the test skip to use testenv.SkipFlaky and link to the Go issue.
Update #24826
Change-Id: I7a0ea3325ffcaa790b25f8cdc429fb52e96a41c7
Reviewed-on: https://go-review.googlesource.com/106636
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
TestServerDuplicateBackgroundRead has been causing crashes on the
netbsd-arm-bsiegert builder, with the system becoming completely
unresponsive (probably deadlocked). Skip this test while the crash
is under investigation.
Upstream bug report is http://gnats.netbsd.org/53173.
Change-Id: Ib48f19005cf2cbba8a27e75e689c2acb025d8870
Reviewed-on: https://go-review.googlesource.com/106295
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
The docs for ResponseWriter.Write say
// If the Header
// does not contain a Content-Type line, Write adds a Content-Type set
// to the result of passing the initial 512 bytes of written data to
// DetectContentType.
The header X-Content-Type-Options:nosniff is an explicit directive that
content-type should not be sniffed.
This changes the behavior of Response.WriteHeader so that, when
there is an X-Content-Type-Options:nosniff header, but there is
no Content-type header, the following happens:
1. A Content-type:application/octet-stream is added
2. A warning is logged via the server's logging mechanism.
Previously, a content-type would have been silently added based on
heuristic analysis of the first 512B which might allow a hosted
GIF like http://www.thinkfu.com/blog/gifjavascript-polyglots to be
categorized as JavaScript which might allow a CSP bypass, loading
as a script despite `Content-Security-Policy: script-src 'self' `.
----
https://fetch.spec.whatwg.org/#x-content-type-options-header
defines the X-Content-Type-Options header.
["Polyglots: Crossing Origins by Crossing Formats"](http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.905.2946&rep=rep1&type=pdf)
explains Polyglot attacks in more detail.
Change-Id: I2c8800d2e4b4d10d9e08a0e3e5b20334a75f03c0
Reviewed-on: https://go-review.googlesource.com/89275
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
FreeBSD/Dragonfly and Solaris have identical implementations of
sendFile. Keep one and adjust the comments accordingly.
Change-Id: I77b0f88a4816dd6e40f5cb33919c44606401ac6b
Reviewed-on: https://go-review.googlesource.com/104915
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Although these changes have no essential influence, I think this is a better point.
Change-Id: I571d3a14c948d2fd7bc9561f47f33e9e4c90683f
GitHub-Last-Rev: d8c5d18006
GitHub-Pull-Request: golang/go#24697
Reviewed-on: https://go-review.googlesource.com/104895
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The go/printer (and thus gofmt) uses a heuristic to determine
whether to break alignment between elements of an expression
list which is spread across multiple lines. The heuristic only
kicked in if the entry sizes (character length) was above a
certain threshold (20) and the ratio between the previous and
current entry size was above a certain value (4).
This heuristic worked reasonably most of the time, but also
led to unfortunate breaks in many cases where a single entry
was suddenly much smaller (or larger) then the previous one.
The behavior of gofmt was sufficiently mysterious in some of
these situations that many issues were filed against it.
The simplest solution to address this problem is to remove
the heuristic altogether and have a programmer introduce
empty lines to force different alignments if it improves
readability. The problem with that approach is that the
places where it really matters, very long tables with many
(hundreds, or more) entries, may be machine-generated and
not "post-processed" by a human (e.g., unicode/utf8/tables.go).
If a single one of those entries is overlong, the result
would be that the alignment would force all comments or
values in key:value pairs to be adjusted to that overlong
value, making the table hard to read (e.g., that entry may
not even be visible on screen and all other entries seem
spaced out too wide).
Instead, we opted for a slightly improved heuristic that
behaves much better for "normal", human-written code.
1) The threshold is increased from 20 to 40. This disables
the heuristic for many common cases yet even if the alignment
is not "ideal", 40 is not that many characters per line with
todays screens, making it very likely that the entire line
remains "visible" in an editor.
2) Changed the heuristic to not simply look at the size ratio
between current and previous line, but instead considering the
geometric mean of the sizes of the previous (aligned) lines.
This emphasizes the "overall picture" of the previous lines,
rather than a single one (which might be an outlier).
3) Changed the ratio from 4 to 2.5. Now that we ignore sizes
below 40, a ratio of 4 would mean that a new entry would have
to be 4 times bigger (160) or smaller (10) before alignment
would be broken. A ratio of 2.5 seems more sensible.
Applied updated gofmt to all of src and misc. Also tested
against several former issues that complained about this
and verified that the output for the given examples is
satisfactory (added respective test cases).
Some of the files changed because they were not gofmt-ed
in the first place.
For #644.
For #7335.
For #10392.
(and probably more related issues)
Fixes#22852.
Change-Id: I5e48b3d3b157a5cf2d649833b7297b33f43a6f6e
This reverts commit CL 103975 (a9b799a229).
Reason for revert: adds data race, breaks race builders, and Brad forgot
to run the Trybots.
Change-Id: Id227dad7069560dbb3ea978a1dcd77ce1979034e
Reviewed-on: https://go-review.googlesource.com/104015
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
An application using syscall.RawConn in a particular way must take
account of the operating system or platform-dependent behavior.
This change consolidates duplicate code and improves the test coverage
for applications that use socket options.
Change-Id: Ie42340ac5373875cf1fd9123df0e99a1e7ac280f
Reviewed-on: https://go-review.googlesource.com/95335
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Apply the same approach as in CL 102397.
Updates #24580
Change-Id: I65955f62a70807c87216519d03f3643a8f214dee
Reviewed-on: https://go-review.googlesource.com/103655
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Map the error returned when a dial is aborted from the context package
error to the internal net package error. For example, context.Canceled
errors map to errCanceled, and context.DeadlineExceeded errors map to
poll.ErrTimeout.
Fixes#23648
Change-Id: Idf9d3d08052d540740c0b054503aaed931dc5b1e
Reviewed-on: https://go-review.googlesource.com/103518
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
A very small number of old browsers consider content as HTML
even when it is explicitly stated in the Content-Type header
that it is not. If content served is based on user-supplied
input, then an XSS is possible. Introduce three mitigations:
+ Don't reflect user input in error strings
+ Set a Content-Disposition header when requesting a resource
that should never be displayed in a browser window
+ Set X-Content-Type-Options: nosniff on all responses
Change-Id: I81c9d6736e0439ebd1db99cd7fb701cc56d24805
Reviewed-on: https://go-review.googlesource.com/102318
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I grepped for "bytes.Buffer" and "buf.String" and mostly ignored test
files. I skipped a few on purpose and probably missed a few others,
but otherwise I think this should be most of them.
Updates #18990
Change-Id: I5a6ae4296b87b416d8da02d7bfaf981d8cc14774
Reviewed-on: https://go-review.googlesource.com/102479
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
RFC 8081 declares a top level font media type for all types of fonts.
Updating the mime types in sniffer to reflect the new changes.
Fixes#24524
Change-Id: Iba6cef4c5974e9930e14705720d42550ee87ba56
Reviewed-on: https://go-review.googlesource.com/102458
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The build dashboard is dotted with net test failures.
We cannot declare all builders to have flaky networks,
although all fundamentally do.
Instead, add a simple retry/backoff loop to the ones that
show up most commonly on the dashboard at this moment.
If this approach works well in practice, we can
incrementally apply it to other flaky net tests.
Change-Id: I69c1ca6ce5b347ad549c7eb18d0438373f6e2489
Reviewed-on: https://go-review.googlesource.com/102397
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
VerifyHostname is called by tls.Conn during Handshake and does not need to be called explicitly.
Change-Id: I22b7fa137e76bb4be3d0018813a571acfb882219
Reviewed-on: https://go-review.googlesource.com/98618
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This is optimization is only for IPv4. It allocates a result buffer and
writes the IPv4 octets as dotted decimal into it before converting
it to a string just once, reducing allocations.
Benchmark shows performance improvement:
name old time/op new time/op delta
IPString/IPv4-8 284ns ± 4% 144ns ± 6% -49.35% (p=0.000 n=19+17)
IPString/IPv6-8 1.34µs ± 5% 1.14µs ± 5% -14.37% (p=0.000 n=19+20)
name old alloc/op new alloc/op delta
IPString/IPv4-8 24.0B ± 0% 16.0B ± 0% -33.33% (p=0.000 n=20+20)
IPString/IPv6-8 232B ± 0% 224B ± 0% -3.45% (p=0.000 n=20+20)
name old allocs/op new allocs/op delta
IPString/IPv4-8 3.00 ± 0% 2.00 ± 0% -33.33% (p=0.000 n=20+20)
IPString/IPv6-8 12.0 ± 0% 11.0 ± 0% -8.33% (p=0.000 n=20+20)
Fixes#24306
Change-Id: I4e2d30d364e78183d55a42907d277744494b6df3
Reviewed-on: https://go-review.googlesource.com/99395
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Since that method uses 'mux.m', we need to lock the mutex to avoid data races.
Change-Id: I998448a6e482b5d6a1b24f3354bb824906e23172
GitHub-Last-Rev: 163a7d4942
GitHub-Pull-Request: golang/go#23994
Reviewed-on: https://go-review.googlesource.com/96575
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
The SOCK_CLOEXEC and SOCK_NONBLOCK flags to the socket syscall and the
accept4 syscall are supported since OpenBSD 5.7.
Follows CL 40895 and CL 94295
Change-Id: Icaf35ace2ef5e73279a70d4f1a9fbf3be9371e6c
Reviewed-on: https://go-review.googlesource.com/97196
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
There are a few places where the integer value is used.
Use the equivalent constants to aid with readability.
Change-Id: I023b1dbe605340544c056d0e0d9d6d5a7d7d0edc
GitHub-Last-Rev: c1c90bcd25
GitHub-Pull-Request: golang/go#24123
Reviewed-on: https://go-review.googlesource.com/96984
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Ian Lance Taylor
Andrei Tudor Călin
shogo-ma
Aman Gupta
Michael Fraenkel
Brad Fitzpatrick
Mikio Hara
David Url
Mike Samuel
Josh Bleecher Snyder
adrienpetel
Benny Siegert
Ryan Slade
Tobias Klauser
chanxuehong
Robert Griesemer
James Hartig
Gregory Man
Kunpei Sakai
Ben Burkert
Andrew Bonventre
Agniva De Sarker
Ross Light
Paul Querna
Ian Gudger
Vladimir Kuzmin
as
Damien Mathieu
unknown
OneOfOne