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

proposal: net/http: add a ServeConn method to http.Server to handle net.Conn #36673

Open
wzshiming opened this issue Jan 21, 2020 · 10 comments
Open

Comments

@wzshiming
Copy link

@wzshiming wzshiming commented Jan 21, 2020

// x/net/http
func (s *Server) ServeConn(c net.Conn, opts *ServeConnOpts) 

or

// github.com/valyala/fasthttp
func (s *Server) ServeConn(c net.Conn) error 
@bontibon
Copy link
Contributor

@bontibon bontibon commented Jan 21, 2020

What is the use case for something like this? Could you just create a net.Listener implementation which returns your connection when Accept is called?

@wzshiming
Copy link
Author

@wzshiming wzshiming commented Jan 21, 2020

This is what I wrote now, but it does n’t feel good.

type Listener struct {
	ch chan net.Conn
}

func NewListener() *Listener {
	return &Listener{
		ch: make(chan net.Conn),
	}
}

// Send a conn to listener.
func (l *Listener) Send(conn net.Conn) error {
	l.ch <- conn
	return nil
}

// Accept waits for and returns the next connection to the listener.
func (l *Listener) Accept() (net.Conn, error) {
	conn, ok := <-l.ch
	if !ok {
		return nil, io.ErrClosedPipe
	}
	return conn, nil
}

// Close closes the listener.
// Any blocked Accept operations will be unblocked and return errors.
func (l *Listener) Close() error {
	close(l.ch)
	return nil
}

// Addr returns the listener's network address.
func (l *Listener) Addr() net.Addr {
	return noneAddr{}
}

type noneAddr struct {
}

func (noneAddr) Network() string {
	return "none"
}
func (noneAddr) String() string {
	return "none"
}

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jan 21, 2020

Yeah, implementing your own single connection listener is what's usually done. I see a few on GitHub:

https://github.com/search?q=singleConnListener&type=Code

And IIRC we have one or more in the standard library's tests.

@toothrot toothrot added this to the Backlog milestone Jan 21, 2020
@wzshiming
Copy link
Author

@wzshiming wzshiming commented Feb 2, 2020

==================
WARNING: DATA RACE
Read at 0x00c000001b60 by goroutine 75:
  net/http.http2ConfigureServer()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/h2_bundle.go:3828 +0x307
  net/http.(*Server).onceSetNextProtoDefaults()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:3168 +0x145
  net/http.(*Server).onceSetNextProtoDefaults-fm()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:3158 +0x41
  sync.(*Once).doSlow()
      /usr/local/Cellar/go/1.13.5/libexec/src/sync/once.go:66 +0x100
  sync.(*Once).Do()
      /usr/local/Cellar/go/1.13.5/libexec/src/sync/once.go:57 +0x68
  net/http.(*Server).setupHTTP2_ServeTLS()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:3132 +0x68
  net/http.(*Server).ServeTLS()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:2948 +0x53

Previous write at 0x00c000001b60 by goroutine 42:
  net/http.http2ConfigureServer()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/h2_bundle.go:3835 +0x63b
  net/http.(*Server).onceSetNextProtoDefaults()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:3168 +0x145
  net/http.(*Server).onceSetNextProtoDefaults-fm()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:3158 +0x41
  sync.(*Once).doSlow()
      /usr/local/Cellar/go/1.13.5/libexec/src/sync/once.go:66 +0x100
  sync.(*Once).Do()
      /usr/local/Cellar/go/1.13.5/libexec/src/sync/once.go:57 +0x68
  net/http.(*Server).setupHTTP2_ServeTLS()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:3132 +0x68
  net/http.(*Server).ServeTLS()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:2948 +0x53
  github.com/wzshiming/pipe/stream/http.(*server).serve()

There seems to be something wrong with TLS
I can only add "h2" to tls.nextprotos in advance

s.tls.NextProtos = append(s.tls.NextProtos, "h2")
func (s *server) ServeConn(ctx context.Context, conn net.Conn) {
	listener := &singleConnListener{conn}
	var svc = http.Server{
		Handler:   s,
		TLSConfig: s.tls,
		BaseContext: func(net.Listener) context.Context {
			return ctx
		},
	}
	if s.tls == nil {
		err := svc.Serve(listener)
		if err != nil {
			log.Println("[ERROR]", err)
		}
	} else {
		err := svc.ServeTLS(listener, "", "")
		if err != nil {
			log.Println("[ERROR]", err)
		}
	}
}

@wzshiming
Copy link
Author

@wzshiming wzshiming commented Feb 2, 2020

==================
WARNING: DATA RACE
Write at 0x00c00008b738 by goroutine 75:
  net/http.http2ConfigureServer()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/h2_bundle.go:3825 +0x2c4
  net/http.(*Server).onceSetNextProtoDefaults()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:3168 +0x145
  net/http.(*Server).onceSetNextProtoDefaults-fm()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:3158 +0x41
  sync.(*Once).doSlow()
      /usr/local/Cellar/go/1.13.5/libexec/src/sync/once.go:66 +0x100
  sync.(*Once).Do()
      /usr/local/Cellar/go/1.13.5/libexec/src/sync/once.go:57 +0x68
  net/http.(*Server).setupHTTP2_ServeTLS()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:3132 +0x68
  net/http.(*Server).ServeTLS()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:2948 +0x53
  github.com/wzshiming/pipe/stream/http.(*server).serve()
  
Previous write at 0x00c00008b738 by goroutine 43:
  net/http.http2ConfigureServer()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/h2_bundle.go:3825 +0x2c4
  net/http.(*Server).onceSetNextProtoDefaults()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:3168 +0x145
  net/http.(*Server).onceSetNextProtoDefaults-fm()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:3158 +0x41
  sync.(*Once).doSlow()
      /usr/local/Cellar/go/1.13.5/libexec/src/sync/once.go:66 +0x100
  sync.(*Once).Do()
      /usr/local/Cellar/go/1.13.5/libexec/src/sync/once.go:57 +0x68
  net/http.(*Server).setupHTTP2_ServeTLS()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:3132 +0x68
  net/http.(*Server).ServeTLS()
      /usr/local/Cellar/go/1.13.5/libexec/src/net/http/server.go:2948 +0x53

I think it is necessary to build a pool for the same tlsconfig for reuse

@wzshiming
Copy link
Author

@wzshiming wzshiming commented Feb 2, 2020

func (s *server) serve(ctx context.Context, listener net.Listener, handler http.Handler) {
	var svc = http.Server{
		Handler:   handler,
		TLSConfig: s.tls,
		BaseContext: func(net.Listener) context.Context {
			return ctx
		},
	}
	if s.tls == nil {
		err := svc.Serve(listener)
		if err != nil {
			log.Println("[ERROR] ", err)
		}
	} else {
		err := svc.ServeTLS(listener, "", "")
		if err != nil && err != io.ErrClosedPipe {
			log.Println("[ERROR] ", err)
		}
	}
}

func (s *server) ServeConn(ctx context.Context, conn net.Conn) {
	wait := make(chan struct{})
	s.serve(ctx, &singleConnListener{conn},
		http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
			s.handler.ServeHTTP(rw, r)
			wait <- struct{}{}
		}))
	<-wait
}

The listener will fork a goroutine, need to wait for it to finish.

@agnivade agnivade changed the title net/http: Propose add a ServeConn method to http.Server to handle net.Conn proposal: net/http: add a ServeConn method to http.Server to handle net.Conn Feb 3, 2020
@bluecmd
Copy link

@bluecmd bluecmd commented Jun 19, 2020

+1. x/net/http2 already has this - but it would be great to at the very least have something that implements this for http1.1 as well.

Even better would be if net/http had one that could do http1.1 and http2 based on the connection it was given (if c.ConnectionState().NegotiatedProtocol == "h2" yadda yadda).

@emersion
Copy link

@emersion emersion commented Oct 19, 2020

I'd need this as well, here's my use-case.

I'm writing an HTTP server behind a TLS reverse proxy. The reverse proxy terminates TLS connections, and forwards them to my HTTP server. The PROXY protocol is used to forward TLS metadata (e.g. ALPN).

I'd like to make HTTP2 work if advertised by the client by ALPN. However, net/http won't enable HTTP2 unless the connection is a *tls.Conn. The x/net/http2 package won't support HTTP1. So it's not easy to do the routing.

One solution would be to listen for incoming connections manually, and add them to the HTTP1 or HTTP2 servers depending on the relayed ALPN protocol string. This requires ServeConn.

Is this use-case enough for you? I'm willing to work on a patch.

(Another solution would be to make net/http assert on an interface rather than *tls.Conn.)

@networkimprov
Copy link

@networkimprov networkimprov commented Oct 19, 2020

cc @ianlancetaylor re incoming proposal

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Oct 19, 2020
@ianlancetaylor ianlancetaylor removed this from the Backlog milestone Oct 19, 2020
@ianlancetaylor ianlancetaylor added this to the Proposal milestone Oct 19, 2020
@ShivanshVij
Copy link

@ShivanshVij ShivanshVij commented Jun 24, 2021

Any updates on this? I'd love to take a crack at it myself if that's acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Incoming
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
10 participants