In JS client, Group names are not encoded in the querystring when a transport reconnects #1233

Closed
Xiaohongt opened this Issue Jan 4, 2013 · 8 comments

Comments

Projects
None yet
4 participants
Contributor

Xiaohongt commented Jan 4, 2013

e.g. if add connection to group "group++1",
Groups.Add(Context.ConnectionId, "group++1");

Then groups in the request querystring is something like "groups=%5B%22MyHub.group++1%22%5D"

Note the special characters "++" are not encoded, so the group name actually becomes "group 1" in this case.

The special character "+" in querystring should be encoded like %2B.

This will affect the streaming transports on reconnect and long polling on every single message.

Owner

DamianEdwards commented Jan 4, 2013

Can we see if this repros in the .NET client? Just looks like a simple case of not building the querystring correctly.

NTaylorMullen was assigned Jan 4, 2013

Contributor

Xiaohongt commented Jan 4, 2013

in.Net client, group names are encoded in the querystring, not repro

Owner

davidfowl commented Jan 4, 2013

The .NET client rocks 😄

Contributor

NTaylorMullen commented Jan 4, 2013

lol

Contributor

NTaylorMullen commented Jan 4, 2013

Can you post specific reproduction steps?

I've done the following tests to try and reproduce on my end but the + symbols are always encoded (with & without reconnect):

  1. Raw Sample -> Join group++1 -> Force reconnect
  2. Chat Sample -> Join group++1 -> Force reconnect
Owner

DamianEdwards commented Jan 4, 2013

Looks like we may need to use encodeURIComponent() instead of escape(), see http://xkr.us/articles/javascript/encode-compare/

JSFiddle: http://jsfiddle.net/DamianEdwards/BX6K4/1/

Contributor

NTaylorMullen commented Jan 4, 2013

Aw, I just noticed that the network monitor in Chrome encodes it automatically for me, hence why I wasn't seeing it reproduced.

@NTaylorMullen NTaylorMullen added a commit that referenced this issue Jan 4, 2013

@NTaylorMullen NTaylorMullen Changed .escape in JS source to instead be encodeURIComponent to ensu…
…re all URI componenets are encoded correctly

- Also removed an empty line in transportheartbeat

#1233
ed228f5

@NTaylorMullen NTaylorMullen added a commit that referenced this issue Jan 4, 2013

@NTaylorMullen NTaylorMullen Changed .escape in JS source to instead be encodeURIComponent to ensu…
…re all URI componenets are encoded correctly

- Also removed an empty line in transportheartbeat

#1233
23623a4

@NTaylorMullen NTaylorMullen added a commit that referenced this issue Jan 4, 2013

@NTaylorMullen NTaylorMullen Added TransportUtilityFacts which unit test some of the transport log…
…ic tests

- Specifically I added a test to confirm the encoding of groups from
getUrl
#1233
915fbb3

Xiaohongt was assigned Jan 4, 2013

Contributor

Xiaohongt commented Jan 5, 2013

verified

Xiaohongt closed this Jan 5, 2013

@NTaylorMullen NTaylorMullen added a commit that referenced this issue Jan 6, 2013

@NTaylorMullen NTaylorMullen Cleaned up TransportUtilityFacts
- Moved a variable declaration to the top of the test
- Added semicolon after object creation
- Called "getUrl" in place instead of detaching it at the top
#1233
e7250f8

@NTaylorMullen NTaylorMullen added a commit that referenced this issue Jan 6, 2013

@NTaylorMullen NTaylorMullen Cleaned up TransportUtilityFacts
- Moved a variable declaration to the top of the test
- Added semicolon after object creation
- Called "getUrl" in place instead of detaching it at the top
#1233
9f011ef
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment