Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements encoding from Parser #73

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

Ericbla
Copy link
Contributor

@Ericbla Ericbla commented Feb 7, 2018

Hello,

Here is an implementation of encode() function for this great declaratibe binary-parser.

This method is intended to be the opposite of the parse(buffer) method. It relies on the unchanged declaration of the Parser and then just is able to generate a buffer from a provided object

encode(object) -> Buffer

Almost all types supported by parse are also supported by encode function. The exception is for associative arrays (array type with key option) as there is no mean to determine keys order.

Only some options have not the same effect on parse and encode.
For example the formatter function has a new encoder function counter part.
And little endian is not taken into account for bit{n}.

I have duplicated almost all unit tests of parsing to use with encoding and mainly try to re-encode what have been parsed.

The implementation currently use a fixed size Buffer for encoding (The size: 256 bytes can be adjusted with an option in the Parser constructor and Parser.start()).
It would be a good idea to replace it in the future with a dynamic Buffer implementation (that can adjust its size during encoding).

I hope you will adopt this new feature as it is seems (at least for me) a very common usage to have both parsing and encoding from a single declarative spec. (e.g. when handling protocols).

Regards.

--
Eric

@emcee-ai emcee-ai mentioned this pull request Feb 13, 2018
@Ericbla
Copy link
Contributor Author

Ericbla commented Feb 27, 2018

The Implementation now uses smart-buffer for the encoding feature. So, it is no more needed to estimate the size of the encoded result before using parser.encode().

I have also added additional parameters to the formatter (and associated encoder) function so that we can use other criteria than the sole value to provide some transformation.

@Ericbla
Copy link
Contributor Author

Ericbla commented Mar 14, 2018

If people want to try it. I have published this work as a new npm packge binary-parser-encoder.

@Sigurthorb
Copy link

This helped me so much, thank you!

@ab-kily
Copy link

ab-kily commented Apr 30, 2018

@Ericbla thank you for your work! You've done a very useful thing. I've tested your encoder and in most cases it works great. But there is one case it throws a TypeError. Because your repo https://github.com/Ericbla/binary-parser has no Issues ability, so I'm putting my bug report here.

Consider the following example:

const util = require('util')
const Parser = require("binary-parser-encoder").Parser;

var packet = new Parser()
    .uint16("len")
    .array("payloads", {
        type: new Parser().uint8("cmd").array('params',{
            type: new Parser().uint8('param'),
            readUntil: function(item,buffer) {
                return buffer.length == 2;
            }
        }),
        lengthInBytes: function() {
            return this.len - 4;
        },
    })
    .uint16("crc");

var test = Buffer.from("0008AAB1B2B3FFFF", "hex");
console.log(util.inspect(packet.parse(test),false,null));
console.log(util.inspect(buf = packet.encode(packet.parse(test)),false,null));

When I parse my packet, I've got the following structure:

{
    len: 8,
    payloads: [{
        cmd: 170,
        params: [{
            param: 177
        }, {
            param: 178
        }, {
            param: 179
        }]
    }],
    crc: 65535
}

But when I trying to encode it using the same parser, I get the following error:

evalmachine.<anonymous>:20
smartBuffer.writeUInt8($tmp11.param);
                             ^

TypeError: Cannot read property 'param' of undefined
    at Parser.compiledEncode (evalmachine.<anonymous>:20:30)
    at Parser.encode (/home/bagger/src/node-ssi/node_modules/binary-parser-encoder/lib/binary_parser.js:455:15)
    at Object.<anonymous> (/home/bagger/src/node-ssi/test.js:21:39)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.runMain (module.js:604:10)
    at run (bootstrap_node.js:383:7)

It seems the problem with an unusial way of using readUntil function:

            readUntil: function(item,buffer) {
                return buffer.length == 2;
            }

If I change the code above to:

            length: 3

your encoder will work fine.

So I'm interesting is there a bug with encoder, or threr is something wrong with my code?

@Ericbla
Copy link
Contributor Author

Ericbla commented Aug 3, 2018

@ab-kily Thanks for reporting this issue. (I have now activated the Issue management on my forked repo).
The problem is the support of readUntil function in array encoding. I should have mention that in the doc. You can not use the buffer parameter to do some read-ahead on the buffer you are currently generating.

To fix this, I have to ignore the readUntil function during encoding. But I will also loose the possibility to exploit this function based on the item value.

I guess I will choose this solution (as for the buffer encoding) as it makes sens from encoding perspective.

B.R.

Eric

@Ericbla
Copy link
Contributor Author

Ericbla commented Aug 3, 2018

Just published the version 1.4.0 of binary-parser-encoder package on npm registry.

Should fix the problem of @ab-kily.

Finally, I kept the readUntil (function) option when encoding arrays, but added 2 things:

  • Limit the array exploration to the number or array elements (even if readUntil is never true)
  • Added an encodeUntil (item, object) function option that overwrite the readUntilfunction when present. (Mandatory if you have a readUntil function based of the buffer read-ahead).

@gregory
Copy link

gregory commented Oct 14, 2018

@keichi @Ericbla any update on this? :) Is this going to get merged or should we use the fork?

@fxp
Copy link

fxp commented Aug 17, 2019

Any update?

@elby22
Copy link

elby22 commented Dec 18, 2019

Would love to use this feature, but I am using the fork for now. Is there anything I can do to help?

@rbiggers
Copy link

rbiggers commented Jun 3, 2020

@keichi @Ericbla Why has this PR been open for so long?

@amizan8653
Copy link

@Ericbla : thank you SO much for making this extension! I was dreading having to make an encoder myself, until I saw that you've done this for the community! 👍 I do feel like it's an extremely common use case where if you're decoding binary, you'd also like to re-encode it back.

@eliellis
Copy link

What's the word on this PR?

@keichi
Copy link
Owner

keichi commented Jun 17, 2021

My initial intention was to focus on deserialization only but I'm willing to merge this PR if many people can benefit. If someone could translate this PR to TypeScript and rebase it onto the current master, it would very much appreciated.

@rluvaton
Copy link

rluvaton commented Jul 7, 2021

@Ericbla If you want I can do it, please let me know 😀

@Yuri-M-Dias
Copy link

Hello everyone,

@Ericbla are you still willing to work on this? If @keichi is still willing to merge it, I can do the updating to get them on-par. My company has been using the encoder-branch for a long time, and we even did some customizations for it, but I wanted to improve the work a bit, and it makes absolutely no sense to work on it when it hasn't been updated for the latest version.

Thanks in advance.

@Ericbla
Copy link
Contributor Author

Ericbla commented Jul 26, 2022

Hi @Yuri-M-Dias and @keichi,

As you guessed, I'm no more able to actively maintain the endoder part of the parser.
You will find in the encoder branch of my repo (https://github.com/Ericbla/binary-parser) the last effort to reconcile the 2 projects (moving to TS), but since the usage of DataView on top of Buffer in the parser part, things tends to diverge a little bit as the encoder part relies on smart-buffer (with basic Buffer interface).

Since I'm no more using this library, and also no more involved in any JavaScript/TS project (back to legacy C++ projects !), I would prefer if someone else could continue the adventure.

It should not be too difficult form the current state of my repos (https://github.com/Ericbla/binary-parser) to merge the last modifications from @keichi parser and generate a new pull-request for the original @keichi project.

Best regards.

--
Eric

Hello everyone,

@Ericbla are you still willing to work on this? If @keichi is still willing to merge it, I can do the updating to get them on-par. My company has been using the encoder-branch for a long time, and we even did some customizations for it, but I wanted to improve the work a bit, and it makes absolutely no sense to work on it when it hasn't been updated for the latest version.

Thanks in advance.

@stereokai
Copy link

For those who are interested, I created a new PR (#243) bringing this one to the latest version of binary-parser.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet