From 9267824a7cc5608b893606352874eb4c9bb05e99 Mon Sep 17 00:00:00 2001 From: Frederik Rothenberger Date: Wed, 20 Apr 2022 10:40:57 +0200 Subject: [PATCH 1/4] Retain complete function types on decode --- packages/candid/src/idl.test.ts | 18 +++++++++++++- packages/candid/src/idl.ts | 43 ++++++++++++++++++++++++++------- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/packages/candid/src/idl.test.ts b/packages/candid/src/idl.test.ts index afd60210e..87e46841d 100644 --- a/packages/candid/src/idl.test.ts +++ b/packages/candid/src/idl.test.ts @@ -6,6 +6,8 @@ import * as IDL from './idl'; import { Principal } from '@dfinity/principal'; import { fromHexString, toHexString } from './utils/buffer'; import { idlLabelToId } from './utils/hash'; +import fs from 'fs'; +import { fromHex, toHex } from '@dfinity/agent/lib/cjs/utils/buffer'; function testEncode(typ: IDL.Type, val: any, hex: string, _str: string) { expect(toHexString(IDL.encode([typ], [val]))).toEqual(hex); @@ -543,7 +545,21 @@ test('decode unknown func', () => { fromHexString('4449444c016a0171017d01010100010103caffee03666f6f'), )[0] as any; expect(value).toEqual([Principal.fromText('w7x7r-cok77-xa'), 'foo']); - expect(value.type()).toEqual(IDL.Func([], [], [])); + expect(value.type()).toEqual(IDL.Func([IDL.Text], [IDL.Nat], ['query'])); +}); + +test('decode unknown funcasdf', () => { + const value = IDL.decode( + [IDL.Unknown], + fromHexString('4449444c016a0371717a027178010101000101000c696e7374616c6c5f636f6465'), + )[0] as any; +}); + +it('fooo2', () => { + fs.readFile('candid.txt', 'utf8', (err, data) => { + const decoded = IDL.decode([IDL.Unknown], fromHex(data))[0] as any; + console.log(decoded.type().display()); + }); }); test('decode / encode unknown mutual recursive lists', () => { diff --git a/packages/candid/src/idl.ts b/packages/candid/src/idl.ts index d0a49e660..ff5680e4b 100644 --- a/packages/candid/src/idl.ts +++ b/packages/candid/src/idl.ts @@ -1327,7 +1327,7 @@ export class FuncClass extends ConstructType<[PrincipalId, string]> { } else if (ann === 'oneway') { return new Uint8Array([2]); } else { - throw new Error('Illeagal function annotation'); + throw new Error('Illegal function annotation'); } } } @@ -1471,15 +1471,34 @@ export function decode(retTypes: Type[], bytes: ArrayBuffer): JsonValue[] { break; } case IDLTypeIds.Func: { - for (let k = 0; k < 2; k++) { - let funcLength = Number(lebDecode(pipe)); - while (funcLength--) { - slebDecode(pipe); + const args = []; + let argLength = Number(lebDecode(pipe)); + while (argLength--) { + args.push(Number(slebDecode(pipe))); + } + const returnValues = []; + let returnValuesLength = Number(lebDecode(pipe)); + while (returnValuesLength--) { + returnValues.push(Number(slebDecode(pipe))); + } + const annotations = []; + let annotationLength = Number(lebDecode(pipe)); + while (annotationLength--) { + const annotation = Number(lebDecode(pipe)); + switch (annotation) { + case 1: { + annotations.push('query'); + break; + } + case 2: { + annotations.push('oneway'); + break; + } + default: + throw new Error('unknown annotation'); } } - const annLen = Number(lebDecode(pipe)); - safeRead(pipe, annLen); - typeTable.push([ty, undefined]); + typeTable.push([ty, [args, returnValues, annotations]]); break; } case IDLTypeIds.Service: { @@ -1594,7 +1613,12 @@ export function decode(retTypes: Type[], bytes: ArrayBuffer): JsonValue[] { return Variant(fields); } case IDLTypeIds.Func: { - return Func([], [], []); + const [args, returnValues, annotations] = entry[1]; + return Func( + args.map((t: number) => getType(t)), + returnValues.map((t: number) => getType(t)), + annotations, + ); } case IDLTypeIds.Service: { return Service({}); @@ -1603,6 +1627,7 @@ export function decode(retTypes: Type[], bytes: ArrayBuffer): JsonValue[] { throw new Error('Illegal op_code: ' + entry[0]); } } + rawTable.forEach((entry, i) => { const t = buildType(entry); table[i].fill(t); From b3398c5b3e2b6b8efd6a4262b443f04c05dcbea6 Mon Sep 17 00:00:00 2001 From: Frederik Rothenberger Date: Wed, 20 Apr 2022 11:28:04 +0200 Subject: [PATCH 2/4] Retain complete service types on decode --- packages/candid/src/idl.test.ts | 18 +----------------- packages/candid/src/idl.ts | 33 ++++++++++++++++++++++++++------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/packages/candid/src/idl.test.ts b/packages/candid/src/idl.test.ts index 87e46841d..cc270a384 100644 --- a/packages/candid/src/idl.test.ts +++ b/packages/candid/src/idl.test.ts @@ -6,8 +6,6 @@ import * as IDL from './idl'; import { Principal } from '@dfinity/principal'; import { fromHexString, toHexString } from './utils/buffer'; import { idlLabelToId } from './utils/hash'; -import fs from 'fs'; -import { fromHex, toHex } from '@dfinity/agent/lib/cjs/utils/buffer'; function testEncode(typ: IDL.Type, val: any, hex: string, _str: string) { expect(toHexString(IDL.encode([typ], [val]))).toEqual(hex); @@ -536,7 +534,7 @@ test('decode unknown service', () => { fromHexString('4449444c026a0171017d00690103666f6f0001010103caffee'), )[0] as any; expect(value).toEqual(Principal.fromText('w7x7r-cok77-xa')); - expect(value.type()).toEqual(IDL.Service({})); + expect(value.type()).toEqual(IDL.Service({ foo: IDL.Func([IDL.Text], [IDL.Nat], []) })); }); test('decode unknown func', () => { @@ -548,20 +546,6 @@ test('decode unknown func', () => { expect(value.type()).toEqual(IDL.Func([IDL.Text], [IDL.Nat], ['query'])); }); -test('decode unknown funcasdf', () => { - const value = IDL.decode( - [IDL.Unknown], - fromHexString('4449444c016a0371717a027178010101000101000c696e7374616c6c5f636f6465'), - )[0] as any; -}); - -it('fooo2', () => { - fs.readFile('candid.txt', 'utf8', (err, data) => { - const decoded = IDL.decode([IDL.Unknown], fromHex(data))[0] as any; - console.log(decoded.type().display()); - }); -}); - test('decode / encode unknown mutual recursive lists', () => { // original types const List1 = IDL.Rec(); diff --git a/packages/candid/src/idl.ts b/packages/candid/src/idl.ts index ff5680e4b..f340e6bd4 100644 --- a/packages/candid/src/idl.ts +++ b/packages/candid/src/idl.ts @@ -1,17 +1,20 @@ // tslint:disable:max-classes-per-file import { Principal as PrincipalId } from '@dfinity/principal'; import { JsonValue } from './types'; -import { concat, PipeArrayBuffer as Pipe, toHexString } from './utils/buffer'; +import { concat, PipeArrayBuffer as Pipe } from './utils/buffer'; import { idlLabelToId } from './utils/hash'; import { lebDecode, lebEncode, + readIntLE, + readUIntLE, safeRead, safeReadUint8, slebDecode, slebEncode, + writeIntLE, + writeUIntLE, } from './utils/leb128'; -import { readIntLE, readUIntLE, writeIntLE, writeUIntLE } from './utils/leb128'; // tslint:disable:max-line-length /** @@ -1503,12 +1506,14 @@ export function decode(retTypes: Type[], bytes: ArrayBuffer): JsonValue[] { } case IDLTypeIds.Service: { let servLength = Number(lebDecode(pipe)); + const methods = []; while (servLength--) { - const l = Number(lebDecode(pipe)); - safeRead(pipe, l); - slebDecode(pipe); + const nameLength = Number(lebDecode(pipe)); + const funcName = new TextDecoder().decode(safeRead(pipe, nameLength)); + const funcType = slebDecode(pipe); + methods.push([funcName, funcType]); } - typeTable.push([ty, undefined]); + typeTable.push([ty, methods]); break; } default: @@ -1621,7 +1626,21 @@ export function decode(retTypes: Type[], bytes: ArrayBuffer): JsonValue[] { ); } case IDLTypeIds.Service: { - return Service({}); + const rec: Record = {}; + const methods = entry[1] as [[string, number]]; + for (const [name, typeRef] of methods) { + let type: Type | undefined = getType(typeRef); + + if (type instanceof RecClass) { + // unpack reference type + type = type.getType(); + } + if (!(type instanceof FuncClass)) { + throw new Error('Illegal service definition: services can only contain functions'); + } + rec[name] = type; + } + return Service(rec); } default: throw new Error('Illegal op_code: ' + entry[0]); From bf5154166c0e5b832a4b105a274e00cd06abf0e8 Mon Sep 17 00:00:00 2001 From: Frederik Rothenberger Date: Wed, 20 Apr 2022 20:11:18 +0200 Subject: [PATCH 3/4] Fix incorrect decoding of optional fields --- packages/candid/src/idl.test.ts | 20 ++++++++++++++++++++ packages/candid/src/idl.ts | 18 ++++++++++++++++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/packages/candid/src/idl.test.ts b/packages/candid/src/idl.test.ts index cc270a384..ae9256d49 100644 --- a/packages/candid/src/idl.test.ts +++ b/packages/candid/src/idl.test.ts @@ -608,3 +608,23 @@ test('decode / encode unknown nested record', () => { const decodedValue2 = IDL.decode([recordType], fromHexString(reencoded))[0] as any; expect(decodedValue2).toEqual(value); }); + +test('should correctly decode expected optional fields with lower hash than required fields', () => { + const HttpResponse = IDL.Record({ + body: IDL.Text, + headers: IDL.Vec(IDL.Tuple(IDL.Text, IDL.Text)), + streaming_strategy: IDL.Opt(IDL.Text), + status_code: IDL.Int, + upgrade: IDL.Opt(IDL.Bool), + }); + const encoded = + '4449444c036c04a2f5ed880471c6a4a19806019ce9c69906029aa1b2f90c7c6d7f6e7e010003666f6f000101c801'; + const value = IDL.decode([HttpResponse], fromHexString(encoded))[0]; + expect(value).toEqual({ + body: 'foo', + headers: [], + status_code: BigInt(200), + streaming_strategy: [], + upgrade: [true], + }); +}); diff --git a/packages/candid/src/idl.ts b/packages/candid/src/idl.ts index f340e6bd4..57bb4e8d1 100644 --- a/packages/candid/src/idl.ts +++ b/packages/candid/src/idl.ts @@ -924,15 +924,29 @@ export class RecordClass extends ConstructType> { const x: Record = {}; let idx = 0; for (const [hash, type] of record._fields) { + let [expectKey, expectType] = this._fields[idx]; + + // skip expected optional fields that are not present in the data + while ( + (expectType instanceof OptClass || expectType instanceof ReservedClass) && + idx < this._fields.length && + idlLabelToId(this._fields[idx][0]) !== idlLabelToId(hash) + ) { + x[expectKey] = []; + idx++; + [expectKey, expectType] = this._fields[idx]; + } + if (idx >= this._fields.length || idlLabelToId(this._fields[idx][0]) !== idlLabelToId(hash)) { - // skip field + // skip unexpected fields present in the data type.decodeValue(b, type); continue; } - const [expectKey, expectType] = this._fields[idx]; x[expectKey] = expectType.decodeValue(b, type); idx++; } + + // initialize left over expected optional fields for (const [expectKey, expectType] of this._fields.slice(idx)) { if (expectType instanceof OptClass || expectType instanceof ReservedClass) { // TODO this assumes null value in opt is represented as [] From af3fd58ed5f9e7175cff1e739b07d226bab5375e Mon Sep 17 00:00:00 2001 From: Frederik Rothenberger Date: Wed, 20 Apr 2022 20:50:15 +0200 Subject: [PATCH 4/4] Refactor record parsing loop --- docs/generated/changelog.html | 6 ++++- packages/candid/src/idl.ts | 45 ++++++++++++++++++++++------------- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/docs/generated/changelog.html b/docs/generated/changelog.html index 628d9da97..9d2ad0c19 100644 --- a/docs/generated/changelog.html +++ b/docs/generated/changelog.html @@ -10,7 +10,11 @@

Agent-JS Changelog

-

Version 0.10.5

+

Version 0.11.1

+
    +
  • Fix for a corner case that could lead to incorrect decoding of record types.
    • +
+

Version 0.11.0

  • makeNonce now returns unique values. Previously only the first byte of the nonce was diff --git a/packages/candid/src/idl.ts b/packages/candid/src/idl.ts index 57bb4e8d1..8ef93c5d1 100644 --- a/packages/candid/src/idl.ts +++ b/packages/candid/src/idl.ts @@ -922,32 +922,43 @@ export class RecordClass extends ConstructType> { throw new Error('Not a record type'); } const x: Record = {}; - let idx = 0; - for (const [hash, type] of record._fields) { - let [expectKey, expectType] = this._fields[idx]; - - // skip expected optional fields that are not present in the data - while ( - (expectType instanceof OptClass || expectType instanceof ReservedClass) && - idx < this._fields.length && - idlLabelToId(this._fields[idx][0]) !== idlLabelToId(hash) - ) { - x[expectKey] = []; - idx++; - [expectKey, expectType] = this._fields[idx]; + + let expectedRecordIdx = 0; + let actualRecordIdx = 0; + while (actualRecordIdx < record._fields.length) { + const [hash, type] = record._fields[actualRecordIdx]; + + if (expectedRecordIdx >= this._fields.length) { + // skip unexpected left over fields present on the wire + type.decodeValue(b, type); + actualRecordIdx++; + continue; } - if (idx >= this._fields.length || idlLabelToId(this._fields[idx][0]) !== idlLabelToId(hash)) { - // skip unexpected fields present in the data + const [expectKey, expectType] = this._fields[expectedRecordIdx]; + if (idlLabelToId(this._fields[expectedRecordIdx][0]) !== idlLabelToId(hash)) { + // the current field on the wire does not match the expected field + + // skip expected optional fields that are not present on the wire + if (expectType instanceof OptClass || expectType instanceof ReservedClass) { + x[expectKey] = []; + expectedRecordIdx++; + continue; + } + + // skip unexpected interspersed fields present on the wire type.decodeValue(b, type); + actualRecordIdx++; continue; } + x[expectKey] = expectType.decodeValue(b, type); - idx++; + expectedRecordIdx++; + actualRecordIdx++; } // initialize left over expected optional fields - for (const [expectKey, expectType] of this._fields.slice(idx)) { + for (const [expectKey, expectType] of this._fields.slice(expectedRecordIdx)) { if (expectType instanceof OptClass || expectType instanceof ReservedClass) { // TODO this assumes null value in opt is represented as [] x[expectKey] = [];