Skip to content

Commit

Permalink
net/http: use ASCII space trimming throughout
Browse files Browse the repository at this point in the history
Security hardening against HTTP request smuggling. Thank you to ZeddYu
for reporting this issue.

Change-Id: I98bd9f8ffe58360fc3bca9dc5d9a106773e55373
Reviewed-on: https://go-review.googlesource.com/c/go/+/231419
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
  • Loading branch information
FiloSottile committed May 6, 2020
1 parent d5734d4 commit 2189852
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 15 deletions.
5 changes: 3 additions & 2 deletions src/net/http/cgi/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"log"
"net"
"net/http"
"net/textproto"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -276,8 +277,8 @@ func (h *Handler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
continue
}
header, val := parts[0], parts[1]
header = strings.TrimSpace(header)
val = strings.TrimSpace(val)
header = textproto.TrimString(header)
val = textproto.TrimString(val)
switch {
case header == "Status":
if len(val) < 3 {
Expand Down
11 changes: 6 additions & 5 deletions src/net/http/cookie.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package http
import (
"log"
"net"
"net/textproto"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -60,11 +61,11 @@ func readSetCookies(h Header) []*Cookie {
}
cookies := make([]*Cookie, 0, cookieCount)
for _, line := range h["Set-Cookie"] {
parts := strings.Split(strings.TrimSpace(line), ";")
parts := strings.Split(textproto.TrimString(line), ";")
if len(parts) == 1 && parts[0] == "" {
continue
}
parts[0] = strings.TrimSpace(parts[0])
parts[0] = textproto.TrimString(parts[0])
j := strings.Index(parts[0], "=")
if j < 0 {
continue
Expand All @@ -83,7 +84,7 @@ func readSetCookies(h Header) []*Cookie {
Raw: line,
}
for i := 1; i < len(parts); i++ {
parts[i] = strings.TrimSpace(parts[i])
parts[i] = textproto.TrimString(parts[i])
if len(parts[i]) == 0 {
continue
}
Expand Down Expand Up @@ -242,7 +243,7 @@ func readCookies(h Header, filter string) []*Cookie {

cookies := make([]*Cookie, 0, len(lines)+strings.Count(lines[0], ";"))
for _, line := range lines {
line = strings.TrimSpace(line)
line = textproto.TrimString(line)

var part string
for len(line) > 0 { // continue since we have rest
Expand All @@ -251,7 +252,7 @@ func readCookies(h Header, filter string) []*Cookie {
} else {
part, line = line, ""
}
part = strings.TrimSpace(part)
part = textproto.TrimString(part)
if len(part) == 0 {
continue
}
Expand Down
4 changes: 2 additions & 2 deletions src/net/http/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -756,15 +756,15 @@ func parseRange(s string, size int64) ([]httpRange, error) {
var ranges []httpRange
noOverlap := false
for _, ra := range strings.Split(s[len(b):], ",") {
ra = strings.TrimSpace(ra)
ra = textproto.TrimString(ra)
if ra == "" {
continue
}
i := strings.Index(ra, "-")
if i < 0 {
return nil, errors.New("invalid range")
}
start, end := strings.TrimSpace(ra[:i]), strings.TrimSpace(ra[i+1:])
start, end := textproto.TrimString(ra[:i]), textproto.TrimString(ra[i+1:])
var r httpRange
if start == "" {
// If no start is specified, end specifies the
Expand Down
3 changes: 2 additions & 1 deletion src/net/http/httptest/recorder.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"io/ioutil"
"net/http"
"net/textproto"
"strconv"
"strings"

Expand Down Expand Up @@ -221,7 +222,7 @@ func (rw *ResponseRecorder) Result() *http.Response {
// This a modified version of same function found in net/http/transfer.go. This
// one just ignores an invalid header.
func parseContentLength(cl string) int64 {
cl = strings.TrimSpace(cl)
cl = textproto.TrimString(cl)
if cl == "" {
return -1
}
Expand Down
3 changes: 2 additions & 1 deletion src/net/http/httputil/reverseproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"log"
"net"
"net/http"
"net/textproto"
"net/url"
"strings"
"sync"
Expand Down Expand Up @@ -387,7 +388,7 @@ func shouldPanicOnCopyError(req *http.Request) bool {
func removeConnectionHeaders(h http.Header) {
for _, f := range h["Connection"] {
for _, sf := range strings.Split(f, ",") {
if sf = strings.TrimSpace(sf); sf != "" {
if sf = textproto.TrimString(sf); sf != "" {
h.Del(sf)
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/net/http/transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -660,9 +660,9 @@ func fixLength(isResponse bool, status int, requestMethod string, header Header,
// Content-Length headers if they differ in value.
// If there are dups of the value, remove the dups.
// See Issue 16490.
first := strings.TrimSpace(contentLens[0])
first := textproto.TrimString(contentLens[0])
for _, ct := range contentLens[1:] {
if first != strings.TrimSpace(ct) {
if first != textproto.TrimString(ct) {
return 0, fmt.Errorf("http: message cannot contain multiple Content-Length headers; got %q", contentLens)
}
}
Expand Down Expand Up @@ -701,7 +701,7 @@ func fixLength(isResponse bool, status int, requestMethod string, header Header,
// Logic based on Content-Length
var cl string
if len(contentLens) == 1 {
cl = strings.TrimSpace(contentLens[0])
cl = textproto.TrimString(contentLens[0])
}
if cl != "" {
n, err := parseContentLength(cl)
Expand Down Expand Up @@ -1032,7 +1032,7 @@ func (bl bodyLocked) Read(p []byte) (n int, err error) {
// parseContentLength trims whitespace from s and returns -1 if no value
// is set, or the value if it's >= 0.
func parseContentLength(cl string) (int64, error) {
cl = strings.TrimSpace(cl)
cl = textproto.TrimString(cl)
if cl == "" {
return -1, nil
}
Expand Down

0 comments on commit 2189852

Please sign in to comment.