Object with zero-length string key and Set/Map value results in error

[dumbmatter]

I got this bug report https://github.com/dumbmatter/realistic-structured-clone/issues/8 and somehow didn't notice it until now.

Here's a short script to reproduce:

const Typeson = require("typeson");
const preset = require("typeson-registry/dist/presets/builtin");

const TSON = new Typeson().register(preset);

const test = (obj) => {
    const encapsulated = TSON.encapsulate(obj);
    console.log("encapsulated", encapsulated);

    const revived = TSON.revive(encapsulated);
    console.log("revived", revived);
}

console.log("This works");
test({"": "some value"});

console.log("\nThis works");
test({"a": new Set()});

console.log("\nThis fails");
test({"": new Set()});

As you can see... "" as a key works. new Set() as a value works. But together, it produces this error.

This works
encapsulated { '': 'some value' }
revived { '': 'some value' }

This works
encapsulated { a: [], '$types': { a: 'set' } }
revived { a: Set(0) {} }

This fails
encapsulated { '': [], '$types': { '': 'set' } }
TypeError: object is not iterable (cannot read property Symbol(Symbol.iterator))
    at new Set (<anonymous>)
    at Object.revive (/home/jdscheff/projects/realistic-structured-clone/node_modules/typeson-registry/dist/presets/builtin.js:1:21697)
    at executeReviver (/home/jdscheff/projects/realistic-structured-clone/node_modules/typeson/dist/typeson-commonjs2.min.js:1:12438)
    at reducer (/home/jdscheff/projects/realistic-structured-clone/node_modules/typeson/dist/typeson-commonjs2.min.js:1:13964)
    at Array.reduce (<anonymous>)
    at _revive (/home/jdscheff/projects/realistic-structured-clone/node_modules/typeson/dist/typeson-commonjs2.min.js:1:13862)
    at Typeson.revive (/home/jdscheff/projects/realistic-structured-clone/node_modules/typeson/dist/typeson-commonjs2.min.js:1:13990)
    at test (/home/jdscheff/projects/realistic-structured-clone/bug.js:10:26)
    at Object.<anonymous> (/home/jdscheff/projects/realistic-structured-clone/bug.js:21:1)

Similar error with Map instead of Set. In a little testing with other objects, I was not able to trigger the error. Although with new Date() it also does something weird:

This works
encapsulated { '': 'some value' }
revived { '': 'some value' }

This works
encapsulated { a: 1621404266214, '$types': { a: 'date' } }
revived { a: 2021-05-19T06:04:26.214Z }

This fails
encapsulated { '': 1621404266215, '$types': { '': 'date' } }
revived Invalid Date

This is with the latest versions of typeson and typeson-registry.

[brettz9]

I've confirmed this is an issue (and in typeson core), seemingly confined to top-level objects with the empty string key (probably because the empty string is otherwise used to indicate the root object). I'm surprised this wasn't caught as we do escape even for objects with $types, but apparently got through. Unfortunately, I'm a bit too preoccupied to look into a fix for this now, and not sure when I might be able to. Should be able to review a PR, however.

[brettz9]

I've finally got around to looking into this. I've submitted a PR to fix this at https://github.com/dfahlander/typeson/pull/30 but thought it would be good to confirm it works for you before merging if possible.

There were two issues, both related to the fact that we were internally sometimes using the empty string to refer to the root object and sometimes (kind of) allowing for the empty string.

This PR now should insist that:

1. The empty string will only be present to represent the root object when the object and type are encapsulated in $, e.g., {$: {'': 1234567890}, $types: {$: {'': 'Date'}}}. (We previously used this escaping mechanism for certain scenarios, but not for the empty-string-as-root scenario.
2. Revival will not treat the normal presence of the empty string as root, but instead as a regular empty string property.

@dumbmatter : Do you think you'll have a chance to take a look relatively soon? Sorry this is quite a while later. I'd like to get the fix out soon, but I think this is unfortunately somewhat of a breaking change, so better to have another pair of eyes on it, even if you're not really familiar with the internals. If not, that's fine; I can just look to release, as it is passing the new tests, and I believe addresses the situation.

@dfahlander : This should, I think, be a breaking change given that the encapsulation format may now necessarily differ, though I expect only a subset of users will be impacted.

[dumbmatter]

I spent 5 minutes trying to get my original example code to run and I'm lost in CJS/ESM issues that I don't want to deal with right now :) regardless, I probably can't say much more than if the tests pass or not.

[dfahlander]

Thanks @brettz9 🙏

[brettz9]

Realized there was a problem with the approach in my previous PR. Experimented and then realized there was a simpler solution which also minimized format changes.

The PR also adds some more examples of the typeson format in the README so people could see more how the format was build, including escaping.

I'll just wait a little in case anyone wants to review, but basically I've just distinguished between a path for whole objects and an empty string path by escaping the empty string as '' (and then escaping '' instances as '''').