Skip to content
This repository has been archived by the owner on Sep 17, 2019. It is now read-only.

Server.handleConn hangs #53

Closed
dvyukov opened this issue May 3, 2015 · 3 comments
Closed

Server.handleConn hangs #53

dvyukov opened this issue May 3, 2015 · 3 comments

Comments

@dvyukov
Copy link

dvyukov commented May 3, 2015

The following test hangs in Server.handleConn while the connection is already effectively closed.

package http2

import (
    "io"
    "net"
    "net/http"
    "testing"
    "time"
)

var data = "PRI * HTTP/2.0\r\n\r\nSM" +
    "\r\n\r\n\x00\x00\x00\x01\ainfinfin\ad"

func TestFuzz(t *testing.T) {
    s := &http.Server{}
    s2 := &Server{MaxReadFrameSize: 1 << 16, PermitProhibitedCipherSuites: true}
    c := &MyConn{[]byte(data), false, false}
    s2.handleConn(s, c, http.HandlerFunc(handler))
    if !c.closed {
        panic("connection is not closed")
    }
}

func handler(w http.ResponseWriter, req *http.Request) {
    w.Write([]byte("hello"))
}

type MyConn struct {
    data    []byte
    closed  bool
    written bool
}

func (c *MyConn) Read(b []byte) (n int, err error) {
    if len(c.data) == 0 {
        return 0, io.EOF
    }
    n = copy(b, c.data)
    c.data = c.data[n:]
    return
}

func (c *MyConn) Write(b []byte) (n int, err error) {
    c.written = true
    return len(b), nil
}

func (c *MyConn) Close() error {
    c.closed = true
    return nil
}

func (c *MyConn) LocalAddr() net.Addr {
    return &net.TCPAddr{net.IP{127, 0, 0, 1}, 49706, ""}
}

func (c *MyConn) RemoteAddr() net.Addr {
    return &net.TCPAddr{net.IP{127, 0, 0, 1}, 49706, ""}
}

func (c *MyConn) SetDeadline(t time.Time) error {
    return nil
}

func (c *MyConn) SetReadDeadline(t time.Time) error {
    return nil
}

func (c *MyConn) SetWriteDeadline(t time.Time) error {
    return nil
}

on commit 8348f2f

@dvyukov
Copy link
Author

dvyukov commented May 8, 2015

@bradfitz, do these new bugs look legit to you?

@bradfitz
Copy link
Owner

bradfitz commented May 8, 2015

Yes, there are wrong assumptions of the API contact between the Framer and the server loop around what type of errors can be returned. Stream-level errors are being lost and causing these problems.

I just haven't had time to fix yet.

@bradfitz
Copy link
Owner

Fix out for review: https://go-review.googlesource.com/15735

c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
There was a design problem earlier where the serve goroutine assumed
that the readFrame goroutine could return only connection-level
errors, but the readFrames goroutine (and the underlying Framer)
assumed it could return stream-level errors (type StreamError) and
have them handled as stream errors in the layers above. That's how it
should have been, and what this CL does.

Now readFrames returns both the Frame and error from ReadFrames
together as a pair, and an error isn't necessarily fatal to the
connection.

Fixes golang/go#12733
Fixes bradfitz/http2#53

Change-Id: If4406ceaa019886893d3c61e6bfce25ef74560d3
Reviewed-on: https://go-review.googlesource.com/15735
Reviewed-by: Dmitry Vyukov <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants