Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
go version go1.8.3 darwin/amd64
What operating system and processor architecture are you using (go env)?
GOARCH="amd64" GOBIN="" GOEXE="" GOHOSTARCH="amd64" GOHOSTOS="darwin" GOOS="darwin" GOPATH="/Users/jsilva/go" GORACE="" GOROOT="/usr/local/Cellar/go/1.8.3/libexec" GOTOOLDIR="/usr/local/Cellar/go/1.8.3/libexec/pkg/tool/darwin_amd64" GCCGO="gccgo" CC="clang" GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/_n/q8bzrfk93ss0nf_0fbk2ns8c0000gn/T/go-build250479046=/tmp/go-build -gno-record-gcc-switches -fno-common" 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"
What did you do?
Fetch "http://107-182-231-168.webnow.net.br/status2.xsl?mount=/98fm.aac" or a similar URL from another Icecast server using http.Client.
Source: https://gist.github.com/joaolsilva/d24b7bf5602f0d6615eb5d385fd0ffdc https://play.golang.org/p/MiokU7tQgI
What did you expect to see?
The content of the URL
What did you see instead?
err: Get http://107-182-231-168.webnow.net.br/status2.xsl?mount=/98fm.aac: malformed MIME header line: Content-Disposition = attachment; filename=file.asc
The MIME header might indeed be malformed, but other http clients such as cURL, browsers, etc. work (they seem to ignore the malformed line) and return the content.
Comment From: odeke-em
/cc @tombergan
Comment From: tombergan
Sigh.
On one hand, other browsers accepting this response is evidence that we should accept it as well.
On the other hand, the error comes from the net/textproto library and that "malformed MIME header line" error has been there since the initial implementation in 2010. The net/textproto library is widely used beyond net/http and I'm concerned that removing the error will break other users of net/textproto. There are many other uses of that library, including uses inside and outside the standard library. Also, this, although that ship has sailed.
This will require some thought.
Edit: Maybe this isn't so bad. I suppose we could add an IgnoreMalformedHeaderLines bool to textproto.Reader which defaults to false and is set to true by net/http.
Comment From: rsc
ping @tombergan. should we do this for Go 1.10?
Comment From: tombergan
We're talking about this on golang.org/cl/75350. I think we should fix it but I'll leave it up to Brad on that CL if it's too late for 1.10.
Comment From: tombergan
This is not going to happen for 1.10 because it's too late in the release cycle to resolve the open API questions.
Comment From: odeke-em
Hello there @joaolsilva thank you for this bug report!
So currently with Go tip(pre-Go1.11) I can't produce this bug, and I also tried with Go1.8 but no dice, so perhaps the server might have fixed the header? However, I have made a simple self contained snippet https://play.golang.org/p/whxnvo9-ZlF or inlined below, that can be run end-to-end without relying on an external server and I have tweaked the various parts of the "Content-Disposition" header but can't produce the problem either
package main
import (
"io/ioutil"
"log"
"net/http"
"net/http/httptest"
)
func main() {
log.SetFlags(0)
cst := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Query().Get("k") {
case "":
w.Header().Set("Content-Disposition", `attachment; filename="file.txt"`)
case "unq":
w.Header().Set("Content-Disposition", `attachment; filename=file.txt`)
case "lq":
w.Header().Set("Content-Disposition", `attachment; filename="file.txt`)
case "rq":
w.Header().Set("Content-Disposition", `attachment; filename=file.txt"`)
}
w.Write([]byte(`Hello, World!`))
}))
defer cst.Close()
for _, sym := range []string{"", "unq", "lq", "rq"} {
resp, err := http.Get(cst.URL + "?k=" + sym)
if err != nil {
log.Printf("Failed to perform plain get: %v", err)
continue
}
blob, _ := ioutil.ReadAll(resp.Body)
_ = resp.Body.Close()
log.Printf("sym=%-5q blob:%q\n", sym, blob)
}
}
which just gives me
$ go run repro.go
sym="" blob:"Hello, World!"
sym="unq" blob:"Hello, World!"
sym="lq" blob:"Hello, World!"
sym="rq" blob:"Hello, World!"
that is all the data successfully parsed. Could you please take a look and tweak as possible?
I also tried reverting some changes to net/textproto as per https://github.com/golang/go/commits/master/src/net/textproto/reader.go but hit a point where my Go recompilation was crashing so figured I'd make the repro and hand it over for someone else to take a look. Also /cc @bradfitz
Comment From: joaolsilva
The original URL fixed the header. The following URL still has the same error:
http://ruby.streamguys.com:8120/status2.xsl?mount=/live
err: Get http://ruby.streamguys.com:8120/status2.xsl?mount=/live: net/http: HTTP/1.x transport connection broken: malformed MIME header line: Content-Disposition = attachment; filename=file.asc
I'm able to reproduce this issue with both go version go1.10.3 darwin/amd64 and go version devel +75fdeaa801 Fri Jun 22 13:28:19 2018 +0000 darwin/amd64
The headers are:
curl -I -XGET 'http://ruby.streamguys.com:8120/status2.xsl?mount=/live'
HTTP/1.0 200 OK
Content-Type: text/plain
Content-Length: 207
Content-Disposition = attachment; filename=file.asc
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate
Pragma: no-cache
Access-Control-Allow-Origin: *
Access-Control-Allow-Headers: Origin, Accept, X-Requested-With, Content-Type
Access-Control-Allow-Methods: GET, OPTIONS, HEAD
Please note the Content-Disposition line, and how it's missing the ':'
I've updated the gist at https://gist.github.com/joaolsilva/d24b7bf5602f0d6615eb5d385fd0ffdc with the new URL.
Comment From: bradfitz
I'm inclined to do nothing here.
That's a pretty blatant spec violation, even if browsers accept it. But browsers accepting too much has led to security problems in the past, so I'm not eager to go down that road.
Comment From: joaolsilva
@bradfitz , what can a Go program do to override this behavior and still use net/http? All other http libraries for Go that I've tried had significant issues (not thread-safe, etc.).
Comment From: odeke-em
The original URL fixed the header. The following URL still has the same error:
@joaolsilva thanks for the new URL. Perhaps might you have noticed in the self-contained repro in https://github.com/golang/go/issues/21290#issuecomment-399357404
Comment From: joaolsilva
if possible, for posterity please update that code to reproduce your bug so that in the future when someone is trying to fix it they don't rely on a server that can fix its response as the first URL did.
I can't (or at least I don't know how). The error originates from net/textproto/reader.go. The ReadMIMEHeaderfunction fails if it can't find a ':' in the header, however setting a header using w.Header().Set() will write well formed headers.
Comment From: odeke-em
Perfect, thank you for the prescription @joaolsilva! Here is the self contained repro for posterity
package main
import (
"io/ioutil"
"log"
"net/http"
"net/http/httptest"
)
func main() {
log.SetFlags(0)
cst := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
conn, bufw, _ := w.(http.Hijacker).Hijack()
conn.Write([]byte(
`HTTP/1.0 200 OK
Content-Length: 13
Content-Disposition = attachment; filename=file.asc
Hello, World!`))
bufw.Flush()
conn.Close()
}))
defer cst.Close()
resp, err := http.Get(cst.URL)
if err != nil {
log.Fatalf("Failed to perform plain get: %v", err)
}
_, _ = ioutil.ReadAll(resp.Body)
_ = resp.Body.Close()
}
Comment From: C-Sto
As @bradfitz noted, total spec violation - however it would be good to be able to explicitly allow mangled headers when working with misbehaving servers
Comment From: freswa
Wanted to add another use case to allow broken headers:
curl -D - 'https://identity.apple.com/pushcert/caservice/renew'
``` HTTP/1.1 200 Server: Apple Date: Thu, 12 Oct 2023 12:29:23 GMT Content-Length: 0 Connection: keep-alive Set-Cookie: X_CSRF_COOKIE=b1e7b35d8ff41d2e330e555d2d90b107; expires=Thu, 12-Oct-23 13:29:23 GMT; domain=.apple.com; path=/; HttpOnly; secure 1;: mode=block max-age=31536000;: includeSubdomains Strict-Transport-Security: max-age=31536000; includeSubdomains X-XSS-Protection: 1; mode=block X-Content-Type-Options: nosniff Host: identity.apple.com X-Frame-Options: SAMEORIGIN
Comment From: leonklingele
I have created https://github.com/freswa/dovecot-xaps-daemon/pull/36 which adds code to ignore known, malformed HTTP response headers such as the ones returned from Apple.
Comment From: seankhliao
given how it's possible to work around when absolutely necessary, keeping the standard library's client spec compliant is probably the best.