Repository: keichi/binary-parser (ts)

Issue:
Bit field formatter
You can't use formatters with bitfields other than the last one in a sequence, since the bitfield reading code generation gets deferred until the last one, but the formatter code is inserted after each field in a sequence. Thus all the other bit field values get overwritten when the actual reading and bit shifting code is executed, because the formatters were called earlier.

**EDIT:**

It's been almost five years, but I thought I'd add a concrete example. 😄 I'm using `binary-parser@1.6.2` for these.

Here's a simple parser that parses uint8 values, and uses formatter for those (although these formatters just return a constant string).

```js
new Parser()
    .uint8('a', { formatter: () => 'first' })
    .uint8('b', { formatter: () => 'second' })
    .uint8('c', { formatter: () => 'third' });
```

The generated code is fine. First we read the value to the `vars` and then we immediately replace that value with the formatted version.

```js
vars.a = dataView.getUint8(offset, false);
offset += 1;
vars.a = (() => 'first').call(this, vars.a);

vars.b = dataView.getUint8(offset, false);
offset += 1;
vars.b = (() => 'second').call(this, vars.b);

vars.c = dataView.getUint8(offset, false);
offset += 1;
vars.c = (() => 'third').call(this, vars.b);
```

Here's the same thing, except we're parsing bitfields. We're still using formatters.

```js
new Parser()
    .bit8('a', { formatter: () => 'first' })
    .bit8('b', { formatter: () => 'second' })
    .bit8('c', { formatter: () => 'third' });
```

Now there's a bug in the generated code:

```js
vars.a = (() => 'first').call(this, vars.a);  // Formatter called with vars.a being undefined
vars.b = (() => 'second').call(this, vars.b); // Same with vars.b
var $tmp1 = dataView.getUint16(offset);
var $tmp2 = dataView.getUint8(offset + 2);
var $tmp0 = ($tmp1 << 8) | $tmp2;
offset += 3;
vars.a = $tmp0 >> 16 & 255; // The values are finally parsed
vars.b = $tmp0 >> 8 & 255;
vars.c = $tmp0 >> 0 & 255;
vars.c = (() => 'third').call(this, vars.c); // But only the last one is properly formatted
```

When parsing multiple successive bitfields, the code that populates the varibles is "deferred" to the end. However, the code that calls the formatters is not deferred. Thus the formatters get called too early, when the values don't exist yet.

The "deferring" feature here causes the issue. Or rather the lack of deferring for the formatters. The bitfield parsing code is only generated [when there are no more bitfields](https://github.com/keichi/binary-parser/blob/06701a0ba05cfa92a17b826a234f22cb8bfcb1dd/lib/binary_parser.ts#L857-L904) in the succession. But the code that calls the formatters is generated [every time](https://github.com/keichi/binary-parser/blob/06701a0ba05cfa92a17b826a234f22cb8bfcb1dd/lib/binary_parser.ts#L806-L808).
bit32 always returns 0
``` js
require('binary-parser').Parser.start().bit32('a').getCode()
```

returns

``` js
var vars = {};
var offset = 0;
if (!Buffer.isBuffer(buffer)) {
throw new Error("argument buffer is not a Buffer object");
}
var $tmp0 = buffer.readUInt32BE(offset);
offset += 4;
vars.a = $tmp0 >> 0 & 0;
return vars;
```

Please note the `& 0`, which will always lead to the result `0`.

Bugs with .bitN functions
It seems the `.bitN` functions don't always respect `options` passed to them. However, `.nest`-ing them in a new parser does cause them to respect `options`, unless the `nest`-ed `.bitN` functions align to a byte.

Additionally, `.nest`-ed parsers consume up to the next byte boundary, even when only parsing `.bitN` functions.

Examples:
https://repl.it/Klzf/20
.bit2 formatter don't work
.bit2('Sensor', { formatter: function (Sensor) { console.log(Sensor) } });

console.log is empty

bit field sequence longer than 4-bytes
I have a bit field described as follows:

[bit12][bit12][bit12][bit2][bit12]...

However, I get the error "bit field sequence longer than 4-bytes is not suported." I understand why the error is occurring. How do I parse this?

Thanks for great software!
Formatter and assert functions called too early for bitN()
Hi all.

I would like to format multi-bit field result. I define formatter function but as seen from the generated code, formatter is called before the actual bitfield is extracted. Same holds true for assert function for bitfields.
I assume formatting was not intended functionality for bitfields, but for my payload parsers it would be super-duper awesome.

Original code:
```
var Parser = require("binary-parser").Parser;

var test_sub = new Parser()
  .endianess('little')
  .bit3("my_param", {
      formatter: function(arg) {
        console.log(arg);
        return arg*arg; // should modify the value
      }
  })
  .bit5("the_other_param")

var test_parser = new Parser()
  .endianess('little')
  .uint8("packet_type", { assert: 0x04 })
  .nest("bitfields", {"type":test_sub});

var buf = Buffer.from("0482", "hex");

var res = test_parser.parse(buf);
console.log(JSON.stringify(res, null, 2));
```

Compiled code:
```
(function anonymous(imports,TextDecoder
) {
return function (buffer, constructorFn) { var dataView = new DataView(buffer.buffer, buffer.byteOffset, buffer.length);
var offset = 0;
var vars = {};
vars.packet_type = dataView.getUint8(offset, false);
offset += 1;
if (4 !== vars.packet_type) {
throw new Error("Assert error: vars.packet_type is " + 4);
}
vars.bitfields = {};
vars.bitfields.my_param = imports[0].call(this, vars.bitfields.my_param); // tries to modify but its undefined here
var $tmp0 = dataView.getUint8(offset);
offset += 1;
vars.bitfields.my_param = $tmp0 >> 0 & 7; // after calling the formatter now it extracts the value
vars.bitfields.the_other_param = $tmp0 >> 3 & 31;
return vars;
 };
})
```

Console output:
```
undefined
{
  "packet_type": 4,
  "bitfields": {
    "my_param": 2,
    "the_other_param": 16
  }
}
```

Pull request description:
Allow long bit field sequences
Fixes #95
Fixes #35
Fixes #29
Fixes #56
Fixes #71
Fixes #160

Patch:
diff --git a/lib/binary_parser.ts b/lib/binary_parser.ts
index 5edc73a..93294b1 100644
--- a/lib/binary_parser.ts
+++ b/lib/binary_parser.ts
@@ -978,11 +978,11 @@ export class Parser {
           this.generateWrapper(ctx);
           break;
       }
-      this.generateAssert(ctx);
+      if (this.type !== "bit") this.generateAssert(ctx);
     }
 
     const varName = ctx.generateVariable(this.varName);
-    if (this.options.formatter) {
+    if (this.options.formatter && this.type !== "bit") {
       this.generateFormatter(ctx, varName, this.options.formatter);
     }
 
@@ -1036,6 +1036,9 @@ export class Parser {
   private generateBit(ctx: Context) {
     // TODO find better method to handle nested bit fields
     const parser = JSON.parse(JSON.stringify(this));
+    parser.options = this.options;
+    parser.generateAssert = this.generateAssert.bind(this);
+    parser.generateFormatter = this.generateFormatter.bind(this);
     parser.varName = ctx.generateVariable(parser.varName);
     ctx.bitFields.push(parser);
 
@@ -1043,46 +1046,89 @@ export class Parser {
       !this.next ||
       (this.next && ["bit", "nest"].indexOf(this.next.type) < 0)
     ) {
-      let sum = 0;
-      ctx.bitFields.forEach(
-        (parser) => (sum += parser.options.length as number)
-      );
-
       const val = ctx.generateTmpVariable();
 
-      if (sum <= 8) {
-        ctx.pushCode(`var ${val} = dataView.getUint8(offset);`);
-        sum = 8;
-      } else if (sum <= 16) {
-        ctx.pushCode(`var ${val} = dataView.getUint16(offset);`);
-        sum = 16;
-      } else if (sum <= 24) {
-        const val1 = ctx.generateTmpVariable();
-        const val2 = ctx.generateTmpVariable();
-        ctx.pushCode(`var ${val1} = dataView.getUint16(offset);`);
-        ctx.pushCode(`var ${val2} = dataView.getUint8(offset + 2);`);
-        ctx.pushCode(`var ${val} = (${val1} << 8) | ${val2};`);
-        sum = 24;
-      } else if (sum <= 32) {
-        ctx.pushCode(`var ${val} = dataView.getUint32(offset);`);
-        sum = 32;
-      } else {
-        throw new Error(
-          "Currently, bit field sequence longer than 4-bytes is not supported."
-        );
-      }
-      ctx.pushCode(`offset += ${sum / 8};`);
+      ctx.pushCode(`var ${val} = 0;`);
+
+      const getMaxBits = (from = 0) => {
+        let sum = 0;
+        for (let i = from; i < ctx.bitFields.length; i++) {
+          const length = ctx.bitFields[i].options.length as number;
+          if (sum + length > 32) break;
+          sum += length;
+        }
+        return sum;
+      };
+
+      const getBytes = (sum: number) => {
+        if (sum <= 8) {
+          ctx.pushCode(`${val} = dataView.getUint8(offset);`);
+          sum = 8;
+        } else if (sum <= 16) {
+          ctx.pushCode(`${val} = dataView.getUint16(offset);`);
+          sum = 16;
+        } else if (sum <= 24) {
+          ctx.pushCode(
+            `${val} = (dataView.getUint16(offset) << 8) | dataView.getUint8(offset + 2);`
+          );
+          sum = 24;
+        } else {
+          ctx.pushCode(`${val} = dataView.getUint32(offset);`);
+          sum = 32;
+        }
+        ctx.pushCode(`offset += ${sum / 8};`);
+        return sum;
+      };
 
       let bitOffset = 0;
       const isBigEndian = this.endian === "be";
 
-      ctx.bitFields.forEach((parser) => {
-        const length = parser.options.length as number;
+      let sum = 0;
+      let rem = 0;
+
+      ctx.bitFields.forEach((parser, i) => {
+        let length = parser.options.length as number;
+        if (length > rem) {
+          if (rem) {
+            const mask = -1 >>> (32 - rem);
+            ctx.pushCode(
+              `${parser.varName} = (${val} & 0x${mask.toString(16)}) << ${
+                length - rem
+              };`
+            );
+            length -= rem;
+          }
+          bitOffset = 0;
+          rem = sum = getBytes(getMaxBits(i) - rem);
+        }
         const offset = isBigEndian ? sum - bitOffset - length : bitOffset;
-        const mask = (1 << length) - 1;
+        const mask = -1 >>> (32 - length);
+
+        ctx.pushCode(
+          `${parser.varName} ${
+            length < (parser.options.length as number) ? "|=" : "="
+          } ${val} >> ${offset} & 0x${mask.toString(16)};`
+        );
+
+        // Ensure value is unsigned
+        if ((parser.options.length as number) === 32) {
+          ctx.pushCode(`${parser.varName} >>>= 0`);
+        }
+
+        if (parser.options.assert) {
+          parser.generateAssert(ctx);
+        }
+
+        if (parser.options.formatter) {
+          parser.generateFormatter(
+            ctx,
+            parser.varName,
+            parser.options.formatter
+          );
+        }
 
-        ctx.pushCode(`${parser.varName} = ${val} >> ${offset} & ${mask};`);
         bitOffset += length;
+        rem -= length;
       });
 
       ctx.bitFields = [];


Test patch:
diff --git a/test/primitive_parser.ts b/test/primitive_parser.ts
index 2b18c75..21794d8 100644
--- a/test/primitive_parser.ts
+++ b/test/primitive_parser.ts
@@ -120,6 +120,7 @@ function primitiveParserTests(
         const bytes = [];
 
         s = s.replace(/\s/g, "");
+        deepStrictEqual(s.length % 8, 0);
         for (let i = 0; i < s.length; i += 8) {
           bytes.push(parseInt(s.slice(i, i + 8), 2));
         }
@@ -212,6 +213,49 @@ function primitiveParserTests(
           e: 1,
         });
       });
+      it("should parse 32-bit fields", () => {
+        const parser1 = new Parser().bit32("a");
+        const buf1 = binaryLiteral("10110101011101010111001010011101");
+        deepStrictEqual(parser1.parse(buf1), { a: 3044373149 });
+        const parser2 = new Parser().bit6("a").bit32("b").bit2("c");
+        const buf2 = binaryLiteral(
+          "101101 10110101011101010111001010011101 11"
+        );
+        deepStrictEqual(parser2.parse(buf2), { a: 45, b: 3044373149, c: 3 });
+      });
+      it("should parse arbitrarily large bit field sequence", () => {
+        const parser1 = new Parser()
+          .bit1("a")
+          .bit24("b")
+          .bit4("c")
+          .bit2("d")
+          .bit9("e");
+        const buf = binaryLiteral(
+          "1 101010101010101010101010 1111 01 110100110"
+        );
+        deepStrictEqual(parser1.parse(buf), {
+          a: 1,
+          b: 11184810,
+          c: 15,
+          d: 1,
+          e: 422,
+        });
+
+        const parser2 = new Parser()
+          .endianness("little")
+          .bit1("a")
+          .bit24("b")
+          .bit4("c")
+          .bit2("d")
+          .bit9("e");
+        deepStrictEqual(parser2.parse(buf), {
+          a: 1,
+          b: 11184829,
+          c: 10,
+          d: 2,
+          e: 422,
+        });
+      });
       it("should parse nested bit fields", () => {
         const parser = new Parser().bit1("a").nest("x", {
           type: new Parser().bit2("b").bit4("c").bit1("d"),
@@ -228,6 +272,24 @@ function primitiveParserTests(
           },
         });
       });
+
+      it("should assert bit fields", () => {
+        const parser = new Parser()
+          .bit4("a", { assert: (v) => v === 10 })
+          .bit3("b", { assert: (v) => v === 7 })
+          .bit1("c", { assert: (v) => v === 1 });
+        const buf = binaryLiteral("1010 111 1");
+        deepStrictEqual(parser.parse(buf), { a: 10, b: 7, c: 1 });
+      });
+
+      it("should format bit fields", () => {
+        const parser = new Parser()
+          .bit4("a", { formatter: (v) => v * 2 })
+          .bit3("b", { formatter: (v) => v * 3 })
+          .bit1("c", { formatter: (v) => v * 4 });
+        const buf = binaryLiteral("1010 111 1");
+        deepStrictEqual(parser.parse(buf), { a: 20, b: 21, c: 4 });
+      });
     });
 
     describe("String parser", () => {