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

JDKZlibEncoder or JDKZlibDecoder can't work correct with large data #6804

Closed
lsqlebai opened this issue Jun 2, 2017 · 8 comments
Closed
Assignees
Labels

Comments

@lsqlebai
Copy link

lsqlebai commented Jun 2, 2017

Expected behavior

work

Actual behavior

decode error(protobuf decode error)

Steps to reproduce

i have 2 server. and them communicate with tcp.
and the pipeline init code double with them.
when the send data > 16000 byte (maybe less). the decode side can't correct decode (protobuf decode error).

Minimal yet complete reproducer code (or URL to code)

decode error code:
Code:

    ChannelPipeline pipeline = ch.pipeline();
    pipeline.addLast(new ProtobufVarint32FrameDecoder());
    pipeline.addLast(ZlibCodecFactory.newZlibDecoder(ZlibWrapper.NONE));
    pipeline.addLast(new ProtobufDecoder(GameServer.ServerData.getDefaultInstance()));
    pipeline.addLast(new ProtobufVarint32LengthFieldPrepender());
    pipeline.addLast(ZlibCodecFactory.newZlibEncoder(ZlibWrapper.NONE));
    pipeline.addLast(new ProtobufEncoder());

i had 2 test:
remove zlib, can be work.like this:

  ChannelPipeline pipeline = ch.pipeline();
    //receive
    pipeline.addLast(new ProtobufVarint32FrameDecoder());
    pipeline.addLast(new ProtobufDecoder(GameServer.ServerData.getDefaultInstance()));
    //send
    pipeline.addLast(new ProtobufVarint32LengthFieldPrepender());
    pipeline.addLast(new ProtobufEncoder());

change to the JZlibDecoder and JZlibEncoder can be work. like this:

    ChannelPipeline pipeline = ch.pipeline();
    //receive
    pipeline.addLast(new ProtobufVarint32FrameDecoder());
    pipeline.addLast(new JZlibDecoder(ZlibWrapper.NONE));
    pipeline.addLast(new ProtobufDecoder(GameServer.ServerData.getDefaultInstance()));
    //send
    pipeline.addLast(new ProtobufVarint32LengthFieldPrepender());
    pipeline.addLast(new JZlibEncoder(ZlibWrapper.NONE));
    pipeline.addLast(new ProtobufEncoder());

Netty version

4.1.7

JVM version (e.g. java -version)

java version "1.8.0_101"

OS version (e.g. uname -a)

windows

@normanmaurer
Copy link
Member

@lsqlebai can you please provide a full reproducer ? Also please update to latest netty version just to be sure.

@lsqlebai
Copy link
Author

lsqlebai commented Jun 2, 2017

sorry the netty version is 4.1.6.Final

@lsqlebai
Copy link
Author

lsqlebai commented Jun 2, 2017

@normanmaurer What would you want? my project is so large. maybe i can give u a demo when i free.

@normanmaurer
Copy link
Member

@lsqlebai I want you to upgrade to latest 4.1. version and try again if it happens. If it still happens provide me some way to reproduce.

@Scottmitch
Copy link
Member

my project is so large. maybe i can give u a demo when i free.

Ideally folks that file issues will provide a minimal/complete (so we can run it, but don't have to understand your entire application logic) ... the easier it is to reproduce the more likely we will be able to figure out the issue (assuming its a Netty issue) and fix it.

@lsqlebai
Copy link
Author

lsqlebai commented Jun 4, 2017

@Scottmitch @normanmaurer i wrote a demo, how can i give you? by email or???

@lsqlebai
Copy link
Author

lsqlebai commented Jun 5, 2017

@Scottmitch @normanmaurer would u please try this code: https://github.com/lsqlebai/testzlib
that can be reproduce .

Scottmitch added a commit to Scottmitch/netty that referenced this issue Jun 5, 2017
Motivation:
JdkZlibDecoder will allocate a new buffer when the previous buffer is filled with inflated data, but JZlibDecoder will attempt to use the same buffer by resizing. This leads to inconsistent results when these two decoders that are intended to be functionality equivalent.

Modifications:
- JdkZlibDecoder should attempt to resize and reuse the existing buffer instead of creating multiple buffers

Result:
Fixes netty#6804
@Scottmitch
Copy link
Member

I took a brief look. It seems like the issue is in this case the JdkZlibDecoder happens to break the decode up into 2 buffers and the JZlibDecoder decodes into a single buffer, and the ProtobufDecoder#decoder assumes all content is in the same ByteBuf. See #6816 for a PR which makes the two decoders consistent.

@Scottmitch Scottmitch self-assigned this Jun 5, 2017
Scottmitch added a commit that referenced this issue Jun 6, 2017
Motivation:
JdkZlibDecoder will allocate a new buffer when the previous buffer is filled with inflated data, but JZlibDecoder will attempt to use the same buffer by resizing. This leads to inconsistent results when these two decoders that are intended to be functionality equivalent.

Modifications:
- JdkZlibDecoder should attempt to resize and reuse the existing buffer instead of creating multiple buffers

Result:
Fixes #6804
liuzhengyang pushed a commit to liuzhengyang/netty that referenced this issue Sep 10, 2017
Motivation:
JdkZlibDecoder will allocate a new buffer when the previous buffer is filled with inflated data, but JZlibDecoder will attempt to use the same buffer by resizing. This leads to inconsistent results when these two decoders that are intended to be functionality equivalent.

Modifications:
- JdkZlibDecoder should attempt to resize and reuse the existing buffer instead of creating multiple buffers

Result:
Fixes netty#6804
kiril-me pushed a commit to kiril-me/netty that referenced this issue Feb 28, 2018
Motivation:
JdkZlibDecoder will allocate a new buffer when the previous buffer is filled with inflated data, but JZlibDecoder will attempt to use the same buffer by resizing. This leads to inconsistent results when these two decoders that are intended to be functionality equivalent.

Modifications:
- JdkZlibDecoder should attempt to resize and reuse the existing buffer instead of creating multiple buffers

Result:
Fixes netty#6804
pulllock pushed a commit to pulllock/netty that referenced this issue Oct 19, 2023
Motivation:
JdkZlibDecoder will allocate a new buffer when the previous buffer is filled with inflated data, but JZlibDecoder will attempt to use the same buffer by resizing. This leads to inconsistent results when these two decoders that are intended to be functionality equivalent.

Modifications:
- JdkZlibDecoder should attempt to resize and reuse the existing buffer instead of creating multiple buffers

Result:
Fixes netty#6804
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants