Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http: extra space between response version and statuscode causes error: malformed HTTP status code "" #19989

Closed
andybalholm opened this issue Apr 15, 2017 · 10 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@andybalholm
Copy link
Contributor

andybalholm commented Apr 15, 2017

When the status line of an HTTP response has two spaces between the HTTP version and the status code:

HTTP/1.0  401 Unauthorized

the Go HTTP client returns an error: malformed HTTP status code ""

Browsers and curl accept the extra space without complaining, even though the RFC indicates a single space character here. Should we do the same?

# go version
go version go1.8 freebsd/amd64
# go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="freebsd"
GOOS="freebsd"
GOPATH="/root"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/freebsd_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -gno-record-gcc-switches"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
# cat malformed.go
package main

import (
	"fmt"
	"net/http"
)

func main() {
	_, err := http.Get("http://74.114.75.28/connections.htm")
	fmt.Println(err)
}
# go run malformed.go
Get http://74.114.75.28/connections.htm: malformed HTTP status code ""
# telnet 74.114.75.28 80
Trying 74.114.75.28...
Connected to c-74-114-75-28.netflash.net.
Escape character is '^]'.
GET /connections.htm HTTP/1.0

HTTP/1.0  401 Unauthorized 
Content-type: text/html 
WWW-Authenticate: Basic realm=""

<HTML><BODY><H1>Your Authentication failed<BR></H1><B>Your Request was denied <BR>You do not have permission to view this page</B><BR></BODY></HTML>Connection closed by foreign host.
# curl http://74.114.75.28/connections.htm
<HTML><BODY><H1>Your Authentication failed<BR></H1><B>Your Request was denied <BR>You do not have permission to view this page</B><BR></BODY></HTML>
@odeke-em odeke-em changed the title http: extra space in response causes error: malformed HTTP status code "" net/http: extra space between response version and statuscode causes error: malformed HTTP status code "" Apr 15, 2017
@odeke-em
Copy link
Member

/cc @bradfitz

@bradfitz
Copy link
Contributor

I'd rather not. Accepting all garbage in violation of specs leads to madness and insecurity. Postel was a little optimistic. I love how strict HTTP/2 is compared to HTTP/1.x. If we've gone this long with this strictness, I don't think there's a problem. (Or is this new in Go 1.8 compares to Go 1.7?)

At least, I won't consider changing our client until the upstream server is fixed. Is there an open bug for the server already? (Which server is it?)

@bradfitz bradfitz added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Apr 15, 2017
@andybalholm
Copy link
Contributor Author

The server is something related to an RTK GPS system. I don't know anything more about it than that. I suspect it's using CGI scripts that manually generate the headers, since the spacing isn't consistent. Fetching /index.htm gives a 200 OK with correct spacing.

If it works in a browser, but not through my proxy, it's my proxy's fault :-(

@bradfitz
Copy link
Contributor

You didn't answer my Go 1.7 question.

@andybalholm
Copy link
Contributor Author

Go 1.7 gives the same error.

@bradfitz
Copy link
Contributor

Here's a workaround: set the ReverseProxy.Transport to your own http.Transport and override Transport.DialContext to a wrapper around Dialer.DialContext that wraps its returned Conns in:

type fixHTTPReader struct {
	r io.Reader
}

var httpSpaceSpace = []byte("HTTP/1.0  ")

func (f fixHTTPReader) Read(p []byte) (n int, err error) {
	n, err = f.r.Read(p)
	p = p[:n]
	if bytes.HasPrefix(p, httpSpaceSpace) {
		copy(p[9:], p[10:])
		n--
	}
	return
}

e.g. https://play.golang.org/p/Wsx-dmmkaS

@andybalholm
Copy link
Contributor Author

It's not a reverse proxy. It's a forward proxy (github.com/andybalholm/redwood). That's why I don't have any control over the upstream server, or even much information about it. We're exposed to all the brokenness of the whole internet, not just our own data center.

I'm enough of a pragmatist to advocate ignoring extra spaces, but not enough of one to wrap all my upstream connections in a custom reader for the sake of one broken server. So if the relaxed parsing is deemed too Postel, I guess we'll just need to tell our user to figure out a way to bypass the proxy for that particular host.

@bradfitz
Copy link
Contributor

Sigh. Changes for one server are sad. But sure... if you want to fix it and you include a test, I'll approve it. If browsers accept it, I suppose that's reason enough.

@bradfitz bradfitz added NeedsFix The path to resolution is known, but the work has not been done. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Apr 16, 2017
@bradfitz bradfitz added this to the Go1.9Maybe milestone Apr 16, 2017
@gopherbot
Copy link

CL https://golang.org/cl/40933 mentions this issue.

@RalphCorderoy
Copy link

This should not be approved. It is one broken server that has tiny market share. If Go changes to be lax here with its large and growing usage then that's a bigger window for more broken servers to creep into existence. Meanwhile, a couple of years down the line, either no one is using this broken GPS or it has a fix because of complaints but Go is stuck violating the standard.

I realise that's a pain for @andybalholm, but the pain shouldn't be shared.

"Protocol designs and implementations should be maximally strict" — https://tools.ietf.org/html/draft-thomson-postel-was-wrong-00

That also describes how HTTP had degraded due to varying implementations and a six-year effort managed to pick through those to create HTTP/1.1; to "restore the ability to create and maintain interoperable implementations". net/http shouldn't chip away at that work.

@golang golang locked and limited conversation to collaborators Apr 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants