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

NetHTTPResponseWriter support hijackerConn for fasthttp adaptor #946

Closed
wants to merge 1 commit into from

Conversation

yoyofx
Copy link

@yoyofx yoyofx commented Jan 19, 2021

if the request connection upgrade the hijacker connection, that handler needed direct return after ServeHTTP handler process .

@erikdubbelboer
Copy link
Collaborator

What is hijackerConn? Where is IsHijackerConn set to true? Why put Ctx in there if you're not going to use it?

@yoyofx
Copy link
Author

yoyofx commented Jan 19, 2021

What is hijackerConn? Where is IsHijackerConn set to true? Why put Ctx in there if you're not going to use it?

  1. because the netHTTPResponseWriter is not implement Hijacker interface , that is call name 'hijackerConn' .
  2. I need call ctx.Hijack(func(netConn net.Conn) func in http.Handler , upgrade it to websocket.
  3. IsHijackerConn call adpter handler, that's 'Hijack' not needed anything. and then hold the net.conn in http.Handler.

I used to this example: https://github.com/yoyofx/yoyogo/blob/dev/web/websocketupgrader.go#L49

@yoyofx
Copy link
Author

yoyofx commented Jan 19, 2021

readme:
http.Hijacker interface, the Hijack() (net.Conn, *bufio.ReadWriter, error) is sync func.

ctx.Hijack(func(netConn net.Conn) is callback func ,that is async.
In the 'adpter' .if don't direct return on after call ctx.Hijjack func , fasthttp is async process fasthttp.RequestHandler.
and then the net.Conn will not wait to be used.

@erikdubbelboer
Copy link
Collaborator

I don't understand, with your pull NetHTTPResponseWriter still doesn't implement http.Hijacker. If that if your desire then why not implement it? Is there code missing in your pull maybe? Right now your just adding fields to the struct and not using them? Can you maybe add a test case that fails before your pull and succeeds after?

@oSethoum
Copy link

oSethoum commented Oct 5, 2022

@erikdubbelboer Hi, am having the same issue when using the fasthttpadaptor
image
how can I fix it?

@erikdubbelboer
Copy link
Collaborator

I tried implementing this, but it's impossible with the way hijacking works in net/http and fasthttp. They are just incompatible.

@oSethoum
Copy link

oSethoum commented Oct 6, 2022

@erikdubbelboer Thanks for trying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants