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

Update PacketDecoder.java #301

Merged
merged 1 commit into from
Dec 26, 2015
Merged

Update PacketDecoder.java #301

merged 1 commit into from
Dec 26, 2015

Conversation

lpage30
Copy link
Contributor

@lpage30 lpage30 commented Dec 25, 2015

FIX for: Polling clients sporatically get prematurely disconnected #264
The issue is piggy-backed messages are getting dropped on polling, The code I removed was simply taking the 1st message and dropping the rest.
Part of that rest could've been an Pong or Ping (as well as other data), and thus causes the dropping of the polling client connection.

FIX for: Polling clients sporatically get prematurely disconnected mrniko#264
The issue is piggy-backed messages are getting dropped on polling, The code I removed was simply taking the 1st message and dropping the rest.
Part of that rest could've been an Pong or Ping (as well as other data), and thus causes the dropping of the polling client connection.
@lpage30 lpage30 closed this Dec 25, 2015
@lpage30 lpage30 reopened this Dec 25, 2015
@mrniko
Copy link
Owner

mrniko commented Dec 26, 2015

oh, thanks! I have been waiting it!

mrniko pushed a commit that referenced this pull request Dec 26, 2015
Update PacketDecoder.java
@mrniko mrniko merged commit 3a49630 into mrniko:master Dec 26, 2015
@xiukongtiao
Copy link

Hi,
@lpage30
When I run the demo netty-socketio-demo on Polling after I upgrade the netty-socketio version from 1.7.7 to 1.7.12.
I found if I send a English string like 'abc', it works good on Polling
But if I send a chinese word like '你好', it works good on Websocket, bu for polling there will be a exception:
[nioEventLoopGroup-3-8] ERROR com.corundumstudio.socketio.handler.InPacketHandler - Error during data processing. Client sessionId: 7610ac17-00dc-4d4e-848a-7da0bac5e3f1, data: :42["chatevent",{"userName":"user452","message":"你好"}]
java.lang.IllegalStateException
at com.corundumstudio.socketio.protocol.UTF8CharsScanner.getActualLength(UTF8CharsScanner.java:108)
at com.corundumstudio.socketio.protocol.PacketDecoder.decodePackets(PacketDecoder.java:137)
at com.corundumstudio.socketio.handler.InPacketHandler.channelRead0(InPacketHandler.java:65)
at com.corundumstudio.socketio.handler.InPacketHandler.channelRead0(InPacketHandler.java:36)
at io.netty.channel.SimpleChannelInboundHandler.channelRead(SimpleChannelInboundHandler.java:105)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:372)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:358)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:350)
at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:102)
at io.netty.handler.codec.MessageToMessageCodec.channelRead(MessageToMessageCodec.java:111)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:372)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:358)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:350)
at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:102)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:372)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:358)

And I have checked the code, I foud this change looks like the reason, can you help me to fonfirm it,
If I rollback this change, it worked okay to support Chinese, but I do not know if I rollback this change if there will be a new trouble?

@lpage30
Copy link
Contributor Author

lpage30 commented Feb 14, 2017

  1. Were you definitely working before WITH POLLING?
  2. nature of change was small: 52139a5#diff-c39cd8103fe1a5b9e96341e3dca5a0da
  3. only other thought is some encoding issue. The line below the change extracts the string out as
    CharsetUtil.ISO_8859_1 and then writes it as UTF-8. What encoding (if any) have you tried for your string?
    8859 does support chinese, and it was there before.

@xiukongtiao
Copy link

  1. Yes, I am very sure. just change 1.7.7 to 1.7.12 in pom of mrniko's demo (https://github.com/mrniko/netty-socketio-demo), then Chinese message does not work. after I rollback your change, it works good, You can try send "你好"。

  2. Yes, your change is very small, I have checked before your change, the part of content always be cut before ":", right? after that, only english can be decoded, I guess your change is a great change, but the decode method depends on the code you killed.

  3. UTF8 is my encoding, if I rollback your change without any othrer change, it works good!

Yes, if I rollback your change, it works, but I belive if I rollback your changed code, it means another problem I think.

Could you help me to have a look please? thank you very much!

@devpage
Copy link

devpage commented Feb 15, 2017

没有遇到过这种问题,我直接用的就是最新的release

@lpage30
Copy link
Contributor Author

lpage30 commented Feb 22, 2017

Few things to try:

  1. I am wondering about the new line 70:
    it does a getBytes(UTF8) when everywhere else there is a getBytes(CharsetUtil.ISO_8859_1).
    This could be a surgical change and test for you to try first.

  2. I am wondering what if you remove the new line at 68:
    packet = new String(packet.getBytes(CharsetUtil.ISO_8859_1), CharsetUtil.UTF_8);
    The code already decodes the string as ISO 8859 which supports simplified chinese, and before (when the line was getting split) this line was ensuring the encoding for the new substring.
    It is the only thing I can currently think of (besides changing charset).

@lpage30
Copy link
Contributor Author

lpage30 commented Feb 22, 2017

Actually the more I look at that #1 I mentioned with getBytes(UTF8) the more I think that is the issue.
its the ONLY getBytes that uses UTF8, everywhere else we use ISO 8859.....

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

Successfully merging this pull request may close these issues.

None yet

4 participants