diff --git a/src/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java b/src/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java index f9d2489bd0..9fe4e334b8 100644 --- a/src/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java +++ b/src/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java @@ -214,7 +214,7 @@ private SymbolTable getSystemSymbolTable() { /** * Read-only snapshot of the local symbol table at the reader's current position. */ - private class LocalSymbolTableSnapshot implements SymbolTable, SymbolTableAsStruct { + private class LocalSymbolTableSnapshot implements _Private_LocalSymbolTable, SymbolTableAsStruct { // The system symbol table. private final SymbolTable system = IonReaderContinuableApplicationBinary.this.getSystemSymbolTable(); @@ -425,6 +425,17 @@ public IonStruct getIonRepresentation(ValueFactory valueFactory) { } return structCache.getIonRepresentation(valueFactory); } + + @Override + public _Private_LocalSymbolTable makeCopy() { + // This is a mutable copy. LocalSymbolTable handles the mutability concerns. + return new LocalSymbolTable(importedTables, Arrays.asList(idToText)); + } + + @Override + public SymbolTable[] getImportedTablesNoCopy() { + return importedTables.getImportedTablesNoCopy(); + } } /** @@ -544,7 +555,7 @@ String getSymbol(int sid) { } int localSymbolOffset = sid - firstLocalSymbolId; if (localSymbolOffset > localSymbolMaxOffset) { - throw new IonException("Symbol ID exceeds the max ID of the symbol table."); + throw new UnknownSymbolException(sid); } return symbols[localSymbolOffset]; } @@ -565,7 +576,7 @@ private SymbolToken getSymbolToken(int sid) { } } if (sid >= symbolTableSize) { - throw new IonException("Symbol ID exceeds the max ID of the symbol table."); + throw new UnknownSymbolException(sid); } SymbolToken token = symbolTokensById.get(sid); if (token == null) { diff --git a/src/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java b/src/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java index dcce7d6a23..8e06fafe14 100644 --- a/src/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java +++ b/src/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java @@ -97,6 +97,11 @@ public SymbolTable pop_passed_symbol_table() { return null; } symbolTableLastTransferred = currentSymbolTable; + if (symbolTableLastTransferred.isLocalTable()) { + // This method is called when transferring the reader's symbol table to either a writer or an IonDatagram. + // Those cases require a mutable copy of the reader's symbol table. + return ((_Private_LocalSymbolTable) symbolTableLastTransferred).makeCopy(); + } return symbolTableLastTransferred; } diff --git a/src/com/amazon/ion/impl/LocalSymbolTable.java b/src/com/amazon/ion/impl/LocalSymbolTable.java index 56198844c1..cc6ca89497 100644 --- a/src/com/amazon/ion/impl/LocalSymbolTable.java +++ b/src/com/amazon/ion/impl/LocalSymbolTable.java @@ -51,7 +51,7 @@ * Instances of this class are safe for use by multiple threads. */ class LocalSymbolTable - implements SymbolTable + implements _Private_LocalSymbolTable { static class Factory implements _Private_LocalSymbolTableFactory @@ -329,7 +329,8 @@ else if (fieldType == IonType.SYMBOL && ION_SYMBOL_TABLE.equals(reader.stringVal return new LocalSymbolTableImports(importsList); } - synchronized LocalSymbolTable makeCopy() + @Override + public synchronized _Private_LocalSymbolTable makeCopy() { return new LocalSymbolTable(this, getMaxId()); } @@ -604,19 +605,8 @@ public SymbolTable[] getImportedTables() return myImportsList.getImportedTables(); } - /** - * Returns the imported symbol tables without making a copy. - *

- * Note: Callers must not modify the resulting SymbolTable array! - * This will violate the immutability property of this class. - * - * @return - * the imported symtabs, as-is; the first element is a system - * symtab, the rest are non-system shared symtabs - * - * @see #getImportedTables() - */ - SymbolTable[] getImportedTablesNoCopy() + @Override + public SymbolTable[] getImportedTablesNoCopy() { return myImportsList.getImportedTablesNoCopy(); } diff --git a/src/com/amazon/ion/impl/_Private_IonBinaryWriterBuilder.java b/src/com/amazon/ion/impl/_Private_IonBinaryWriterBuilder.java index 80f9d8d1dd..367f37a068 100644 --- a/src/com/amazon/ion/impl/_Private_IonBinaryWriterBuilder.java +++ b/src/com/amazon/ion/impl/_Private_IonBinaryWriterBuilder.java @@ -157,7 +157,7 @@ public void setInitialSymbolTable(SymbolTable symtab) if (symtab.isLocalTable()) { SymbolTable[] imports = - ((LocalSymbolTable) symtab).getImportedTablesNoCopy(); + ((_Private_LocalSymbolTable) symtab).getImportedTablesNoCopy(); for (SymbolTable imported : imports) { if (imported.isSubstitute()) @@ -353,7 +353,7 @@ SymbolTable buildContextSymbolTable() return myInitialSymbolTable; } - return ((LocalSymbolTable) myInitialSymbolTable).makeCopy(); + return ((_Private_LocalSymbolTable) myInitialSymbolTable).makeCopy(); } diff --git a/src/com/amazon/ion/impl/_Private_LocalSymbolTable.java b/src/com/amazon/ion/impl/_Private_LocalSymbolTable.java new file mode 100644 index 0000000000..f900fd0c83 --- /dev/null +++ b/src/com/amazon/ion/impl/_Private_LocalSymbolTable.java @@ -0,0 +1,25 @@ +package com.amazon.ion.impl; + +import com.amazon.ion.SymbolTable; + +interface _Private_LocalSymbolTable extends SymbolTable { + + /** + * @return a mutable copy of the symbol table. + */ + _Private_LocalSymbolTable makeCopy(); + + /** + * Returns the imported symbol tables without making a copy. + *

+ * Note: Callers must not modify the resulting SymbolTable array! + * This will violate the immutability property of this class. + * + * @return + * the imported symtabs, as-is; the first element is a system + * symtab, the rest are non-system shared symtabs + * + * @see SymbolTable#getImportedTables() + */ + SymbolTable[] getImportedTablesNoCopy(); +} diff --git a/src/com/amazon/ion/impl/_Private_Utils.java b/src/com/amazon/ion/impl/_Private_Utils.java index be806aeb72..ce68c35a19 100644 --- a/src/com/amazon/ion/impl/_Private_Utils.java +++ b/src/com/amazon/ion/impl/_Private_Utils.java @@ -800,7 +800,7 @@ public static SymbolTable copyLocalSymbolTable(SymbolTable symtab) } SymbolTable[] imports = - ((LocalSymbolTable) symtab).getImportedTablesNoCopy(); + ((_Private_LocalSymbolTable) symtab).getImportedTablesNoCopy(); // Iterate over each import, we assume that the list of imports // rarely exceeds 5. @@ -816,7 +816,7 @@ public static SymbolTable copyLocalSymbolTable(SymbolTable symtab) } } - return ((LocalSymbolTable) symtab).makeCopy(); + return ((_Private_LocalSymbolTable) symtab).makeCopy(); } /** diff --git a/test/com/amazon/ion/DatagramTest.java b/test/com/amazon/ion/DatagramTest.java index 493efb7eda..831ccfd036 100644 --- a/test/com/amazon/ion/DatagramTest.java +++ b/test/com/amazon/ion/DatagramTest.java @@ -28,8 +28,11 @@ import com.amazon.ion.impl._Private_IonValue; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Collection; import java.util.Iterator; + +import com.amazon.ion.system.IonReaderBuilder; import org.junit.Before; import org.junit.Test; @@ -720,4 +723,25 @@ public void testSetWithSymbolTable() assertEquals(yID + 1, v1.symbolValue().getSid()); //now x has a mapping in the existing symbol table } + + @Test + public void modifySymbolsAfterLoadThenSerialize() + throws Exception + { + IonDatagram dg = system().getLoader().load(TestUtils.ensureBinary(system(), "{foo: bar::baz}".getBytes(StandardCharsets.UTF_8))); + IonStruct struct = (IonStruct) dg.get(0); + IonSymbol baz = (IonSymbol) struct.get("foo"); + baz.setTypeAnnotations("bar", "abc"); + byte[] serialized = dg.getBytes(); + try (IonReader reader = IonReaderBuilder.standard().build(serialized)) { + assertEquals(IonType.STRUCT, reader.next()); + reader.stepIn(); + assertEquals(IonType.SYMBOL, reader.next()); + assertEquals("foo", reader.getFieldName()); + String[] annotations = reader.getTypeAnnotations(); + assertEquals(2, annotations.length); + assertEquals("bar", annotations[0]); + assertEquals("abc", annotations[1]); + } + } } diff --git a/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java b/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java index b7422c473d..ff41c7a785 100644 --- a/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java +++ b/test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java @@ -23,6 +23,7 @@ import com.amazon.ion.SystemSymbols; import com.amazon.ion.TestUtils; import com.amazon.ion.Timestamp; +import com.amazon.ion.UnknownSymbolException; import com.amazon.ion.impl.bin._Private_IonManagedBinaryWriterBuilder; import com.amazon.ion.impl.bin._Private_IonManagedWriter; import com.amazon.ion.impl.bin._Private_IonRawWriter; @@ -769,6 +770,36 @@ public void invalidVersionMarker(boolean constructFromBytes) throws Exception { }); } + @ParameterizedTest(name = "constructFromBytes={0}") + @ValueSource(booleans = {true, false}) + public void unknownSymbolInFieldName(boolean constructFromBytes) throws Exception { + reader = readerFor(constructFromBytes, 0xD3, 0x8A, 0x21, 0x01); + assertSequence(next(IonType.STRUCT), STEP_IN, next(IonType.INT)); + assertThrows(UnknownSymbolException.class, reader::getFieldNameSymbol); + assertThrows(UnknownSymbolException.class, reader::getFieldName); + reader.close(); + } + + @ParameterizedTest(name = "constructFromBytes={0}") + @ValueSource(booleans = {true, false}) + public void unknownSymbolInAnnotation(boolean constructFromBytes) throws Exception { + reader = readerFor(constructFromBytes, 0xE4, 0x81, 0x8A, 0x21, 0x01); + assertSequence(next(IonType.INT)); + assertThrows(UnknownSymbolException.class, reader::getTypeAnnotationSymbols); + assertThrows(UnknownSymbolException.class, reader::getTypeAnnotations); + reader.close(); + } + + @ParameterizedTest(name = "constructFromBytes={0}") + @ValueSource(booleans = {true, false}) + public void unknownSymbolInValue(boolean constructFromBytes) throws Exception { + reader = readerFor(constructFromBytes, 0x71, 0x0A); + assertSequence(next(IonType.SYMBOL)); + assertThrows(UnknownSymbolException.class, reader::symbolValue); + assertThrows(UnknownSymbolException.class, reader::stringValue); + reader.close(); + } + /** * Feeds all bytes from the given array into the pipe one-by-one, asserting before each byte that the reader * is not positioned on a value. diff --git a/test/com/amazon/ion/system/IonBinaryWriterBuilderTest.java b/test/com/amazon/ion/system/IonBinaryWriterBuilderTest.java index 410ac4509e..f27c6eb381 100644 --- a/test/com/amazon/ion/system/IonBinaryWriterBuilderTest.java +++ b/test/com/amazon/ion/system/IonBinaryWriterBuilderTest.java @@ -24,9 +24,12 @@ import static org.junit.Assert.fail; import com.amazon.ion.IonCatalog; +import com.amazon.ion.IonReader; import com.amazon.ion.IonSystem; +import com.amazon.ion.IonType; import com.amazon.ion.IonWriter; import com.amazon.ion.SymbolTable; +import com.amazon.ion.TestUtils; import com.amazon.ion.impl.Symtabs; import com.amazon.ion.impl._Private_IonBinaryWriterBuilder; import com.amazon.ion.impl._Private_IonWriter; @@ -34,6 +37,8 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.nio.charset.StandardCharsets; + import org.junit.Assert; import org.junit.Test; @@ -299,6 +304,40 @@ public void testImmutableInitialSymtab() assertTrue(symbolTableEquals(lst, writer.getSymbolTable())); } + @Test + public void testInitialSymtabFromReader() + throws Exception + { + SymbolTable lst; + try (IonReader reader = IonReaderBuilder.standard().build(TestUtils.ensureBinary(IonSystemBuilder.standard().build(), "foo".getBytes(StandardCharsets.UTF_8)))) { + assertEquals(IonType.SYMBOL, reader.next()); + lst = reader.getSymbolTable(); + } + + _Private_IonBinaryWriterBuilder b = + _Private_IonBinaryWriterBuilder.standard(); + b.setInitialSymbolTable(lst); + assertSame(lst, b.getInitialSymbolTable()); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + IonWriter writer = b.build(out); + assertTrue(symbolTableEquals(lst, writer.getSymbolTable())); + + writer = b.build(out); + assertTrue(symbolTableEquals(lst, writer.getSymbolTable())); + + writer.writeSymbol("bar"); + writer.writeSymbol("foo"); + writer.close(); + + try (IonReader reader = IonReaderBuilder.standard().build(out.toByteArray())) { + assertEquals(IonType.SYMBOL, reader.next()); + assertEquals("bar", reader.stringValue()); + assertEquals(IonType.SYMBOL, reader.next()); + assertEquals("foo", reader.stringValue()); + } + } + @Test(expected = UnsupportedOperationException.class) public void testInitialSymtabImmutability() {