amazon-ion / ion-js

A JavaScript implementation of Amazon Ion.
http://amzn.github.io/ion-docs/
Apache License 2.0
353 stars 48 forks source link

Shared Symbol Tables resolve text to SIDs incorrectly #645

Closed zslayton closed 3 years ago

zslayton commented 3 years ago

The SharedSymbolTable class has a member field called _idsByText that contains a mapping of symbol text back to their corresponding symbol IDs:

protected readonly _idsByText: Map<string, number>;

SharedSymbolTable's implementation of getSymbolId looks like this:

getSymbolId(text: string): number | undefined {
    return this._idsByText[text];
}

Notice that it uses []-syntax to index into _idsByText.

_idsByText is a Map, not an Object, which means we should access its key value pairs using the get method to avoid accidentally referring to properties on the Map's prototype.

Using []-indexing causes SharedSymbolTable#getSymbolId to unintentionally access Map prototype properties in the event of naming collisions.

For example, running:

const sid = sst.getSymbolId("size");

when "size" is not in the symbol table should return undefined. However, Map's prototype provides a size property:

The value of size is an integer representing how many entries the Map object has. A set accessor function for size is undefined; you can not change this property.

which means that sst.getSymbolId("size") will return a plausible-looking number.

This issue intersects with #644, but both problems must be addressed independently.

zslayton commented 3 years ago

Fixed in #648.