fix: Correctly decode optional struct fields

docs/generated/changelog.html

@@ -10,7 +10,11 @@
     <h1>Agent-JS Changelog</h1>
 
     <section>
-      <h2>Version 0.10.5</h2>
+      <h2>Version 0.11.1</h2>
+      <ul>
+        <li>Fix for a corner case that could lead to incorrect decoding of record types.</li>
+      </ul>
+      <h2>Version 0.11.0</h2>
       <ul>
         <li>
           makeNonce now returns unique values. Previously only the first byte of the nonce was

packages/candid/src/idl.test.ts

@@ -534,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', () => {
@@ -543,7 +543,7 @@ 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 / encode unknown mutual recursive lists', () => {
@@ -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],
+  });
+});

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
 /**
@@ -919,18 +922,43 @@ export class RecordClass extends ConstructType<Record<string, any>> {
       throw new Error('Not a record type');
     }
     const x: Record<string, any> = {};
-    let idx = 0;
-    for (const [hash, type] of record._fields) {
-      if (idx >= this._fields.length || idlLabelToId(this._fields[idx][0]) !== idlLabelToId(hash)) {
-        // skip field
+
+    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;
+      }
+
+      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;
       }
-      const [expectKey, expectType] = this._fields[idx];
+
       x[expectKey] = expectType.decodeValue(b, type);
-      idx++;
+      expectedRecordIdx++;
+      actualRecordIdx++;
     }
-    for (const [expectKey, expectType] of this._fields.slice(idx)) {
+
+    // initialize left over expected optional fields
+    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] = [];
@@ -1327,7 +1355,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,25 +1499,46 @@ 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: {
           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:
@@ -1594,15 +1643,35 @@ 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({});
+        const rec: Record<string, FuncClass> = {};
+        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]);
     }
   }
+
   rawTable.forEach((entry, i) => {
     const t = buildType(entry);
     table[i].fill(t);