Repeated messages with protobuf id >= 16 are packed incorrectly

[dixonjoel]

RepeatedMessages.zip

In the attached project, I've generated a client and server from the included 'repeated.proto'. It includes a nested message like this:

// Nested repeated message
message RepeatedMessage
{
    repeated SimpleMessage field_id_15 = 15;
    repeated SimpleMessage field_id_16 = 16;
}

To reproduce, run the TestRepeatedMessage_server.Run Service.vi and then run TestPacking.vi in the project.

Inspecting the resulting Any from TestPacking.vi, field_id_15 and field_id_16 are serialized differently. In the case of field_id_16, there is one byte left off of the end (the '64' byte at index 19 from field_id_15).

Compare the serialized bytes from the two fields:

It appears that because the field id takes two bytes to serialize, grpc-labview is off-by-one on its byte count. These are the likely code error candidates that we've identified:

grpc-labview/src/lv_message_value.cc

Line 55 in 0f954d4

totalSize += 1UL * _value.size();

This code appears to always allocated 1 byte for the field instead of something like this that calculates the tag value size:

grpc-labview/src/lv_message_value.cc

Line 32 in 0f954d4

return WireFormatLite::TagSize(_protobufId, WireFormatLite::TYPE_MESSAGE) + WireFormatLite::MessageSize(*_value);

Software:
Using grpc-labview 1.2.0.1 and LabVIEW 2020 SP1

AB#2679961

[ni-sujain]

@dixonjoel I tried debugging this. While I agree that line 55 grpc-labview/src/lv_message_value.cc calculates it wrong and it needs to use TagSize() method, even after fixing that I still get one byte extra for the field_id_16. Its due to nature of grpc serialization, I guess.

I also saw that in grpc protobuf documentation its mentioned that it will take two bytes for field index 16 and above, that is why we see extra byte.

Do you have an example outside of grpc-labview which serializes field_id_15 and field_id_16 with same number of bytes?

[dixonjoel]

@dixonjoel I tried debugging this. While I agree that line 55 grpc-labview/src/lv_message_value.cc calculates it wrong and it needs to use TagSize() method, even after fixing that I still get one byte extra for the field_id_16. Its due to nature of grpc serialization, I guess.

I also saw that in grpc protobuf documentation its mentioned that it will take two bytes for field index 16 and above, that is why we see extra byte.

Do you have an example outside of grpc-labview which serializes field_id_15 and field_id_16 with same number of bytes?

@ni-sujain That is correct and expected. The field index does take an extra byte. And this part of the serialization is already correct. As you can see in the screenshots, the byte '122' for field index 15 and the bytes '130' and '1' for field index 16 are correct at the beginning. The problem lies at the end of the buffer. In the field index 15 case, the last byte is '64' whereas in the field index 16 case, the last byte is '0'. There should be an additional '64' at the end and that buffer should be 1 byte longer overall. So with your change, you may already be able to see the last byte as '64' which would indicate your change fixed the bug.

[nischalks]

This is now available as part of this release https://github.com/ni/grpc-labview/releases/tag/v1.2.2.1