jmesnil / stomp-websocket

Stomp client for Web browsers and node.js apps
http://jmesnil.net/stomp-websocket/doc/
Apache License 2.0
1.44k stars 587 forks source link

PR #77 breaks IE8+SockJS Support #95

Open friscoMad opened 9 years ago

friscoMad commented 9 years ago

I am not fully sure on the internals of the issue but I think that SockJS JSON.parse on IE8 removes the \0 character from the frame and the new partial message support adds new controls so it does not consider the data as a full frame so it will keep waiting even if the frame was fully sent.

I understand that the issue is not within StompJS but I am not sure if SockJS can fix the issue, and it seems this was working previously.

I have seen this issue with a Spring Boot 1.2.0 application and IE8, the result is that the connection callback is never called so I can not subscribe to topics.

friscoMad commented 9 years ago

Reviewing the code to get some more info for SockJS developers I have found the issue.

Split on IE<9 does not return empty values at the start or end so even if the frame is full (line 92 of compiled code)

frames = datas.split(RegExp("" + Byte.NULL + Byte.LF + "*"));

does not return the expected [frame, ""] array it returns [frame] and the frame without trailing \0 goes throught the last_frame code, which I can not understand as it checks for a \n or a full frame (this is not possible as it would have been splitted before) but our stripped down frame (without \0) can not sattisfy so it is considered a partial response.

I would suggest checking the ending of the datas var to see if it ends with a complete frame or not and then treat the last_frame accordingly.

friscoMad commented 9 years ago

I finally ended using http://xregexp.com/ polyfill to work around IE<9 inconsistencies, I don't think it is a great solution but I didn't wanted to mess in stomp code and I don't see anyone caring about this issue.

aidanwhiteley commented 9 years ago

Hi - just to confirm that I saw exactly the same problem using Spring Boot 1.2.1, stomp-websocket 2.3.4 and sock.js 0.3.4. The symptoms in IE8 were that messages could be sent OK (and received by other browsers) but the IE8 browser didn't receive any messages. It just sat there i.e. it didn't trigger any of the error handling code which seems to tie up with the analysis above. Taking the fix up code from http://blog.stevenlevithan.com/archives/cross-browser-split (which is now in the xregexp package) sorted it for me with a conditional include for IE8.

Many thanks to frisco82 for the analysis above - you saved me from a lot of hair pulling!

friscoMad commented 9 years ago

No probs, it took me some time to debug the issue but it was clear when I saw the changes. The sad part is that there has been no fix for this issue for 2 months fur such a big regression. This is a de facto standard for a high level api over ws and being pushed by Spring I thought it will get some more love.

It should be easy to fix but some tests are needed and I don't have the setup to do it.

mreed123 commented 9 years ago

I see the issue as well with IE8, must support IE8-11 and other browsers with this product. Will try the fix mentioned, thanks!