Skip to content

Commit 012fe85

Browse files
authored
Merge pull request #8035 from JamesNK/jamesnk/int32wrapper-blocksize
Fix parsing negative Int32Value that crosses segment boundary
2 parents 397d34c + fdf4ef6 commit 012fe85

File tree

3 files changed

+47
-4
lines changed

3 files changed

+47
-4
lines changed

csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,25 @@ public void ReadWholeMessage_VaryingBlockSizes_FromSequence()
343343
}
344344
}
345345

346+
[Test]
347+
public void ReadInt32Wrapper_VariableBlockSizes()
348+
{
349+
byte[] rawBytes = new byte[] { 202, 1, 11, 8, 254, 255, 255, 255, 255, 255, 255, 255, 255, 1 };
350+
351+
for (int blockSize = 1; blockSize <= rawBytes.Length; blockSize++)
352+
{
353+
ReadOnlySequence<byte> data = ReadOnlySequenceFactory.CreateWithContent(rawBytes, blockSize);
354+
AssertReadFromParseContext(data, (ref ParseContext ctx) =>
355+
{
356+
ctx.ReadTag();
357+
358+
var value = ParsingPrimitivesWrappers.ReadInt32Wrapper(ref ctx);
359+
360+
Assert.AreEqual(-2, value);
361+
}, true);
362+
}
363+
}
364+
346365
[Test]
347366
public void ReadHugeBlob()
348367
{

csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,28 @@ public void NonDefaultSingleValues()
8787
});
8888
}
8989

90+
[Test]
91+
public void NegativeSingleValues()
92+
{
93+
var message = new TestWellKnownTypes
94+
{
95+
FloatField = -12.5f,
96+
DoubleField = -12.25d,
97+
Int32Field = -1,
98+
Int64Field = -2
99+
};
100+
101+
MessageParsingHelpers.AssertWritingMessage(message);
102+
103+
MessageParsingHelpers.AssertRoundtrip(TestWellKnownTypes.Parser, message, parsed =>
104+
{
105+
Assert.AreEqual(-12.5f, parsed.FloatField);
106+
Assert.AreEqual(-12.25d, parsed.DoubleField);
107+
Assert.AreEqual(-1, parsed.Int32Field);
108+
Assert.AreEqual(-2L, parsed.Int64Field);
109+
});
110+
}
111+
90112
[Test]
91113
public void NonNullDefaultIsPreservedThroughSerialization()
92114
{

csharp/src/Google.Protobuf/ParsingPrimitivesWrappers.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,11 @@ internal static class ParsingPrimitivesWrappers
160160

161161
internal static uint? ReadUInt32Wrapper(ref ReadOnlySpan<byte> buffer, ref ParserInternalState state)
162162
{
163-
// length:1 + tag:1 + value:5(varint32-max) = 7 bytes
164-
if (state.bufferPos + 7 <= state.bufferSize)
163+
// field=1, type=varint = tag of 8
164+
const int expectedTag = 8;
165+
// length:1 + tag:1 + value:10(varint64-max) = 12 bytes
166+
// Value can be 64 bits for negative integers
167+
if (state.bufferPos + 12 <= state.bufferSize)
165168
{
166169
// The entire wrapper message is already contained in `buffer`.
167170
int pos0 = state.bufferPos;
@@ -177,8 +180,7 @@ internal static class ParsingPrimitivesWrappers
177180
return ReadUInt32WrapperSlow(ref buffer, ref state);
178181
}
179182
int finalBufferPos = state.bufferPos + length;
180-
// field=1, type=varint = tag of 8
181-
if (buffer[state.bufferPos++] != 8)
183+
if (buffer[state.bufferPos++] != expectedTag)
182184
{
183185
state.bufferPos = pos0;
184186
return ReadUInt32WrapperSlow(ref buffer, ref state);

0 commit comments

Comments
 (0)