From f571a59cfa40926d125337277990ae4e4dde11d4 Mon Sep 17 00:00:00 2001 From: Eric BLANCHARD Date: Sun, 15 Dec 2019 16:54:34 +0100 Subject: [PATCH] Fixes #20 String padded with space (0x20) instead of null (0x0) when reencoding --- README.md | 13 ++++---- lib/binary_parser.ts | 58 ++++++++++++++++++++---------------- package.json | 2 +- test/primitive_parser.js | 4 +-- test/yy_primitive_encoder.js | 20 +++++-------- test/zz_encoder_bugs.js | 49 ++++++++++++++++++++++++++++++ 6 files changed, 100 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 356b33d..f1d13e1 100644 --- a/README.md +++ b/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 diff --git a/lib/binary_parser.ts b/lib/binary_parser.ts index 50062b4..eaa2d81 100644 --- a/lib/binary_parser.ts +++ b/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);'); + } } } diff --git a/package.json b/package.json index 36371cf..9483da4 100644 --- a/package.json +++ b/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", diff --git a/test/primitive_parser.js b/test/primitive_parser.js index b9e1511..b8a8f85 100644 --- a/test/primitive_parser.js +++ b/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', }); diff --git a/test/yy_primitive_encoder.js b/test/yy_primitive_encoder.js index cdce471..75cc3c4 100644 --- a/test/yy_primitive_encoder.js +++ b/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'); diff --git a/test/zz_encoder_bugs.js b/test/zz_encoder_bugs.js index 2ba10de..09b79fa 100644 --- a/test/zz_encoder_bugs.js +++ b/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); + }); + }); });