Add Handshake.Header field #203
Conversation
server.go
Outdated
| @@ -514,6 +522,9 @@ func (u Upgrader) Upgrade(conn io.ReadWriter) (hs Handshake, err error) { | |||
| break | |||
| } | |||
|
|
|||
| // copy and add header | |||
| hs.Header.Add(btsToString(bytes.Clone(k)), btsToString(bytes.Clone(v))) | |||
There was a problem hiding this comment.
to prevent problems caused by using bytespool, a copy is needed
server.go
Outdated
| @@ -241,6 +241,10 @@ func (u HTTPUpgrader) Upgrade(r *http.Request, w http.ResponseWriter) (conn net. | |||
| if h := u.Header; h != nil { | |||
| header[0] = HandshakeHeaderHTTP(h) | |||
| } | |||
|
|
|||
| // set handshake header | |||
| hs.Header = r.Header | |||
There was a problem hiding this comment.
here we use http.Request Header directly without copying it.
|
Hi, thanks for the PR. Can you clarify what you're trying to achieve? |
I copy the headers during the handshake into the |
For example, some developers want to obtain the |
|
I'm not 100% sure what is the point of passing request headers to handshake result. User can access exactly the same data by looking at request directly. What is the benefit of keeping headers in a handshake? |
but, For example, I want to build a pure websocket server, using net + gobwas/ws, in this case I can't do it like you do. |
Based purely on TCP connections, right? Ok, I think I'm starting to understand your case. The only thing that looks scary to me is that we will start allocating/copying all the headers for each request, which is completely not acceptable with the current guarantees. The only thing I see now is to hide it under a flag. |
i got it, i will add build tags support for it later |
|
No-no, not a build tag. Flag in upgraders. |
|
Wait a sec, have you looked at |
I tried it and found that OnHeader captures all the incoming connection headers. So I added the headers to the Handshake |
got it. |
Exactly what you need, isn't it? |
wrong, there is no way to know which connection the header was passed in from. for example, identity authentication needs to be performed on a specific connection, and this process cannot be completed using OnHeader |
|
plz review it. i am not sure if what I did meets your requirements. |
|
I will take a look tomorrow, thanks. |
can u merge now? thanks. |
I think we should add a Header field to Handshake. Some developers may need to process the Header of the corresponding connection. At present, it is difficult for gobwas to implement similar functions.