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

Comments

Projects
None yet
5 participants
@andybalholm
Copy link

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

This comment has been minimized.

Copy link
Member

commented Apr 15, 2017

/cc @bradfitz

@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 15, 2017

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?)

@andybalholm

This comment has been minimized.

Copy link
Author

commented Apr 15, 2017

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

This comment has been minimized.

Copy link
Member

commented Apr 15, 2017

You didn't answer my Go 1.7 question.

@andybalholm

This comment has been minimized.

Copy link
Author

commented Apr 15, 2017

Go 1.7 gives the same error.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 16, 2017

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

This comment has been minimized.

Copy link
Author

commented Apr 16, 2017

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

This comment has been minimized.

Copy link
Member

commented Apr 16, 2017

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 and removed WaitingForInfo labels Apr 16, 2017

@bradfitz bradfitz added this to the Go1.9Maybe milestone Apr 16, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Apr 17, 2017

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

@RalphCorderoy

This comment has been minimized.

Copy link

commented Apr 17, 2017

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.

@gopherbot gopherbot closed this in 668cca6 Apr 17, 2017

@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.
You can’t perform that action at this time.