search

gbarr / perl-Convert-ASN1

encode/decode data using ASN.1 description
http://search.cpan.org/dist/Convert-ASN1/
13 stars 21 forks source link

Patch for issue 3 #4

Closed peter-mogensen closed 12 years ago

peter-mogensen commented 12 years ago

OK... I had hoped that attachments were preserved in some way when replying to emails from github. I've never used a github account before, but it seems straight forward, so here's the patch as a pull req.

gbarr commented 12 years ago

Your patch seemed to introduce a lot of unnecessary whitespace changes to parser.pm to I re-ran mkparse here, which seemed to fix it, and did a fixup of your commit

I also tracked the cause of the encode failure, see my new master branch

gbarr commented 12 years ago

fixes #3

peter-mogensen commented 12 years ago

On 2012-05-31 21:52, Graham Barr wrote:

Your patch seemed to introduce a lot of unnecessary whitespace changes to parser.pm to I re-ran mkparse here, which seemed to fix it, and did a fixup of your commit

Yes... I forgot to mention. I wondered about that, but I installed tarball for byacc you mentioned and ran it, so I don't know why.

I also tracked the cause of the encode failure, see my new master branch

Yes... great :)

btw.. Since the parser was not an object, I took the liberty of making $tagdefault a lexical global var in parser.pm. I could see there were other non-thread safe vars.

The module now seems to eat the entire Kerberos5 spec (without module header) and generate correkt DER.

/Peter

gbarr commented 12 years ago

Thats good to hear it can now consume the Kerberos5 spec. At some point I will get it to read the module header

Maybe I have a patch on my byacc. I built it so long ago I cannot remember. I will have to figure out exactly what I used to build it and put it in git probably so others do not have the same issue

I will push a new release to CPAN today or tomorrow

Thanks Graham.

peter-mogensen commented 12 years ago

On 2012-06-01 00:33, Graham Barr wrote:

I will push a new release to CPAN today or tomorrow

I just discovered that your fix for decoding (7286d70af) seems to break the example in the docs. It no longer prints "7 string" if I include this. Please check.

Also I was wondering if it wouldn't be a nice feature if find() could return macros based on APPLICATION tag in addition to name. (this may warrent a new issue)

It seems calling decode() on a Convert::ASN1 object directly wont try to decode with the correct macro if there's more than one defined. But in principle APPLICATION tags should enable decode to chose the correct macro.

Please correct me if I'm wrong.

/Peter

gbarr commented 12 years ago

Having only one macro defined was meant to be a special case so find was not needed. With multiple macros you must call find.

A Convert::ASN1 object has a selected macro. Calling find just clones the current object and selects that macro. When there is only one macro it should be selected in the original object

Interesting thought though to find the macro given the tag of whats being decoded. I have created issue #5