alexeyxo / protobuf-swift

Google ProtocolBuffers for Apple Swift
http://protobuf.io/#swift
Apache License 2.0
938 stars 138 forks source link

invalidProtocolBuffer "Truncated Message" #204

Closed anuragedushire closed 6 years ago

anuragedushire commented 7 years ago

Version of protoc (protoc --version)

3.1.0

Version of ProtocolBuffers.framework

3.0.6

Description

I have a protocol buffer UserDataProto which occasionally throws exception at the following code -

let userBuilder = UserDataProto.Builder() // // Do other stuff // let userData = try! userBuilder.build() let data = userData.data() let builder = UserDataProto.Builder() try! builder.mergeFrom(data: data) <-- crashes here

I thought this should 'never' happen, as I am merging something from the same type. The error says invalidProtocolBuffer "Truncated Message". Is this a bug in the library? Or, this can really happen because I am doing something odd with the protobuf in "// Do other stuff"?

Note: All the fields in the protobuf are either optional or repeated, in case that matters.

alexeyxo commented 7 years ago

I should to view the .proto file and what do you set in fields.

anuragedushire commented 7 years ago

Edited:

I was able to get to the root cause of this. The problem is that a call to data() on any protobuf calls serializedSize on the protobuf which memoizes the returned value. This seems fine but the problem is that the builder.build() returns the same instance everytime you call build(), and hence any subsequent changes to the protobuf is not counted when returning memoized value.

To demonstrate the problem, I am giving a very simplified version of the protobuf below with a testcode to reproduce the issue.


message UserDataProto {
  repeated TaskScore task_score = 14;
}

message TaskScore {
  optional int32 score = 3;
}

Test code -

  func testProto() {
    let builder1 = UserDataProto.Builder()
    try! builder1.taskScore.append(TaskScore.Builder().build())

    try! builder1.build().data()   // Comment this out for the test to pass

    try! builder1.taskScore.append(TaskScore.Builder().build())

    let testBuilder = UserDataProto.Builder()
    try! testBuilder.mergeFrom(data: builder1.build().data())
    XCTAssertEqual(2, testBuilder.taskScore.count)   // Test fails here
  }

In the code above test fails but does not crash because serializedSize truncates the message to a value which is still interpretable, otherwise the code would crash.

It seems like the current implementation is meant for a builder to be used only once to be built. Something which had not been the case with the java api.

Would you consider it a bug or just a wrong way that I have used the builder?

anuragedushire commented 6 years ago

Has this been fixed?