diff --git a/src/IonLocalSymbolTable.ts b/src/IonLocalSymbolTable.ts index 06c3b80f..fad89a67 100644 --- a/src/IonLocalSymbolTable.ts +++ b/src/IonLocalSymbolTable.ts @@ -24,7 +24,7 @@ import { getSystemSymbolTableImport } from "./IonSystemSymbolTable"; export class LocalSymbolTable { private readonly _import: Import; private readonly offset: number; - private index: SymbolIndex = {}; + private index: SymbolIndex = Object.create(null); constructor(theImport: Import | null, symbols: (string | null)[] = []) { if (theImport === null) { @@ -35,7 +35,7 @@ export class LocalSymbolTable { this.offset = this._import.offset + this._import.length; for (const symbol_ of symbols) { - this.addSymbol(symbol_); + this.assignSymbolId(symbol_); } } @@ -72,6 +72,23 @@ export class LocalSymbolTable { return symbolId; } + // Used during table initialization. Unlike `addSymbol`, `assignSymbolId` will not discard strings that are already + // in the symbol table. Ignoring duplicate symbols during table construction can cause symbol IDs that have already + // been used to encode data to become invalid. For example, if a stream uses symbols "foo", "bar", "baz" to encode + // its data even though "baz" was also defined in an imported table, discarding "baz" will cause data already encoded + // with that ID to become corrupted. + private assignSymbolId(symbol: string | null): number { + // Push the text onto the end of our array of strings no matter what + const symbolId = this.offset + this.symbols.length; + this.symbols.push(symbol); + // If this text isn't already in our index, go ahead and add it. + if (symbol !== null && this.getSymbolId(symbol) === undefined) { + this.index[symbol] = symbolId; + } + // Return the string's index in the symbol table, even if it isn't the lowest one. + return symbolId; + } + getSymbolText(symbolId: number): string | null { if (symbolId > this.maxId) { throw new Error( diff --git a/test/IonLocalSymbolTable.ts b/test/IonLocalSymbolTable.ts index d59c9d25..c0c0dc9f 100644 --- a/test/IonLocalSymbolTable.ts +++ b/test/IonLocalSymbolTable.ts @@ -16,7 +16,7 @@ import {assert} from 'chai'; import {LocalSymbolTable} from "../src/IonLocalSymbolTable"; import {Import} from "../src/IonImport"; -import {getSystemSymbolTableImport} from "../src/IonSystemSymbolTable"; +import {getSystemSymbolTable, getSystemSymbolTableImport} from "../src/IonSystemSymbolTable"; import {Catalog, defaultLocalSymbolTable, SharedSymbolTable} from "../src/Ion"; let defaultCatalog = function () { @@ -151,5 +151,18 @@ describe('Local symbol table', () => { // It is, however, a property (an accessor) on the 'Map' data type. // Asking for its symbol ID should return undefined. assert.isUndefined(symbolTable.getSymbolId("size")); + // Same test for 'toString', which isn't a 'Map' property, but exists on JS Objects instantiated with the + // `{}` literal instead of `Object.create(null)`. + assert.isUndefined(symbolTable.getSymbolId('toString')); + }); + + // See https://github.com/amzn/ion-js/issues/649 + it('Symbol tables do not discard duplicate text during instantiation (Issue #649)', () => { + const extraSymbols = ["foo", "foo", "bar", "bar"]; + const symbolTable = new LocalSymbolTable(getSystemSymbolTableImport(), extraSymbols); + assert.equal( + symbolTable.maxId, + getSystemSymbolTable().numberOfSymbols + extraSymbols.length + ); }); });