Fixes #20 String padded with space (0x20) instead of null (0x0) when reencoding

Author: Ericbla

README.md

@@ -166,25 +166,26 @@ the following keys:
   Supported encodings include `"utf8"`, `"ascii"` and `"hex"`. See
   [`Buffer.toString`](http://nodejs.org/api/buffer.html#buffer_buf_tostring_encoding_start_end)
   for more info.
-- `length ` - (Optional) (Bytes)Length of the string. Can be a number, string or a
+- `length` - (Optional) (Bytes)Length of the string. Can be a number, string or a
   function. Use number for statically sized arrays, string to reference
   another variable and function to do some calculation.
-  Note: when encoding the string is padded with spaces (0x20) at end to fit the length requirement.
+  Note: When encoding the string is padded with a `padd` charecter to fit the length requirement.
 - `zeroTerminated` - (Optional, defaults to `false`) If true, then this parser
-  reads until it reaches zero.
+  reads until it reaches zero (or the specified `length`). When encoding, a *null* character is inserted at end of
+  the string (if the optional `length` allows it).
 - `greedy` - (Optional, defaults to `false`) If true, then this parser reads
   until it reaches the end of the buffer. Will consume zero-bytes. (Note: has
   no effect on encoding function)
 - `stripNull` - (Optional, must be used with `length`) If true, then strip
-  null characters from end of the string. (Note: has no effect on encoding, but
-  when used, then the parse() and encode() functions are not the exact opposite)
+  null characters from end of the string. (Note: When encoding, this will also set the **default** `padd` character
+  to null instead of space)
 - `trim` - (Optional, default to `false`) If true, then trim() (remove leading and trailing spaces)
   the parsed string.
 - `padding` - (Optional, Only used for encoding, default to `right`) If `left` then the string
   will be right aligned (padding left with `padd` char or space) depending of the `length` option
 - `padd` - (Optional, Only used for encoding with `length` specified) A string  from which first character (1 Byte)
   is used as a padding char if necessary (provided string length is less than `length` option). Note: Only 'ascii'
-  or utf8 < 0x80  are alowed (fallback to 'space' padding else).
+  or utf8 < 0x80  are alowed. Note: The default padd character is *space* (or *null* when `stripNull` is used).
 
 ### buffer(name[, options])
 Parse bytes as a buffer. `name` should consist only of alpha numeric

lib/binary_parser.ts

@@ -1156,8 +1156,11 @@ export class Parser {
       ctx.pushCode(
         `while(buffer.readUInt8(offset++) !== 0 && offset - ${start}  < ${len});`
       );
+      //ctx.pushCode(
+      //  `${name} = buffer.toString('${encoding}', ${start}, offset - ${start} < ${len} ? offset - 1 : offset);`
+      //);
       ctx.pushCode(
-        `${name} = buffer.toString('${encoding}', ${start}, offset - ${start} < ${len} ? offset - 1 : offset);`
+        `${name} = buffer.toString('${encoding}', ${start}, buffer.readUInt8(offset -1) == 0 ? offset - 1 : offset);`
       );
     } else if (this.options.length) {
       const len = ctx.generateOption(this.options.length);
@@ -1202,36 +1205,41 @@ export class Parser {
       // Compute padding length
       const padLen = ctx.generateTmpVariable();
       ctx.pushCode(`${padLen} = ${optLength} - ${tmpBuf}.length;`);
-      const padCharVar = ctx.generateTmpVariable();
-      let padChar = ' ';
-      if (this.options.padd && typeof this.options.padd === 'string') {
-        const code = this.options.padd.charCodeAt(0);
-        if (code < 0x80) {
-          padChar = String.fromCharCode(code);
+      if (this.options.zeroTerminated) {
+        ctx.pushCode(`smartBuffer.writeBuffer(${tmpBuf});`);
+        ctx.pushCode(`if (${padLen} > 0) { smartBuffer.writeUInt8(0x00); }`);
+      } else {
+        const padCharVar = ctx.generateTmpVariable();
+        let padChar = this.options.stripNull ? '\u0000' : ' ';
+        if (this.options.padd && typeof this.options.padd === 'string') {
+          const code = this.options.padd.charCodeAt(0);
+          if (code < 0x80) {
+            padChar = String.fromCharCode(code);
+          }
+        }
+        ctx.pushCode(`${padCharVar} = "${padChar}";`);
+        if (this.options.padding === 'left') {
+          // Add heading padding spaces
+          ctx.pushCode(
+            `if (${padLen} > 0) {smartBuffer.writeString(${padCharVar}.repeat(${padLen}));}`
+          );
+        }
+        // Copy the temporary string buffer to current smartBuffer
+        ctx.pushCode(`smartBuffer.writeBuffer(${tmpBuf});`);
+        if (this.options.padding !== 'left') {
+          // Add trailing padding spaces
+          ctx.pushCode(
+            `if (${padLen} > 0) {smartBuffer.writeString(${padCharVar}.repeat(${padLen}));}`
+          );
         }
-      }
-      ctx.pushCode(`${padCharVar} = "${padChar}";`);
-      if (this.options.padding === 'left') {
-        // Add heading padding spaces
-        ctx.pushCode(
-          `if (${padLen} > 0) {smartBuffer.writeString(${padCharVar}.repeat(${padLen}));}`
-        );
-      }
-      // Copy the temporary string buffer to current smartBuffer
-      ctx.pushCode(`smartBuffer.writeBuffer(${tmpBuf});`);
-      if (this.options.padding !== 'left') {
-        // Add trailing padding spaces
-        ctx.pushCode(
-          `if (${padLen} > 0) {smartBuffer.writeString(${padCharVar}.repeat(${padLen}));}`
-        );
       }
     } else {
       ctx.pushCode(
         `smartBuffer.writeString(${name}, "${this.options.encoding}");`
       );
-    }
-    if (this.options.zeroTerminated) {
-      ctx.pushCode('smartBuffer.writeUInt8(0x00);');
+      if (this.options.zeroTerminated) {
+        ctx.pushCode('smartBuffer.writeUInt8(0x00);');
+      }
     }
   }
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "binary-parser-encoder",
-  "version": "1.5.1",
+  "version": "1.5.2",
   "description": "Blazing-fast binary parser builder",
   "main": "dist/binary_parser.js",
   "types": "dist/binary_parser.d.ts",

test/primitive_parser.js

@@ -364,14 +364,14 @@ describe('Primitive parser', function() {
       assert.deepEqual(parser.parse(buffer), { msg: 'hello, world' });
     });
     it('should parser zero terminated fixed-length string', function() {
-      var buffer = Buffer.from('abc\u0000defghij\u0000');
+      var buffer = Buffer.from('abcd\u0000defghij\u0000');
       var parser = Parser.start()
         .string('a', { length: 5, zeroTerminated: true })
         .string('b', { length: 5, zeroTerminated: true })
         .string('c', { length: 5, zeroTerminated: true });
 
       assert.deepEqual(parser.parse(buffer), {
-        a: 'abc',
+        a: 'abcd',
         b: 'defgh',
         c: 'ij',
       });

test/yy_primitive_encoder.js

@@ -412,7 +412,6 @@ describe('Primitive encoder', function() {
       assert.deepEqual(encoded, buffer);
     });
     it('should encode zero terminated fixed-length string', function() {
-      // In that case parsing and encoding are not  the exact oposite
       var buffer = Buffer.from('abc\u0000defghij\u0000');
       var parser = Parser.start()
         .string('a', { length: 5, zeroTerminated: true })
@@ -425,16 +424,15 @@ describe('Primitive encoder', function() {
         b: 'defgh',
         c: 'ij',
       });
+      let encoded = parser.encode(decoded);
+      assert.deepEqual(encoded, buffer);
 
-      var encoded = parser.encode({
-        a: 'abc',
-        b: 'defghzzzzzzz',
-        c: 'ij',
+      encoded = parser.encode({
+        a: 'a234',
+        b: 'b2345',
+        c: 'c2345678',
       });
-      assert.deepEqual(
-        encoded,
-        Buffer.from('abc  \u0000defgh\u0000ij   \u0000')
-      );
+      assert.deepEqual(encoded, Buffer.from('a234\u0000b2345c2345'));
     });
     it('should strip trailing null characters', function() {
       var buffer = Buffer.from('746573740000', 'hex');
@@ -454,10 +452,8 @@ describe('Primitive encoder', function() {
 
       var decoded2 = parser2.parse(buffer);
       assert.equal(decoded2.str, 'test');
-      // In this case (stripNull = true) parsing and encoding are not  the exact oposite
       var encoded2 = parser2.encode(decoded2);
-      assert.notDeepEqual(encoded2, buffer);
-      assert.deepEqual(encoded2, Buffer.from('test  '));
+      assert.deepEqual(encoded2, buffer);
     });
     it('should encode string with zero-bytes internally', function() {
       var buffer = Buffer.from('abc\u0000defghij\u0000');

test/zz_encoder_bugs.js

@@ -349,4 +349,53 @@ describe('Specific bugs testing', function() {
       assert.deepEqual(little2Encoded, data);
     });
   });
+
+  describe('Issue #20 Encoding fixed length null terminated or strip null strings', function() {
+    it('should encode zero terminated fixed-length string', function() {
+      // In that case parsing and encoding are not  the exact oposite
+      let buffer = Buffer.from(
+        '\u0000A\u0000AB\u0000ABC\u0000ABCD\u0000ABCDE\u0000'
+      );
+      let parser = Parser.start()
+        .string('a', { length: 4, zeroTerminated: true })
+        .string('b', { length: 4, zeroTerminated: true })
+        .string('c', { length: 4, zeroTerminated: true })
+        .string('d', { length: 4, zeroTerminated: true })
+        .string('e', { length: 4, zeroTerminated: true })
+        .string('f', { length: 4, zeroTerminated: true })
+        .string('g', { length: 4, zeroTerminated: true })
+        .string('h', { length: 4, zeroTerminated: true });
+
+      let decoded = parser.parse(buffer);
+      assert.deepEqual(decoded, {
+        a: '',
+        b: 'A',
+        c: 'AB',
+        d: 'ABC',
+        e: 'ABCD',
+        f: '',
+        g: 'ABCD',
+        h: 'E',
+      });
+
+      let encoded = parser.encode(decoded);
+      assert.deepEqual(encoded, buffer);
+    });
+
+    it('should encode fixed-length string with stripNull', function() {
+      let parser = Parser.start()
+        .string('a', { length: 8, zeroTerminated: false, stripNull: true })
+        .string('b', { length: 8, zeroTerminated: false, stripNull: true })
+        .string('z', { length: 2, zeroTerminated: false, stripNull: true });
+      let buffer = Buffer.from('ABCD\u0000\u0000\u0000\u000012345678ZZ');
+      let decoded = parser.parse(buffer);
+      assert.deepEqual(decoded, {
+        a: 'ABCD',
+        b: '12345678',
+        z: 'ZZ',
+      });
+      let encoded = parser.encode(decoded);
+      assert.deepEqual(encoded, buffer);
+    });
+  });
 });