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

Why do the generated bindings read until EOF? #196

Open
therealprof opened this issue Jul 1, 2021 · 1 comment
Open

Why do the generated bindings read until EOF? #196

therealprof opened this issue Jul 1, 2021 · 1 comment

Comments

@therealprof
Copy link
Contributor

I just noticed that the generated code does something like:

while !r.is_eof() {
    match r.next_tag(bytes) {

which means that if it is fed a buffer with multiple protobuf messages included it will parse each message, fill in the structures and then overwrite it with the next until eof is reached (hopefully exactly at a message boundary without excess bytes) and then return the last of the messages.

Wouldn't it make more sense to only check for EOF once at the beginning of each (sub-)message to return an error and then only parse the first message, allowing the BytesReader state to be reused by the application to parse more messages or gracefully ignore excess data?

@sollyucko
Copy link

Or look for a 0 tag?

https://github.com/protocolbuffers/protobuf/blob/520c601c99012101c816b6ccc89e8d6fc28fdbb8/java/core/src/main/java/com/google/protobuf/AbstractMessage.java#L421-L446

    @Override
    public BuilderType mergeFrom(
        final CodedInputStream input, final ExtensionRegistryLite extensionRegistry)
        throws IOException {
      boolean discardUnknown = input.shouldDiscardUnknownFields();
      final UnknownFieldSet.Builder unknownFields =
          discardUnknown ? null : UnknownFieldSet.newBuilder(getUnknownFields());
      while (true) {
        final int tag = input.readTag();
        if (tag == 0) {
          break;
        }

        MessageReflection.BuilderAdapter builderAdapter =
            new MessageReflection.BuilderAdapter(this);
        if (!MessageReflection.mergeFieldFrom(
            input, unknownFields, extensionRegistry, getDescriptorForType(), builderAdapter, tag)) {
          // end group tag
          break;
        }
      }
      if (unknownFields != null) {
        setUnknownFields(unknownFields.build());
      }
      return (BuilderType) this;
    }

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

No branches or pull requests

2 participants