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

Using variables from parent scope in a 'choice' #10

Closed
ddaf opened this issue Jan 7, 2015 · 15 comments · Fixed by #166
Closed

Using variables from parent scope in a 'choice' #10

ddaf opened this issue Jan 7, 2015 · 15 comments · Fixed by #166

Comments

@ddaf
Copy link

ddaf commented Jan 7, 2015

Hey Keichi

I was wondering if it's somehow possible to use a variable from a parent scope from inside a choice type item. If it's not implemented I would greatly appreciate a pointer on how to do so.
My use-case is parsing of a DNSRecord:

var TXTRecord = new Parser()
    .string('txt' { length: 'rdlength' }) // rdlength is undefined

var answer = new Parser()
    .nest('name', { type: fqdn_pretty })
    .uint16be('type')
    .uint16be('class')
    .uint32be('ttl')
    .uint16be('rdlength')
    .choice('record', {
        tag: 'type',
        choices: {
            0x01: ARecord,
            0x02: PTRRecord, 0x05: PTRRecord, 0x0c: PTRRecord,
            0x10: TXTRecord,
            0x21: SRVRecord,
            0x1C: AAAARecord
        },
        defaultChoice: new Parser().buffer('rdata', { length: 'rdlength' }) //rdlength is undefined
    })

I enjoy working with your module, thanks for the help in advance!

@keichi
Copy link
Owner

keichi commented Jan 7, 2015

I understand your situation. Unfortunately, there's currently no way to do that. One idea would be to introduce a special variable name like $parent, with which you can reference the parent scope:
defaultChoice: new Parser().buffer('rdata', { length: '$parent.rdlength' })
What do you think about this?

@ddaf
Copy link
Author

ddaf commented Jan 7, 2015

Hmm yes, if you in each scope add a parent variable that points to the outer scope that would make so many more things possible.

You could even crawl up several scopes if needed. Fx. parent.parent.rdlength in a record subtype.

@keichi
Copy link
Owner

keichi commented Jan 7, 2015

So commit 470b366 is the implementation I could come up with so far. This allows you to write parsers like:

var parser = new Parser()
    .uint8('length')
    .uint8('tag')
    .choice('data', {
        tag: 'tag',
        choices: {
            0: new Parser().array('arr', {type: 'uint8', length: '$parent.length'})
        },
        defaultChoice: new Parser()
            .string('name', {length: function() { return this.$parent.length;}})
    });

However, this is obviously circular referencing and causes memory leaking, so I have to find some other ways to implement it... I will give a try, but any suggestions are welcome.

@ddaf
Copy link
Author

ddaf commented Jan 29, 2015

Hey, sorry for coming back to you so late. I came up with an implementation for $parent: ddaf@297202d

Not creating any new vars, just being crafty in ctx.generateVariable. Would this solve the issue of circular referencing?

@ddaf
Copy link
Author

ddaf commented Feb 12, 2015

If you think the implementation is acceptable I'll pretty it up, add tests and send a pull request

@keichi
Copy link
Owner

keichi commented Feb 24, 2015

Hi, sorry for the extreme late reply. Your implementation looks simple and nice to me. However, I'm afraid it would not work if you want to reference parent variables inside functions. For example:

var TXTRecord = new Parser()
    .string('txt', { length: function() {return $parent.recordLen - 1;} });

Any ideas on this? A quick and dirty implementation would be to do string replacement on the result of Function.toString()...

@autarc
Copy link

autarc commented Jul 11, 2015

Considering the options so far I've came up with another solution: both (array and choice) gets an optional parameter called '$parent', which uses an array to define the properties that should be shared by the parent scope. The declarative approach allows to avoid circular references and keep the actual overhead to a minimum: autarc@5dd2752

Whats your opinion ?

@tevaum
Copy link

tevaum commented Feb 1, 2016

I'm using the stuff @autarc implemented and it works like a charm. Will this commit be merged into master?

@rjromay
Copy link

rjromay commented Apr 22, 2016

@autarc solution works for me. The only problem is final object ends up with a lot of '$parent' parameters containing duplicated properties. Would it be possible to delete all $parent fields after completing all parse chain?

@autarc
Copy link

autarc commented Apr 23, 2016

@keichi #17 / #18 are still open to be reviewed and merged :)

@rjromay Its possible, but I would recommend to not include it as the logic is mainly defined for iterations through enhancing the generating of 'choice' and 'array' code. Though it shouldn't be a problem to reduce the output afterwards and remove the $parent references.

@lfk
Copy link

lfk commented Nov 13, 2016

I'm also finding myself wanting this sort of functionality -- in particular the approach of listing which variables the "child parser" should have access to seems clean to me (if the child simply can't inherit all of the previously parsed values by default).

In my particular case I can solve the issue by using the readUntil: (item, buffer) => {} approach to perform read-ahead and stop at the right moment, but simply passing down a dlen value would be much cleaner.

EDIT: Slight revision to my above statement, it turns out it's actually not entirely solved for me with readUntil since this will always read at least one of the defined types, whereas I might want it to read 0. :(

@HarleyGolfGuy
Copy link

HarleyGolfGuy commented Jun 13, 2017

Has this been implemented yet? +1 vote for me on this issue.

@jdbool
Copy link

jdbool commented Mar 25, 2019

Need this.

@panuhorsmalahti
Copy link

I would also like to use this feature, and I think it's a pretty common one use case (e.g. decoding a payload with a different parser).

@avi91
Copy link

avi91 commented Jul 2, 2020

+1

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 a pull request may close this issue.

10 participants