subprotocol is converted to lowercase, problem if remote client is specifying in uppercase about websocket-node HOT 6 CLOSED

Interesting. Looking into this now.

from websocket-node.

Fixed. Here's my description from the commit message:

I'm still maintaining lower case subprotocol names internally, but now am maintaining a mapping in the request object to the exact subprotocol string requested by the client, so that exact string can be returned in the response. This allows us to maintain case insensitivity in subprotocol names for practicality, but to return the exact casing of a subprotocol expected by the client.

from websocket-node.

Thanks for the change 👍 , however I noticed that you've added in a line stating:

this.requestedProtocols = null;

Which is resulting in it being set to null. It's easy to work around but previously this wasn't happening so the change broke my code.

It's only minor but after reviewing what was changed I wondered if it made sense to clear out the requested protocols after accepting a protocol, it still may be handy to know what was originally requested. You're only doing it when protocolFullCase, not sure on the conditions around this.

I'm not worried either way, just thought I would mention it.

Cheers :-)

from websocket-node.

I nulled it because its not useful after the connection has been accepted: the protocol that was chosen to be spoken on the wire is available in connection.protocol. After the connection is accepted, any other protocols suggested by the client that weren't accepted are irrelevant. If you really need them, you can save the value of request.requestedProtocols before calling request.accept().

Happy new year!!!

Brian

Sent from my iPhone

On Dec 31, 2012, at 9:02 PM, James Jenner [email protected] wrote:

Thanks for the change , however I noticed that you've added in a line stating:

this.requestedProtocols = null;
Which is resulting in it being set to null. It's easy to work around but previously this wasn't happening so the change broke my code.

It's only minor but after reviewing what was changed I wondered if it made sense to clear out the requested protocols after accepting a protocol, it still may be handy to know what was originally requested.

I'm not worried either way, just thought I would mention it.

Cheers :-)

—
Reply to this email directly or view it on GitHub.

from websocket-node.

In fact, the entire request object is meant to be garbage collected after accepting the connection, so I recommend not holding a reference to it in order to optimize memory consumption.

Sent from my iPhone

On Dec 31, 2012, at 9:02 PM, James Jenner [email protected] wrote:

Thanks for the change , however I noticed that you've added in a line stating:

this.requestedProtocols = null;
Which is resulting in it being set to null. It's easy to work around but previously this wasn't happening so the change broke my code.

It's only minor but after reviewing what was changed I wondered if it made sense to clear out the requested protocols after accepting a protocol, it still may be handy to know what was originally requested.

I'm not worried either way, just thought I would mention it.

Cheers :-)

—
Reply to this email directly or view it on GitHub.

from websocket-node.

@theturtle32 I seem to still be getting this problem. I'm using 1.0.8 (taken from npm) and I have the exact problem as mentioned a year ago. Why is this case being manipulated at all?

from websocket-node.