Skip to content

Commit 39383be

Browse files
committed
Fixes a binary write corruption bug introduced in 1.11.11.
1 parent ba2328e commit 39383be

2 files changed

Lines changed: 198 additions & 1 deletion

File tree

‎src/main/java/com/amazon/ion/impl/bin/IonRawBinaryWriter.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,18 @@ private void addPatchPoint(final ContainerInfo container, final long position, f
620620
}
621621
// index is now positioned on an ancestor container that has a patch point
622622
for (int i = index + 1; i <= containerIndex; i++) {
623-
containers.get(i).patchIndex = patchPointsLength++;
623+
int patchIndex = patchPointsLength++;
624+
containers.get(i).patchIndex = patchIndex;
625+
// Invalidate any stale patch point data from a previous segment.
626+
// If the ancestor's content ultimately fits in preallocated space, this slot
627+
// will never be overwritten, so we must ensure stale data with a positive length
628+
// is not mistakenly applied during finish().
629+
if (patchIndex < patchPoints.size()) {
630+
PatchPoint existing = patchPoints.get(patchIndex);
631+
if (existing != null) {
632+
existing.initialize(-1, -1, -1);
633+
}
634+
}
624635
}
625636
}
626637

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
package com.amazon.ion.impl.bin;
4+
5+
import com.amazon.ion.IonReader;
6+
import com.amazon.ion.IonType;
7+
import com.amazon.ion.SymbolToken;
8+
import com.amazon.ion.SystemSymbols;
9+
import com.amazon.ion.Timestamp;
10+
import com.amazon.ion.impl.bin.AbstractIonWriter.WriteValueOptimization;
11+
import com.amazon.ion.impl.bin.IonRawBinaryWriter.PreallocationMode;
12+
import com.amazon.ion.impl.bin.IonRawBinaryWriter.StreamCloseMode;
13+
import com.amazon.ion.impl.bin.IonRawBinaryWriter.StreamFlushMode;
14+
import com.amazon.ion.system.IonReaderBuilder;
15+
import org.junit.jupiter.params.ParameterizedTest;
16+
import org.junit.jupiter.params.provider.EnumSource;
17+
18+
import java.io.ByteArrayOutputStream;
19+
import java.io.IOException;
20+
import java.math.BigDecimal;
21+
import java.math.BigInteger;
22+
23+
import static org.junit.jupiter.api.Assertions.assertEquals;
24+
import static org.junit.jupiter.api.Assertions.assertNotNull;
25+
import static org.junit.jupiter.api.Assertions.assertNull;
26+
27+
/**
28+
* Regression test for a bug where stale PatchPoint data from a previous stream segment
29+
* could corrupt output in subsequent segments when the writer is reused.
30+
*/
31+
class IonRawBinaryWriterStalePatchPointTest {
32+
33+
/**
34+
* Creates a raw binary writer with the given preallocation mode.
35+
*/
36+
private IonRawBinaryWriter createWriter(ByteArrayOutputStream out, PreallocationMode mode) throws IOException {
37+
return new IonRawBinaryWriter(
38+
BlockAllocatorProviders.basicProvider(),
39+
11, // Arbitrary (does not affect the test)
40+
out,
41+
WriteValueOptimization.NONE,
42+
StreamCloseMode.NO_CLOSE,
43+
StreamFlushMode.NO_FLUSH,
44+
mode,
45+
true,
46+
false,
47+
null
48+
);
49+
}
50+
51+
@ParameterizedTest
52+
@EnumSource(PreallocationMode.class)
53+
void stalePatchPointFromLargeContainerCorruptsContainerInNextSegment(PreallocationMode mode) throws IOException {
54+
ByteArrayOutputStream out = new ByteArrayOutputStream();
55+
IonRawBinaryWriter writer = createWriter(out, mode);
56+
57+
// Segment 1: list with large list content
58+
// The inner list content exceeds contentMaxLength, causing addPatchPoint to be called.
59+
// The outer list also exceeds contentMaxLength (since it contains the inner list), so it
60+
// calls addPatchPoint in its own popContainer, constructing a PatchPoint at index 0.
61+
writer.writeIonVersionMarker();
62+
writer.stepIn(IonType.LIST);
63+
{
64+
writer.stepIn(IonType.LIST);
65+
{
66+
// Force the list to exceed patch thresholds for both PREALLOCATE_2 (16K) and PREALLOCATE_1 (127)
67+
for (int i = 0; i < (17_000 / 26); i++) {
68+
writer.writeString("abcdefghijklmnopqrstuvwxyz");
69+
}
70+
}
71+
writer.stepOut();
72+
}
73+
writer.stepOut();
74+
writer.finish();
75+
76+
// Segment 2: list with a decimal that has >13 bytes content
77+
// The decimal's encoding exceeds 13 bytes, triggering patchSingleByteTypedOptimisticValue's addPatchPoint path.
78+
// The list's total content fits within preallocated space and does not call addPatchPoint itself. The list's
79+
// patchIndex (assigned by the decimal's ancestor loop) points to the stale PatchPoint from segment 1. Without
80+
// the fix, finish() applies the stale data and corrupts the output.
81+
out.reset();
82+
writer.writeIonVersionMarker();
83+
writer.stepIn(IonType.LIST);
84+
{
85+
writer.writeDecimal(new BigDecimal(new BigInteger("123456789012345678901234567890"), 20));
86+
}
87+
writer.stepOut();
88+
writer.writeInt(123);
89+
writer.finish();
90+
91+
byte[] cycle2Bytes = out.toByteArray();
92+
93+
// Verify we can read the correct data from segment 2
94+
IonReader reader = IonReaderBuilder.standard().build(cycle2Bytes);
95+
assertEquals(IonType.LIST, reader.next());
96+
reader.stepIn();
97+
assertEquals(IonType.DECIMAL, reader.next());
98+
assertNotNull(reader.bigDecimalValue());
99+
assertNull(reader.next());
100+
reader.stepOut();
101+
assertEquals(IonType.INT, reader.next());
102+
assertEquals(123, reader.intValue());
103+
assertNull(reader.next());
104+
105+
reader.close();
106+
writer.close();
107+
}
108+
109+
private void writeLocalSymbolTable(IonRawBinaryWriter writer) throws IOException {
110+
writer.setTypeAnnotationSymbols(SystemSymbols.ION_SYMBOL_TABLE_SID);
111+
writer.stepIn(IonType.STRUCT);
112+
{
113+
writer.setFieldNameSymbol(SystemSymbols.SYMBOLS_SID);
114+
writer.stepIn(IonType.LIST);
115+
{
116+
writer.writeString("sym10");
117+
writer.writeString("sym11");
118+
writer.writeString("sym12");
119+
}
120+
writer.stepOut();
121+
}
122+
writer.stepOut();
123+
}
124+
125+
@ParameterizedTest
126+
@EnumSource(PreallocationMode.class)
127+
void stalePatchPointFromLargeContainerCorruptsAnnotatedContainerInNextSegment(PreallocationMode mode) throws IOException {
128+
ByteArrayOutputStream out = new ByteArrayOutputStream();
129+
IonRawBinaryWriter writer = createWriter(out, mode);
130+
131+
// Segment 1: Large annotated nested structure
132+
writer.writeIonVersionMarker();
133+
writeLocalSymbolTable(writer);
134+
writer.setTypeAnnotationSymbols(10);
135+
writer.stepIn(IonType.STRUCT);
136+
{
137+
writer.setFieldNameSymbol(11);
138+
writer.setTypeAnnotationSymbols(12);
139+
writer.stepIn(IonType.LIST);
140+
{
141+
// Force the list to exceed patch thresholds for both PREALLOCATE_2 (16K) and PREALLOCATE_1 (127)
142+
for (int i = 0; i < (17_000 / 26); i++) {
143+
writer.writeString("abcdefghijklmnopqrstuvwxyz");
144+
}
145+
}
146+
writer.stepOut();
147+
}
148+
writer.stepOut();
149+
writer.finish();
150+
151+
// Segment 2: Small annotated struct with timestamp >13 bytes
152+
out.reset();
153+
writer.writeIonVersionMarker();
154+
writeLocalSymbolTable(writer);
155+
writer.setTypeAnnotationSymbols(10);
156+
writer.stepIn(IonType.STRUCT);
157+
{
158+
writer.setFieldNameSymbol(11);
159+
writer.setTypeAnnotationSymbols(12);
160+
writer.writeTimestamp(Timestamp.valueOf("2026-01-01T00:00:00.123456789012345678901234567890Z"));
161+
}
162+
writer.stepOut();
163+
writer.finish();
164+
165+
byte[] cycle2Bytes = out.toByteArray();
166+
167+
// Verify segment 2
168+
IonReader reader = IonReaderBuilder.standard().build(cycle2Bytes);
169+
assertEquals(IonType.STRUCT, reader.next());
170+
SymbolToken[] annotations = reader.getTypeAnnotationSymbols();
171+
assertNotNull(annotations);
172+
assertEquals(1, annotations.length);
173+
assertEquals("sym10", annotations[0].getText());
174+
reader.stepIn();
175+
assertEquals(IonType.TIMESTAMP, reader.next());
176+
assertEquals("sym11", reader.getFieldName());
177+
SymbolToken[] fieldAnnotations = reader.getTypeAnnotationSymbols();
178+
assertEquals(1, fieldAnnotations.length);
179+
assertEquals("sym12", fieldAnnotations[0].getText());
180+
assertNotNull(reader.timestampValue());
181+
reader.stepOut();
182+
183+
reader.close();
184+
writer.close();
185+
}
186+
}

0 commit comments

Comments
 (0)