quick-protobuf icon indicating copy to clipboard operation
quick-protobuf copied to clipboard

Why do the generated bindings read until EOF?

Open therealprof opened this issue 4 years ago • 1 comments

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?

therealprof avatar Jul 01 '21 14:07 therealprof

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;
    }

sollyucko avatar Jun 28 '22 22:06 sollyucko