diff --git a/packages/marshal/src/marshal-justin.js b/packages/marshal/src/marshal-justin.js index 9f1760ec21f..2c50da8c121 100644 --- a/packages/marshal/src/marshal-justin.js +++ b/packages/marshal/src/marshal-justin.js @@ -6,7 +6,7 @@ import { Nat } from '@agoric/nat'; import { assert, details as X, q } from '@agoric/assert'; import { QCLASS, getErrorConstructor } from './marshal'; -// import { makeReviverIbidTable } from './ibidTables'; +import { makeReviverIbidTable } from './ibidTables'; import './types'; @@ -31,16 +31,30 @@ const { stringify: quote } = JSON; const makeYesIndenter = () => { const strings = []; let level = 0; - const line = () => strings.push('\n', ' '.repeat(level)); + let needSpace = false; + const line = () => { + needSpace = false; + return strings.push('\n', ' '.repeat(level)); + }; return harden({ open: openBracket => { - assert(level >= 1); level += 1; - return strings.push(' ', openBracket); + if (needSpace) { + strings.push(' '); + } + needSpace = false; + return strings.push(openBracket); }, line, - next: token => strings.push(' ', token), + next: token => { + if (needSpace && token !== ',') { + strings.push(' '); + } + needSpace = true; + return strings.push(token); + }, close: closeBracket => { + assert(level >= 1); level -= 1; line(); return strings.push(closeBracket); @@ -96,60 +110,30 @@ const makeNoIndenter = () => { }); }; -const registerIbid = _rawTree => { - // doesn't do anything yet. -}; - -const startIbid = _rawTree => { - // doesn't do anything yet. -}; - -const finishIbid = _rawTree => { - // doesn't do anything yet. -}; - const identPattern = /^[a-zA-Z]\w*$/; /** * @param {Encoding} encoding - * @param {CyclePolicy} _cyclePolicy * @param {boolean=} shouldIndent * @returns {string} */ -const decodeToJustin = (encoding, _cyclePolicy, shouldIndent = false) => { - // const ibidTable = makeReviverIbidTable(cyclePolicy); - const makeIndenter = shouldIndent ? makeYesIndenter : makeNoIndenter; - const out = makeIndenter(); - - const decodeProperty = (name, value) => { - out.line(); - assert.typeof(name, 'string', X`Property name ${name} of must be a string`); - if (name === '__proto__') { - // JavaScript interprets `{__proto__: x, ...}` - // as making an object inheriting from `x`, whereas - // in JSON it is simply a property name. Preserve the - // JSON meaning. - out.next(`["__proto__"]:`); - } else if (identPattern.test(name)) { - out.next(`${name}:`); - } else { - out.next(`${quote(name)}:`); - } - // eslint-disable-next-line no-use-before-define - recur(value); - out.next(','); - }; +const decodeToJustin = (encoding, shouldIndent = false) => { + const ibidTable = makeReviverIbidTable('forbidCycles'); + const ibidMap = new Map(); /** - * Modeled after `fullRevive` in marshal.js + * The first pass populates ibidMap for use by `decode`. + * Since this is the first pass, it should do all input validation. + * Its control flow should mirror `recur` as closely as possible + * and the two should be maintained together. They must visit everything + * in the same order. * * @param {Encoding} rawTree - * @returns {number} + * @returns {void} */ - const recur = rawTree => { + const prepare = rawTree => { if (Object(rawTree) !== rawTree) { - // primitives get quoted - return out.next(quote(rawTree)); + return; } // Assertions of the above to narrow the type. assert.typeof(rawTree, 'object'); @@ -162,17 +146,12 @@ const decodeToJustin = (encoding, _cyclePolicy, shouldIndent = false) => { X`invalid qclass typeof ${q(typeof qclass)}`, ); assert(!Array.isArray(rawTree)); - // Switching on `encoded[QCLASS]` (or anything less direct, like - // `qclass`) does not discriminate rawTree in typescript@4.2.3 and - // earlier. switch (rawTree['@qclass']) { - // Encoding of primitives not handled by JSON case 'undefined': case 'NaN': case 'Infinity': case '-Infinity': { - // Their qclass is their expression source. - return out.next(qclass); + return; } case 'bigint': { const { digits } = rawTree; @@ -181,19 +160,20 @@ const decodeToJustin = (encoding, _cyclePolicy, shouldIndent = false) => { 'string', X`invalid digits typeof ${q(typeof digits)}`, ); - return out.next(`${BigInt(digits)}n`); + return; } case '@@asyncIterator': { - return out.next('Symbol.asyncIterator'); + return; } - case 'ibid': { const { index } = rawTree; - return out.next(`getIbid(${index})`); + // ibidTable's `get` does some input validation. + const rawNode = ibidTable.get(index); + ibidMap.set(rawNode, index); + return; } - case 'error': { - registerIbid(rawTree); + ibidTable.register(rawTree); const { name, message } = rawTree; assert.typeof( name, @@ -209,27 +189,24 @@ const decodeToJustin = (encoding, _cyclePolicy, shouldIndent = false) => { 'string', X`invalid error message typeof ${q(typeof message)}`, ); - return out.next(`${name}(${quote(message)})`); + return; } - case 'slot': { - registerIbid(rawTree); - let { index, iface } = rawTree; - index = Number(Nat(index)); + ibidTable.register(rawTree); + const { index, iface } = rawTree; + assert.typeof(index, 'number'); + Nat(index); assert.typeof(iface, 'string'); - iface = quote(iface); - return out.next(`getSlotVal(${index},${iface})`); + return; } - case 'hilbert': { - startIbid(rawTree); + ibidTable.start(rawTree); const { original, rest } = rawTree; assert( 'original' in rawTree, X`Invalid Hilbert Hotel encoding ${rawTree}`, ); - out.open('{'); - decodeProperty(QCLASS, original); + prepare(original); if ('rest' in rawTree) { assert.typeof( rest, @@ -245,14 +222,155 @@ const decodeToJustin = (encoding, _cyclePolicy, shouldIndent = false) => { !(QCLASS in rest), X`Rest encoding ${rest} must not contain ${q(QCLASS)}`, ); - startIbid(rest); + ibidTable.start(rest); + const names = ownKeys(rest); + for (const name of names) { + assert.typeof( + name, + 'string', + X`Property name ${name} of ${rawTree} must be a string`, + ); + prepare(rest[name]); + } + ibidTable.finish(rest); + } + ibidTable.finish(rawTree); + return; + } + + default: { + assert.fail(X`unrecognized ${q(QCLASS)} ${q(qclass)}`, TypeError); + } + } + } else if (Array.isArray(rawTree)) { + ibidTable.start(rawTree); + const { length } = rawTree; + for (let i = 0; i < length; i += 1) { + prepare(rawTree[i]); + } + ibidTable.finish(rawTree); + } else { + ibidTable.start(rawTree); + const names = ownKeys(rawTree); + for (const name of names) { + assert.typeof( + name, + 'string', + X`Property name ${name} of ${rawTree} must be a string`, + ); + prepare(rawTree[name]); + } + ibidTable.finish(rawTree); + } + }; + + const makeIndenter = shouldIndent ? makeYesIndenter : makeNoIndenter; + const out = makeIndenter(); + + /** + * This is the second pass recursion after the first pass `prepare`. + * The first pass initialized `ibidMap` and did input validation so + * here we can safely assume everything it validated. + * + * @param {Encoding} rawTree + * @returns {number} + */ + const decode = rawTree => { + if (ibidMap.has(rawTree)) { + const index = ibidMap.get(rawTree); + out.next(`initIbid(${index},`); + // eslint-disable-next-line no-use-before-define + recur(rawTree); + return out.next(')'); + } + // eslint-disable-next-line no-use-before-define + return recur(rawTree); + }; + + const decodeProperty = (name, value) => { + out.line(); + if (name === '__proto__') { + // JavaScript interprets `{__proto__: x, ...}` + // as making an object inheriting from `x`, whereas + // in JSON it is simply a property name. Preserve the + // JSON meaning. + out.next(`["__proto__"]:`); + } else if (identPattern.test(name)) { + out.next(`${name}:`); + } else { + out.next(`${quote(name)}:`); + } + decode(value); + out.next(','); + }; + + /** + * Modeled after `fullRevive` in marshal.js + * + * @param {Encoding} rawTree + * @returns {number} + */ + const recur = rawTree => { + if (Object(rawTree) !== rawTree) { + // primitives get quoted + return out.next(quote(rawTree)); + } + // Assertions of the above to narrow the type. + assert.typeof(rawTree, 'object'); + assert(rawTree !== null); + if (QCLASS in rawTree) { + const qclass = rawTree[QCLASS]; + assert.typeof(qclass, 'string'); + assert(!Array.isArray(rawTree)); + // Switching on `encoded[QCLASS]` (or anything less direct, like + // `qclass`) does not discriminate rawTree in typescript@4.2.3 and + // earlier. + switch (rawTree['@qclass']) { + // Encoding of primitives not handled by JSON + case 'undefined': + case 'NaN': + case 'Infinity': + case '-Infinity': { + // Their qclass is their expression source. + return out.next(qclass); + } + case 'bigint': { + const { digits } = rawTree; + return out.next(`${BigInt(digits)}n`); + } + case '@@asyncIterator': { + return out.next('Symbol.asyncIterator'); + } + + case 'ibid': { + const { index } = rawTree; + return out.next(`getIbid(${index})`); + } + + case 'error': { + const { name, message } = rawTree; + return out.next(`${name}(${quote(message)})`); + } + + case 'slot': { + let { index, iface } = rawTree; + index = Number(Nat(index)); + iface = quote(iface); + return out.next(`getSlotVal(${index},${iface})`); + } + + case 'hilbert': { + const { original, rest } = rawTree; + out.open('{'); + decodeProperty(QCLASS, original); + if ('rest' in rawTree) { + assert.typeof(rest, 'object'); + assert(rest !== null); const names = ownKeys(rest); for (const name of names) { decodeProperty(name, rest[name]); } - finishIbid(rest); } - finishIbid(rawTree); return out.close('}'); } @@ -261,38 +379,33 @@ const decodeToJustin = (encoding, _cyclePolicy, shouldIndent = false) => { } } } else if (Array.isArray(rawTree)) { - startIbid(rawTree); const { length } = rawTree; if (length === 0) { - finishIbid(rawTree); return out.next('[]'); } else { out.open('['); for (let i = 0; i < length; i += 1) { out.line(); - recur(rawTree[i]); + decode(rawTree[i]); out.next(','); } - finishIbid(rawTree); return out.close(']'); } } else { - startIbid(rawTree); const names = ownKeys(rawTree); if (names.length === 0) { - finishIbid(rawTree); return out.next('{}'); } else { out.open('{'); for (const name of names) { decodeProperty(name, rawTree[name]); } - finishIbid(rawTree); return out.close('}'); } } }; - recur(encoding); + prepare(encoding); + decode(encoding); return out.done(); }; harden(decodeToJustin); diff --git a/packages/marshal/test/test-marshal-justin.js b/packages/marshal/test/test-marshal-justin.js index a780c7f93f9..1d858289b9e 100644 --- a/packages/marshal/test/test-marshal-justin.js +++ b/packages/marshal/test/test-marshal-justin.js @@ -1,6 +1,6 @@ import '@agoric/install-ses'; import test from 'ava'; -import { makeMarshal } from '../src/marshal'; +import { makeMarshal, Remotable } from '../src/marshal'; import { decodeToJustin } from '../src/marshal-justin'; // this only includes the tests that do not use liveSlots @@ -55,18 +55,39 @@ export const jsonPairs = harden([ '{"@qclass":"hilbert","original":{"@qclass":"hilbert","original":8,"rest":{"foo":"foo1"}},"rest":{"bar":{"@qclass":"hilbert","original":{"@qclass":"undefined"}}}}', '{"@qclass":{"@qclass":8,foo:"foo1"},bar:{"@qclass":undefined}}', ], + + // ibids and slots + [ + '[{"foo":8},{"@qclass":"ibid","index":1}]', + '[initIbid(1,{foo:8}),getIbid(1)]', + ], + [ + '[{"@qclass":"slot","iface":"Alleged: for testing Justin","index":0},{"@qclass":"ibid","index":1}]', + '[initIbid(1,getSlotVal(0,"Alleged: for testing Justin")),getIbid(1)]', + ], ]); +const fakeJustinCompartment = () => { + const getSlotVal = (index, iface) => + Remotable(iface, undefined, { getIndex: () => index }); + const ibids = []; + const initIbid = (index, val) => { + assert(ibids[index] === undefined); + ibids[index] = val; + return val; + }; + const getIbid = index => ibids[index]; + return new Compartment({ getSlotVal, initIbid, getIbid }); +}; + test('serialize decodeToJustin eval round trip pairs', t => { - const c = new Compartment({ - // Will be the endowments assumed by these Justin expressions - }); const { serialize } = makeMarshal(undefined, undefined, { - // We're turning `errorTagging`` off only for the round trip test, not in + // We're turning `errorTagging`` off only for the round trip tests, not in // general. errorTagging: 'off', }); for (const [body, justinSrc] of jsonPairs) { + const c = fakeJustinCompartment(); const encoding = JSON.parse(body); const justinExpr = decodeToJustin(encoding); t.is(justinExpr, justinSrc); @@ -77,20 +98,18 @@ test('serialize decodeToJustin eval round trip pairs', t => { }); // Like "serialize decodeToJustin eval round trip pairs" but uses the indented -// representation *without* checking its specific whitespace decisions, which -// are currently terrible. Just checking that it has equivalent evaluation, and +// representation *without* checking its specific whitespace decisions. +// Just checks that it has equivalent evaluation, and // that the decoder passes the extra `level` balancing diagnostic in // `makeYesIndenter`. test('serialize decodeToJustin indented eval round trip', t => { - const c = new Compartment({ - // Will be the endowments assumed by these Justin expressions - }); const { serialize } = makeMarshal(undefined, undefined, { - // We're turning `errorTagging`` off only for the round trip test, not in + // We're turning `errorTagging`` off only for the round trip tests, not in // general. errorTagging: 'off', }); for (const [body] of jsonPairs) { + const c = fakeJustinCompartment(); const encoding = JSON.parse(body); const justinExpr = decodeToJustin(encoding, true); const value = harden(c.evaluate(`(${justinExpr})`));