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

Some utf8 characters may become "???', if polling is used #516

Merged
merged 3 commits into from
Feb 21, 2018

Conversation

HenryOrz
Copy link
Contributor

@HenryOrz HenryOrz commented Feb 7, 2018

Hi,
I found that there is something confusing at com.corundumstudio.socketio.protocol.PacketDecoder, it may convert some utf8 characters to "???"
I create an issue

// TODO optimize
public ByteBuf preprocessJson(Integer jsonIndex, ByteBuf content) throws IOException {
    String packet = URLDecoder.decode(content.toString(CharsetUtil.UTF_8), CharsetUtil.UTF_8.name());

    if (jsonIndex != null) {
        /**
        * double escaping is required for escaped new lines because unescaping of new lines can be done safely on server-side
        * (c) socket.io.js
        *
        * @see https://github.com/Automattic/socket.io-client/blob/1.3.3/socket.io.js#L2682
        */
        packet = packet.replace("\\\\n", "\\n");

        // skip "d="
        packet = packet.substring(2);
    }
    packet = new String(packet.getBytes(CharsetUtil.ISO_8859_1), CharsetUtil.UTF_8);

    return Unpooled.wrappedBuffer(packet.getBytes(CharsetUtil.UTF_8));
}

this line is very confusing:
packet = new String(packet.getBytes(CharsetUtil.ISO_8859_1), CharsetUtil.UTF_8);

some other detail I record in the issue515

@mrniko
Copy link
Owner

mrniko commented Feb 8, 2018

@HenryOrz
Copy link
Contributor Author

HenryOrz commented Feb 8, 2018

Thanks! I'm not sure about this line.
In my test case, the message has been converted to "???" before calling this line
But I think it maybe affect in some cases.
I will try to check it

@HenryOrz
Copy link
Contributor Author

@mrniko mrniko added this to the 1.7.14 milestone Feb 21, 2018
@mrniko mrniko merged commit 6a28a63 into mrniko:master Feb 21, 2018
@mrniko
Copy link
Owner

mrniko commented Feb 21, 2018

Thank you!

@fcbflying
Copy link

@HenryOrz The polling is used, I removed the line, and it works when send message from client to server. But when sending utf8 characters from server to client, the client received '中���'. If you also have the same issue?

@fengsuxing
Copy link

@HenryOrz The polling is used, I removed the line, and it works when send message from client to server. But when sending utf8 characters from server to client, the client received '中���'. If you also have the same issue?

HAVE YOU SOLVE IT?

@zhaoxiangyu
Copy link

@HenryOrz The polling is used, I removed the line, and it works when send message from client to server. But when sending utf8 characters from server to client, the client received '中���'. If you also have the same issue?

HAVE YOU SOLVE IT?

HAVE YOU SOLVE IT?

@fcbflying
Copy link

@HenryOrz The polling is used, I removed the line, and it works when send message from client to server. But when sending utf8 characters from server to client, the client received '中���'. If you also have the same issue?

HAVE YOU SOLVE IT?

HAVE YOU SOLVE IT?

It works now, but I still rollback from v1.7.18 to v1.7.12. Because I got another bug in v1.7.18, sometimes when reconnect, it still use the previous SocketIOClient rather than create a new one.

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

5 participants