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

gRPC deserializer, multiple nio buffers with multiple messages #1449

Merged
merged 1 commit into from
Mar 19, 2021

Conversation

NiteshKant
Copy link
Collaborator

Motivation

ProtoBufSerializationProvider.ProtoDeserializer when extracting NIO buffers incorrectly reads all bytes from the buffer instead of the length of one message.

Modification

Read only the bytes representing a single message (decodedLengthOfData) while extracting the NIO buffers.

Result

gRPC deserializer works with buffers having multiple NIO buffers and multiple proto messages.

__Motivation__

`ProtoBufSerializationProvider.ProtoDeserializer` when extracting NIO buffers incorrectly reads all bytes from the buffer instead of the length of one message.

__Modification__

Read only the bytes representing a single message (`decodedLengthOfData`) while extracting the NIO buffers.

__Result__

gRPC deserializer works with buffers having multiple NIO buffers and multiple proto messages.
Copy link
Member

@idelpivnitskiy idelpivnitskiy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @NiteshKant! :)

@idelpivnitskiy idelpivnitskiy merged commit 57c7e74 into apple:main Mar 19, 2021
@@ -172,7 +172,7 @@ private static boolean isCompressed(Buffer buffer) throws SerializationException
// into it. Later, proto parser will copy data from this temporary ByteBuffer again.
// To avoid unnecessary copying, we use newCodedInputStream(buffers, lengthOfData).
final ByteBuffer[] buffers = buffer.toNioBuffers(buffer.readerIndex(),
buffer.readableBytes());
decodedLengthOfData);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: put on previous line

@NiteshKant NiteshKant deleted the proto-fix branch March 19, 2021 17:01
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.

3 participants