LST symbol resolution fixes

* LSTs no longer use Objects with default properties to index symbols (Issue #649)
* LSTs no longer discard duplicate symbols during init (Issue #644)

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(

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
+        );
     });
 });