-
Notifications
You must be signed in to change notification settings - Fork 18k
net/http: Segfault when making request to specific URL #22376
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
Comments
Using the same test case against HEAD (a31e0a4)
|
Doesn't appear to be reproducible anymore. |
A bit odd, I had to reboot in order to get the failures to occur again. |
Perhaps the author is changing the contents of that URL and actively switching the Netlify addresses because I got back to my hotel after dinner an hour ago, and now it 404s
However, I was able to capture the 301 response that caused it to break in the first place package main
import (
"bufio"
"bytes"
"flag"
"fmt"
"log"
"net"
"net/http"
)
func main() {
var port int
flag.IntVar(&port, "port", 8877, "the port to run on")
flag.Parse()
addr := fmt.Sprintf(":%d", port)
ln, err := net.Listen("tcp", addr)
if err != nil {
log.Fatal(err)
}
for {
conn, err := ln.Accept()
if err == nil {
go handleConn(conn)
}
}
}
var onRedirect = []byte(
`HTTP/1.1 301 Moved Permanently
Cache-Control: public, max-age=0, must-revalidate
Date: Sun, 22 Oct 2017 04:32:22 GMT
Etag: "fc0674b76d7fc65eab9b07076c43b0e4-ssl"
Strict-Transport-Security: max-age=31536000
Content-Length: 0
Age: 3960
Connection: keep-alive
Server: Netlify
Location: /blog/dont-design-emails/
Content-Type: text/plain`)
var finalResponse = []byte(
`HTTP/1.1 200 OK
Cache-Control: public, max-age=0, must-revalidate
Content-Type: text/html; charset=UTF-8
Date: Sun, 22 Oct 2017 04:32:23 GMT
Etag: "fc0674b76d7fc65eab9b07076c43b0e4-ssl"
Strict-Transport-Security: max-age=31536000
Age: 3960
Content-Length: 12
Connection: keep-alive
Server: Netlify` + "\r\n\r\nHello Gopher")
func handleConn(conn net.Conn) {
in := make([]byte, 1024)
n, err := conn.Read(in)
if err != nil {
return
}
in = in[:n]
log.Printf("in: %q\n", in)
req, err := http.ReadRequest(bufio.NewReader(bytes.NewReader(in)))
if err != nil {
return
}
data := onRedirect
if req.URL.Path != "/" {
data = finalResponse
}
conn.Write(data)
conn.Close()
} We'll need to put that behind self-signed TLS certs so that we take the HTTP/2 path as well. |
I got back to my hotel after a walk and I've written perhaps the closest repro to almost retrace the path taken, HTTP/2 server enabled, HTTP/2 transport but in vain. Repros at https://github.com/odeke-em/bugs/blob/master/golang/22376 and server in particular package main
import (
"flag"
"fmt"
"log"
"net/http"
)
func main() {
var port int
var certFile, keyFile string
flag.StringVar(&certFile, "cert", "cert.pem", "the certificate")
flag.StringVar(&keyFile, "key", "key.pem", "the key file")
flag.IntVar(&port, "port", 8877, "the port to run on")
flag.Parse()
addr := fmt.Sprintf(":%d", port)
if err := http.ListenAndServeTLS(addr, certFile, keyFile, &handler{}); err != nil {
log.Fatal(err)
}
}
type handler struct{}
func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
hdr := w.Header()
if r.URL.Path == "/" {
hdr.Set("Content-Type", "text/plain")
hdr.Set("Age", "3960")
hdr.Set("Connection", "keep-alive")
hdr.Set("Date", "keep-alive")
hdr.Set("Server", "Netlify")
hdr.Set("Cache-Control", "public, max-age=0, must-revalidate")
hdr.Set("Date", "Sun, 22 Oct 2017 04:32:22 GMT")
if false && r.URL.RawPath == "" { // Can't seem to differentiate between localhost:8877 & localhost:8877/
hdr.Set("Location", "/")
} else {
hdr.Set("Location", "/blog/dont-design-emails/")
}
w.WriteHeader(301)
} else {
hdr.Set("Cache-Control", "public, max-age=0, must-revalidate")
hdr.Set("Content-Type", "text/html; charset=UTF-8")
hdr.Set("Date", "Sun, 22 Oct 2017 04:32:23 GMT")
hdr.Set("Age", "3960")
hdr.Set("Connection", "keep-alive")
hdr.Set("Server", "Netlify")
fmt.Fprintf(w, "Hello Gopher")
}
w.(http.Flusher).Flush()
} Perhaps @tombergan @fraenkel @bradfitz y'all might see something here? |
I captured the http2debug when it works and when it fails. It seems we are getting back slightly different responses. BAD response
GOOD
|
Some details in case this endpoint changes: (this is for If OP's program is run with
If run with
|
Thanks for the detailed http2debug logs. I'm pretty sure I know what's wrong and will fix it tomorrow if no one else gets there first. Briefly: cs.bufPipe is nil when isHead but we don't check isHead in processData. Actually we should be treating isHead the same as if the HEADERS frame contained END_STREAM (here). |
Change https://golang.org/cl/72551 mentions this issue: |
Reopening: not fixed until I backport into net/http. |
Change https://golang.org/cl/75210 mentions this issue: |
Change https://golang.org/cl/88319 mentions this issue: |
Change https://golang.org/cl/88655 mentions this issue: |
Change https://golang.org/cl/88675 mentions this issue: |
Change https://golang.org/cl/88676 mentions this issue: |
If a server returns a DATA frame while procesing a HEAD request, the client will discard the data. Fixes golang/go#22376 Change-Id: Ief9c17ddfe51cc17f7f6326c87330ac9d8b9d3ff Reviewed-on: https://go-review.googlesource.com/72551 Run-TryBot: Tom Bergan <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Tom Bergan <[email protected]> Reviewed-on: https://go-review.googlesource.com/88655 Run-TryBot: Andrew Bonventre <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]>
Updates http2 to x/net/http2 git rev 44b7c21 for http2: Discard data reads on HEAD requests https://golang.org/cl/88655 Fixes #22376 Change-Id: I931d9065d7309bc6d3f978bfe8cc6a9f940ce9e9 Reviewed-on: https://go-review.googlesource.com/88676 Run-TryBot: Andrew Bonventre <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
If a server returns a DATA frame while procesing a HEAD request, the client will discard the data. Fixes golang/go#22376 Change-Id: Ief9c17ddfe51cc17f7f6326c87330ac9d8b9d3ff Reviewed-on: https://go-review.googlesource.com/72551 Run-TryBot: Tom Bergan <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Tom Bergan <[email protected]>
What did you do?
I made an HTTP HEAD request for https://www.gkogan.co/blog/dont-design-email. The program crashed. The following program reproduces the issue:
What did you expect to see?
The program should exit with no output.
What did you see instead?
More information
If I instead make a request to
https://www.gkogan.co/blog/dont-design-emails/
(note the trailing slash) rather thanhttps://www.gkogan.co/blog/dont-design-emails
, the program does not crash.I made another program which demonstrates this. See my gist at https://gist.github.com/cgt/bd14b8250eac6a7dc8a9830f5bb0bf85 which contains the program and its output.
I cannot reproduce the crash with any URL other than
https://www.gkogan.co/blog/dont-design-email
.System details
The text was updated successfully, but these errors were encountered: