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

Feature Request: expose handleRawConn or add ServeConn #7315

Closed
CoiaPrant233 opened this issue Jun 9, 2024 · 26 comments
Closed

Feature Request: expose handleRawConn or add ServeConn #7315

CoiaPrant233 opened this issue Jun 9, 2024 · 26 comments
Labels
Status: Requires Reporter Clarification Type: Feature New features or improvements in behavior

Comments

@CoiaPrant233
Copy link

CoiaPrant233 commented Jun 9, 2024

Use case(s) - what problem will this feature solve?

gRPC in custom implemented net.Conn

Proposed Solution

expose handleRawConn

Alternatives Considered

add ServeConn interface

Additional Context

ServeConn should block

	go func() {
		s.serveStreams(context.Background(), st, rawConn)
		s.removeConn(lisAddr, st)
	}()

to

	defer s.removeConn(lisAddr, st)
	s.serveStreams(context.Background(), st, rawConn)
@CoiaPrant233 CoiaPrant233 added the Type: Feature New features or improvements in behavior label Jun 9, 2024
@purnesh42H purnesh42H self-assigned this Jun 11, 2024
@purnesh42H
Copy link
Contributor

@CoiaPrant233 I will take a look and get back to you

@purnesh42H purnesh42H added the P2 label Jun 14, 2024
@purnesh42H
Copy link
Contributor

@CoiaPrant233 could you describe your use case for this request?

@CoiaPrant233
Copy link
Author

@CoiaPrant233 could you describe your use case for this request?

gRPC in websocket.Conn

@easwars
Copy link
Contributor

easwars commented Jun 14, 2024

gRPC in websocket.Conn

Could you please be more specific.

@CoiaPrant233
Copy link
Author

I wrapped a websocket implement net.Conn, but its required block I/O. If http handler function was returned. websocket will be closed automatically.

@purnesh42H
Copy link
Contributor

Could you explain what is your usecase that needs websocket connection? Is it for a web application? In case you haven't yet, take a look at grpc Bidirectional Streaming and see if that can help with your usecase

@CoiaPrant233
Copy link
Author

Could you explain what is your usecase that needs websocket connection? Is it for a web application? In case you haven't yet, take a look at grpc Bidirectional Streaming and see if that can help with your usecase

Some middleware without http/2 or grpc support.
Not all CDN like cloudflare support http/2 to origin or grpc to origin.

@CoiaPrant233
Copy link
Author

So must add a websocket wrapping let connection passthrough middleware.

@purnesh42H
Copy link
Contributor

Some middleware without http/2 or grpc support.
Not all CDN like cloudflare support http/2 to origin or grpc to origin.

Have you tried exploring grpc-web? This allows grpc to work over HTTP/1.1 which is supported by cloudfare.

@CoiaPrant233
Copy link
Author

CoiaPrant233 commented Jun 15, 2024

Some middleware without http/2 or grpc support.
Not all CDN like cloudflare support http/2 to origin or grpc to origin.

Have you tried exploring grpc-web? This allows grpc to work over HTTP/1.1 which is supported by cloudfare.

Yes, but I need Bi-directional stream.

@arjan-bal
Copy link
Contributor

arjan-bal commented Jun 15, 2024

@CoiaPrant233 you can try using an HTTP tunnel (also called HTTP Connect Proxy). This should allow you to bypass the middleware if the HTTP 1.1 proxy server is placed behind the middleware.

gRPC-go supports HTTP tunneling

@CoiaPrant233
Copy link
Author

@CoiaPrant233 you can try using an HTTP tunnel (also called HTTP Connect Proxy). This should allow you to bypass the middleware if the HTTP 1.1 proxy server is placed behind the middleware.

gRPC-go supports HTTP tunneling

I want to passthrough CDN to protect origin server.

@easwars
Copy link
Contributor

easwars commented Jun 17, 2024

Providing a custom way to handle the incoming connection is not something we are very keen on doing. Would one of the following options work for you?

  • Run your own proxy next to your server and terminate the websocket connection there. Have the proxy talk gRPC to your server backend.
  • Provide a net.Listener which actually wraps a websocket conn (to grpc.Serve) and do your custom handling there.

@CoiaPrant233
Copy link
Author

Providing a custom way to handle the incoming connection is not something we are very keen on doing. Would one of the following options work for you?

  • Run your own proxy next to your server and terminate the websocket connection there. Have the proxy talk gRPC to your server backend.
  • Provide a which actually wraps a websocket conn (to ) and do your custom handling there.net.Listener``grpc.Serve

I tried them. But it has some bugs.

@easwars
Copy link
Contributor

easwars commented Jun 18, 2024

I tried them. But it has some bugs.

Could you provide details on what you tried and what bugs you found? Thanks.

@CoiaPrant233
Copy link
Author

I tried them. But it has some bugs.

Could you provide details on what you tried and what bugs you found? Thanks.

WebSocket library not has wait close function, I tried wrapped to net.Listener, but I can't immediately determine if the connection is closed.
If I want to check its state I must send a websocket ping message and check its has write error.

It looks like terrible. In a high concurrency environment, a websocket object will be held for a long time and will not be released even if it is closed.
If blocking IO is used, grpc returns and frees the web socket object in the function immediately after processing the connection.

@easwars
Copy link
Contributor

easwars commented Jun 18, 2024

I tried wrapped to net.Listener, but I can't immediately determine if the connection is closed.

From your wrapped net.Listener you can return a wrapped net.Conn, whose Close method should be invoked when the connection is closed. Does that work for you?

a websocket object will be held for a long time and will not be released even if it is closed

I don't know much about websocket, but gRPC does provide a way to close connections that have been open for more than a configurable time. See here: https://pkg.go.dev/google.golang.org/grpc/keepalive#ServerParameters

What issues did you run into when running your own proxy to terminate the websocket connection?

@easwars easwars removed their assignment Jun 18, 2024
@CoiaPrant233
Copy link
Author

I think blocking IO is better. Just add a new function.

or u can removed async call in handleRawConn

At server.go#L926 already call async function
At server.go#L958 call async function again

I think its useless to call twice async. u can modify it like ServeHTTP

from

// handleRawConn forks a goroutine to handle a just-accepted connection that
// has not had any I/O performed on it yet.
func (s *Server) handleRawConn(lisAddr string, rawConn net.Conn) {
	if s.quit.HasFired() {
		rawConn.Close()
		return
	}
	rawConn.SetDeadline(time.Now().Add(s.opts.connectionTimeout))

	// Finish handshaking (HTTP2)
	st := s.newHTTP2Transport(rawConn)
	rawConn.SetDeadline(time.Time{})
	if st == nil {
		return
	}

	if cc, ok := rawConn.(interface {
		PassServerTransport(transport.ServerTransport)
	}); ok {
		cc.PassServerTransport(st)
	}

	if !s.addConn(lisAddr, st) {
		return
	}
	go func() {
		s.serveStreams(context.Background(), st, rawConn)
		s.removeConn(lisAddr, st)
	}()
}

to

// handleRawConn forks a goroutine to handle a just-accepted connection that
// has not had any I/O performed on it yet.
func (s *Server) handleRawConn(lisAddr string, rawConn net.Conn) {
	if s.quit.HasFired() {
		rawConn.Close()
		return
	}
	rawConn.SetDeadline(time.Now().Add(s.opts.connectionTimeout))

	// Finish handshaking (HTTP2)
	st := s.newHTTP2Transport(rawConn)
	rawConn.SetDeadline(time.Time{})
	if st == nil {
		return
	}

	if cc, ok := rawConn.(interface {
		PassServerTransport(transport.ServerTransport)
	}); ok {
		cc.PassServerTransport(st)
	}

	if !s.addConn(lisAddr, st) {
		return
	}
	defer s.removeConn(lisAddr, st)
	s.serveStreams(context.Background(), st, rawConn)
}

then I can use unsafe to force expose it

@CoiaPrant233
Copy link
Author

I think add new function ServeConn like ServeHTTP
its best resolution.

We need blocking I/O for serve single connection.

Copy link

This issue is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

@github-actions github-actions bot added the stale label Jun 24, 2024
@CoiaPrant233
Copy link
Author

anyone here?

@CoiaPrant233 CoiaPrant233 removed their assignment Jun 25, 2024
@github-actions github-actions bot removed the stale label Jun 25, 2024
Copy link

github-actions bot commented Jul 1, 2024

This issue is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

@github-actions github-actions bot added the stale label Jul 1, 2024
@CoiaPrant233
Copy link
Author

anyone here?

@github-actions github-actions bot removed the stale label Jul 3, 2024
@dfawley dfawley removed the P2 label Jul 3, 2024
@purnesh42H
Copy link
Contributor

@CoiaPrant233 could you provide more details on how you are implementing net.Listener and net.Conn for websocket connection?

@CoiaPrant233
Copy link
Author

@CoiaPrant233 could you provide more details on how you are implementing net.Listener and net.Conn for websocket connection?

package main

import (
	"net/http"

	"github.com/gin-gonic/gin"
	"nhooyr.io/websocket"
)

func Serve(c *gin.Context) {
	if c.IsWebsocket() {
		c.AbortWithStatus(http.StatusUpgradeRequired)
		return
	}

	ws, err := websocket.Accept(c.Writer, c.Request, &websocket.AcceptOptions{})
	if err != nil {
		c.AbortWithStatus(http.StatusBadRequest)
		return
	}

	conn := websocket.NetConn(c,ws,websocket.MessageBinary)
	defer conn.Close()

	// grpc handleRawConn
}

Here is a example

@purnesh42H
Copy link
Contributor

@CoiaPrant233 you need to first implement a custom net.Listener which actually wraps a WebSocket connection. The idea is to create a custom listener that accepts WebSocket connections and then wraps these connections to provide the standard net.Conn interface.

  • Create a struct for the WebSocket listener and implement the net.Listener interface.
  • Next, create a struct for the WebSocket connection and implement the net.Conn interface.
  • After that, you can create the handler to accept WebSocket connections and add them to the listener and finally set up the server to listen for WebSocket connections

This way, as suggested above, your wrapped net.Conn Close method will be invoked and you can handle it as you need.

With a listener we just call Accept and handleRawConn in a loop so you don't have to actually override Serve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Requires Reporter Clarification Type: Feature New features or improvements in behavior
Projects
None yet
Development

No branches or pull requests

6 participants