From 47ee650ca4b2b902fa48f2c8a8fead2840f6482a Mon Sep 17 00:00:00 2001
From: Frederik Rothenberger
 <94825501+frederikrothenberger@users.noreply.github.com>
Date: Wed, 20 Apr 2022 21:19:33 +0200
Subject: [PATCH] fix: Correctly decode optional struct fields (#564)

* Fix incorrect decoding of optional fields
* Refactor record parsing loop
---
 docs/generated/changelog.html   |  6 ++++-
 packages/candid/src/idl.test.ts | 20 +++++++++++++++++
 packages/candid/src/idl.ts      | 39 +++++++++++++++++++++++++++------
 3 files changed, 57 insertions(+), 8 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 @@
     <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
diff --git a/packages/candid/src/idl.test.ts b/packages/candid/src/idl.test.ts
index 410e3d2e5..b851c7ad6 100644
--- a/packages/candid/src/idl.test.ts
+++ b/packages/candid/src/idl.test.ts
@@ -609,3 +609,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 13f9d09a9..508128f28 100644
--- a/packages/candid/src/idl.ts
+++ b/packages/candid/src/idl.ts
@@ -927,18 +927,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[idx];
+
+      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++;
     }
-    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] = [];