Closed sashaweiss-signal closed 1 month ago
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.
Also feel free to open a PR!
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.)
There's a number of additional lengthDelimited
cases that might be impacted too?
But this fix seems reasonable.
Great! I searched through ProtoReader.swift
for lengthDelimted
, and the only thing I found was:
which already has its own length == 0
check.
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 thisfatalError
: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
is0
:There's a comment in that method that suggests that when we actually go to decode a
T: UInt64
for this array, we'll setstate
to.tag
(the originalfatalError
was becausestate
was not being set to.tag
). However, sincelength
is0
(which makes sense, since this array is empty) we'll never actually decode aUInt64
, andstate
remains.lengthDelimited
rather than getting reset to.tag
.I'm able to reproduce this minimally by adding the following to
ProtoReaderTests.swift
:However, if I add the following shim to
ProtoReader.swift
locally, it works:So perhaps this method sould check for
length == 0
and manually resetstate = .tag
and early-return?Please let me know if there's more information I can provide. Thanks!