aboutsummaryrefslogtreecommitdiff
path: root/src/net/textproto/reader.go
AgeCommit message (Collapse)Author
2023-02-14mime/multipart: limit memory/inode consumption of ReadFormDamien Neil
Reader.ReadForm is documented as storing "up to maxMemory bytes + 10MB" in memory. Parsed forms can consume substantially more memory than this limit, since ReadForm does not account for map entry overhead and MIME headers. In addition, while the amount of disk memory consumed by ReadForm can be constrained by limiting the size of the parsed input, ReadForm will create one temporary file per form part stored on disk, potentially consuming a large number of inodes. Update ReadForm's memory accounting to include part names, MIME headers, and map entry overhead. Update ReadForm to store all on-disk file parts in a single temporary file. Files returned by FileHeader.Open are documented as having a concrete type of *os.File when a file is stored on disk. The change to use a single temporary file for all parts means that this is no longer the case when a form contains more than a single file part stored on disk. The previous behavior of storing each file part in a separate disk file may be reenabled with GODEBUG=multipartfiles=distinct. Update Reader.NextPart and Reader.NextRawPart to set a 10MiB cap on the size of MIME headers. Thanks to Jakob Ackermann (@das7pad) for reporting this issue. Fixes #58006 Fixes CVE-2022-41725 Change-Id: Ibd780a6c4c83ac8bcfd3cbe344f042e9940f2eab Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1714276 Reviewed-by: Julie Qiu <julieqiu@google.com> TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <bracewell@google.com> Run-TryBot: Damien Neil <dneil@google.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/468124 Auto-Submit: Michael Pratt <mpratt@google.com> Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Than McIntosh <thanm@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-11-08net/textproto: reject invalid header keys/values in ReadMIMEHeaderDamien Neil
Return an error when parsing a MIME header containing bytes in the key or value outside the set allowed by RFC 7230. For historical compatibility, accept spaces in keys (but do not canonicalize the key in this case). For #53188. Change-Id: I195319362a2fc69c4e506644f78c5026db070379 Reviewed-on: https://go-review.googlesource.com/c/go/+/410714 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Damien Neil <dneil@google.com>
2022-09-28net/textproto: use bytes.Clonecuiweixie
Change-Id: Ic73d667a98df3f2d1705a67e7e8625c6ba65cc0a Reviewed-on: https://go-review.googlesource.com/c/go/+/435284 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
2022-04-11all: gofmt main repoRuss Cox
[This CL is part of a sequence implementing the proposal #51082. The design doc is at https://go.dev/s/godocfmt-design.] Run the updated gofmt, which reformats doc comments, on the main repository. Vendored files are excluded. For #51082. Change-Id: I7332f099b60f716295fb34719c98c04eb1a85407 Reviewed-on: https://go-review.googlesource.com/c/go/+/384268 Reviewed-by: Jonathan Amsterdam <jba@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2022-04-08net/textproto: initialize commonHeader in canonicalMIMEHeaderKeyJohan Jansson
Call initCommonHeader in canonicalMIMEHeaderKey to ensure that commonHeader is initialized before use. Remove all other calls to initCommonHeader, since commonHeader is only used in canonicalMIMEHeaderKey. This prevents a race condition: read of commonHeader before commonHeader has been initialized. Add regression test that triggers the race condition which can be detected by the race detector. Fixes #46363 Change-Id: I00c8c52c6f4c78c0305978c876142c1b388174af Reviewed-on: https://go-review.googlesource.com/c/go/+/397575 Trust: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Trust: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-01all: remove trailing blank doc comment linesRuss Cox
A future change to gofmt will rewrite // Doc comment. // func f() to // Doc comment. func f() Apply that change preemptively to all doc comments. For #51082. Change-Id: I4023e16cfb0729b64a8590f071cd92f17343081d Reviewed-on: https://go-review.googlesource.com/c/go/+/384259 Trust: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
2021-10-06all: use bytes.Cut, strings.CutRuss Cox
Many uses of Index/IndexByte/IndexRune/Split/SplitN can be written more clearly using the new Cut functions. Do that. Also rewrite to other functions if that's clearer. For #46336. Change-Id: I68d024716ace41a57a8bf74455c62279bde0f448 Reviewed-on: https://go-review.googlesource.com/c/go/+/351711 Trust: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-10-20all: update references to symbols moved from io/ioutil to ioRuss Cox
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>
2020-07-09net/textproto: correct documentation of empty line handlingNorman B. Lancaster
Fixes #32493 Change-Id: I9c93791c4cc5c0c14556802733066407de3181ca Reviewed-on: https://go-review.googlesource.com/c/go/+/185542 TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-03-01net/textproto: pass missing argument to fmt.SprintfIan Lance Taylor
The vet tool didn't catch this because the fmt.Sprintf format argument was written as an expression. Fixes #37467 Change-Id: I72c20ba45e3f42c195fa5e68adcdb9837c7d7ad5 Reviewed-on: https://go-review.googlesource.com/c/go/+/221297 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-10-08net/textproto: do not allow multi-line header field namesKatie Hockman
Fixes #34702 Change-Id: I98320d54726e646a310e583283ddab676c3503e7 Reviewed-on: https://go-review.googlesource.com/c/go/+/199838 Run-TryBot: Katie Hockman <katie@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-09-26net/textproto: don't normalize headers with spaces before the colonFilippo Valsorda
RFC 7230 is clear about headers with a space before the colon, like X-Answer : 42 being invalid, but we've been accepting and normalizing them for compatibility purposes since CL 5690059 in 2012. On the client side, this is harmless and indeed most browsers behave the same to this day. On the server side, this becomes a security issue when the behavior doesn't match that of a reverse proxy sitting in front of the server. For example, if a WAF accepts them without normalizing them, it might be possible to bypass its filters, because the Go server would interpret the header differently. Worse, if the reverse proxy coalesces requests onto a single HTTP/1.1 connection to a Go server, the understanding of the request boundaries can get out of sync between them, allowing an attacker to tack an arbitrary method and path onto a request by other clients, including authentication headers unknown to the attacker. This was recently presented at multiple security conferences: https://portswigger.net/blog/http-desync-attacks-request-smuggling-reborn net/http servers already reject header keys with invalid characters. Simply stop normalizing extra spaces in net/textproto, let it return them unchanged like it does for other invalid headers, and let net/http enforce RFC 7230, which is HTTP specific. This loses us normalization on the client side, but there's no right answer on the client side anyway, and hiding the issue sounds worse than letting the application decide. Fixes CVE-2019-16276 Fixes #34540 Change-Id: I6d272de827e0870da85d93df770d6a0e161bbcf1 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/549719 Reviewed-by: Brad Fitzpatrick <bradfitz@google.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/197503 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2019-03-31cmd/go: further reduce init workDaniel Martí
The first biggest offender was crypto/des.init at ~1%. It's cryptographically broken and the init function is relatively expensive, which is unfortunate as both crypto/tls and crypto/x509 (and by extension, cmd/go) import it. Hide the work behind sync.Once. The second biggest offender was flag.sortFlags at just under 1%, used by the Visit flagset methods. It allocated two slices, which made a difference as cmd/go iterates over multiple flagsets during init. Use a single slice with a direct sort.Interface implementation. Another big offender is initializing global maps. Reducing this work in cmd/go/internal/imports and net/textproto gives us close to another whole 1% in saved work. The former can use map literals, and the latter can hide the work behind sync.Once. Finally, compress/flate used newHuffmanBitWriter as part of init, which allocates many objects and slices. Yet it only used one of the slice fields. Allocating just that slice saves a surprising ~0.3%, since we generated a lot of unnecessary garbage. All in all, these little pieces amount to just over 3% saved CPU time. name old time/op new time/op delta ExecGoEnv-8 3.61ms ± 1% 3.50ms ± 0% -3.02% (p=0.000 n=10+10) Updates #26775. Updates #29382. Change-Id: I915416e88a874c63235ba512617c8aef35c0ca8b Reviewed-on: https://go-review.googlesource.com/c/go/+/166459 Run-TryBot: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-11-01net/textproto: do not buffer a line if we know the next line is emptyTravis Bischel
readContinuedLineSlice intends to buffer a continued line of text, where a continued line can continue through newlines so long as the next line begins with a space or tab. The current optimization is to not try to buffer and build a line if we immediately see that the next line begins with an ASCII character. This adds avoiding copying the line if we see that the next line is \n or \r\n as well. Notably, headers always end in \r\n\r\n. In the general, well formatted header case, we can now avoid ever allocating textproto.Reader's internal reusable buf. This can mildly be seen in net/http's BenchmarkClientServer: name old time/op new time/op delta ClientServer-4 66.4µs ± 0% 66.2µs ± 0% -0.35% (p=0.004 n=10+10) name old alloc/op new alloc/op delta ClientServer-4 4.87kB ± 0% 4.82kB ± 0% -1.01% (p=0.000 n=6+10) name old allocs/op new allocs/op delta ClientServer-4 64.0 ± 0% 63.0 ± 0% -1.56% (p=0.000 n=10+10) Change-Id: Id8c2ab69086ac481b90abda289396dcb7bfe8851 Reviewed-on: https://go-review.googlesource.com/c/134227 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-06-01all: update comment URLs from HTTP to HTTPS, where possibleTim Cooper
Each URL was manually verified to ensure it did not serve up incorrect content. Change-Id: I4dc846227af95a73ee9a3074d0c379ff0fa955df Reviewed-on: https://go-review.googlesource.com/115798 Reviewed-by: Ian Lance Taylor <iant@golang.org> Run-TryBot: Ian Lance Taylor <iant@golang.org>
2017-11-27net/textproto: reject all headers with a leading spaceTom Bergan
Previously, golang.org/cl/75350 updated ReadMIMEHeader to ignore the first header line when it begins with a leading space, as in the following example: GET / HTTP/1.1 Host: foo.com Accept-Encoding: gzip However, golang.org/cl/75350 changed ReadMIMEHeader's behavior for the following example: before the CL it returned an error, but after the CL it ignored the first line. GET / HTTP/1.1 Host foo.com Accept-Encoding: gzip This change updates ReadMIMEHeader to always fail when the first header line starts with a space. During the discussion for golang.org/cl/75350, we realized we had three competing needs: 1. HTTP clients should accept malformed response headers when possible (ignoring the malformed lines). 2. HTTP servers should reject all malformed request headers. 3. The net/textproto package is used by multiple protocols (most notably, HTTP and SMTP) which have slightly different parsing semantics. This complicates changes to net/textproto. We weren't sure how to best fix net/textproto without an API change, but it is too late for API changes in Go 1.10. We decided to ignore initial lines that begin with spaces, thinking that would have the least impact on existing users -- malformed headers would continue to parse, but the initial lines would be ignored. Instead, golang.org/cl/75350 actually changed ReadMIMEHeader to succeed in cases where it previously failed (as in the above example). Reconsidering the above two examples, there does not seem to be a good argument to silently ignore ` Host: foo.com` but fail on ` Host foo.com`. Hence, this change fails for *all* headers where the initial line begins with a space. Updates #22464 Change-Id: I68d3d190489c350b0bc1549735bf6593fe11a94c Reviewed-on: https://go-review.googlesource.com/80055 Run-TryBot: Tom Bergan <tombergan@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-11-10net/textproto: ignore initial lines with leading whitespaces in ReadMIMEHeaderWèi Cōngruì
A header line with leading whitespaces is not valid in HTTP as per RFC7230. This change ignores these invalid lines in ReadMIMEHeader. Updates #22464 Change-Id: Iff9f00380d28a9617a55ff7888a76fba82001402 Reviewed-on: https://go-review.googlesource.com/75350 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-03-02all: single space after period.Brad Fitzpatrick
The tree's pretty inconsistent about single space vs double space after a period in documentation. Make it consistently a single space, per earlier decisions. This means contributors won't be confused by misleading precedence. This CL doesn't use go/doc to parse. It only addresses // comments. It was generated with: $ perl -i -npe 's,^(\s*// .+[a-z]\.) +([A-Z]),$1 $2,' $(git grep -l -E '^\s*//(.+\.) +([A-Z])') $ go test go/doc -update Change-Id: Iccdb99c37c797ef1f804a94b22ba5ee4b500c4f7 Reviewed-on: https://go-review.googlesource.com/20022 Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Dave Day <djd@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-03-01all: make copyright headers consistent with one space after periodBrad Fitzpatrick
This is a subset of https://golang.org/cl/20022 with only the copyright header lines, so the next CL will be smaller and more reviewable. Go policy has been single space after periods in comments for some time. The copyright header template at: https://golang.org/doc/contribute.html#copyright also uses a single space. Make them all consistent. Change-Id: Icc26c6b8495c3820da6b171ca96a74701b4a01b0 Reviewed-on: https://go-review.googlesource.com/20111 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-02-25net/textproto: permit all valid token chars in CanonicalMIMEHeaderKey inputBrad Fitzpatrick
Fixes #13767 Change-Id: Ib743db7d9d72022ea911bc5ac535243489425642 Reviewed-on: https://go-review.googlesource.com/18725 Reviewed-by: Andrew Gerrand <adg@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-01-08net/textproto: accept multi-line error messagesDaniel Speichert
Ads documentation for both formats of messages accepted by ReadResponse(). Validity of message should not be altered by the validation process. On message with unexpected code, a properly formatted message was not fully read. Fixes #10230 Change-Id: Ic0b473059a68ab624ce0525e359d0f5d0b8d2117 Reviewed-on: https://go-review.googlesource.com/18172 Reviewed-by: Russ Cox <rsc@golang.org>
2015-10-11net/textproto: properly trim continued lines in MIME headersDidier Spezia
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>
2015-06-30net/textproto: don't treat spaces as hyphens in header keysBrad Fitzpatrick
This was originally done in https://codereview.appspot.com/5690059 (Feb 2012) to deal with bad response headers coming back from webcams, but it presents a potential security problem with HTTP request smuggling for request headers containing "Content Length" instead of "Content-Length". Part of overall HTTP hardening for request smuggling. See RFC 7230. Thanks to Régis Leroy for the report. Change-Id: I92b17fb637c9171c5774ea1437979ae2c17ca88a Reviewed-on: https://go-review.googlesource.com/11772 Reviewed-by: Russ Cox <rsc@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2015-06-22net/textproto: skip zero-length keysJeff R. Allen
A header of ": value" results in an empty key. Do not add it to the headers, because RFC7230 (section 3.2) says that field-names are tokens, which are one or more characters. Fixes #11205. Change-Id: I883be89da1489dc84f98523786b019d1d0169d46 Reviewed-on: https://go-review.googlesource.com/11242 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2014-12-16net/textproto: turn an ancient DoS BUG annotation into a commentBrad Fitzpatrick
Actually fixing this "bug" would be weird, since io.LimitReader already does what we need, as demonstrated by net/http's use. Thanks to @davidfstr for pointing this out. Change-Id: If707bcc698d1666a369b39ddfa9770685fbe3879 Reviewed-on: https://go-review.googlesource.com/1579 Reviewed-by: Rob Pike <r@golang.org>
2014-09-08build: move package sources from src/pkg to srcRuss Cox
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.
generated by cgit v1.3 (git 2.53.0) at 2026-04-15 04:42:16 +0000