Crash when parsing packed `repeated uint64` field with length 0 in Swift

[sashaweiss-signal]

Hi! Thanks for maintaining Wire – I'm loving using it.

I'm using Wire to generate Swift code for proto types and parsing, and am running into a crash when deserializing that I think is a bug in the Swift ProtoReader.

Specifically, I'm using my in-code Wire-generated Swift models to assemble a type that has a [UInt64] field on it, which I'm setting to [], and serialize it. When I later try and deserialize that serialized proto, I'm hitting this fatalError:

Digging into it a little, I'm seeing just before the crash that Wire is trying to decode a repeated uint64 field:

Poking around at those breakpoints, it looks like length is 0:

There's a comment in that method that suggests that when we actually go to decode a T: UInt64 for this array, we'll set state to .tag (the original fatalError was because state was not being set to .tag). However, since length is 0 (which makes sense, since this array is empty) we'll never actually decode a UInt64, and state remains .lengthDelimited rather than getting reset to .tag.

---

I'm able to reproduce this minimally by adding the following to ProtoReaderTests.swift:

func testDecodePackedRepeatedFixedUInt64Empty() throws {
    let data = Foundation.Data(hexEncoded: """
        0A       // (Tag 1 | Length Delimited)
        00       // Length 0
    """)!

    try test(data: data) { reader in
        var values: [UInt64] = []
        try reader.decode(tag: 1) {
            try reader.decode(into: &values, encoding: .fixed)
        }

        XCTAssertEqual(values, [])
    }
}

However, if I add the following shim to ProtoReader.swift locally, it works:

So perhaps this method sould check for length == 0 and manually reset state = .tag and early-return?

Please let me know if there's more information I can provide. Thanks!

[dnkoutso]

thanks a lot for this report! Would it be possible to ask you to attempt something similar in Wire 4.5 or Wire 4.6 so we can see if this regressed at some point or it was always an issue? I will attempt to do it myself using the test you provided too but I can do so a little bit later in the week.

[lickel]

Also feel free to open a PR!

[sashaweiss-signal]

For sure – I just replayed these changes on top of the 4.5.0 and 4.5.6 tags, and the issue's there. Happy to open a PR if the change I suggested seems reasonable. (I'll plan to do that – if nothing else, I'm gonna make a fork to unblock myself for the time being.)

[lickel]

There's a number of additional lengthDelimited cases that might be impacted too?
But this fix seems reasonable.

[sashaweiss-signal]

Great! I searched through ProtoReader.swift for lengthDelimted, and the only thing I found was:

which already has its own length == 0 check.