gbarr/perl-Convert-ASN1

Find macro by tag

Opened this issue · 6 comments

Add a method to find a macro by its tag.

This could be used within decode if called on an object that has no selected macro

I think I'll have a try at this if I get time... The alternative seems to be external peeking into the response before decoding or changing the ASN.1 to wrap all req/resp. in the same macro as you did with Net::LDAP::Message.

... had a look at this.
Finding a macro by tag is easy enough. But actually decoding and returning a useful object might require more API changes
As far as I can see there's way for decode to tell the application which macro it found and thus the structure of the return data. decode() returns in the current API only an unblessed reference to the data. Not anything indicating the macro name.
So there's some API design issues which needs consideration.

Ok... given that the application needs to know which macro is used to decode a PDU and a simple way to do that without changing existing API is to only provide methods to return the macro name (which could then be used with find() - I suggest adding subs like this to ASN1.pm

sub macro_for_application_tag {
    my $self = shift;
    my $tag  = shift; # APPLICATION tag number
    my $tagcode = asn_encode_tag(asn_tag( ASN_APPLICATION,$tag ));
    foreach my $macro (keys %{$self->{tree}}) {
        my $m = $self->{tree}{$macro};
        my $mtag = $m->[0][cTAG];

        # don't care if it's P/C
        substr($mtag,0,1) = chr(ord(substr($mtag,0,1)) & (~ASN_CONSTRUCTOR));

        return $macro if $tagcode eq $mtag;
    }
    Carp::croak("Application tag not found");
}

sub macro_for_pdu {
    my $self = shift;
    # pdu now in $_[0]
    my $t = ord(substr($_[0],0,1));
    unless (($t & 0xC0) == 0x40) {
        Carp::croak("PDU doesn't start with APPLICATION tag");
    }
    my ($n,$tag) = asn_decode_tag(@_);
    foreach my $macro (keys %{$self->{tree}}) {
        my $m = $self->{tree}{$macro};
        my $mtag = $m->[0][cTAG];

        # don't care if it's P/C
        substr($mtag,0,1) = chr(ord(substr($mtag,0,1)) & (~ASN_CONSTRUCTOR));

        return $macro if substr($_[0],0,$n) eq substr($mtag,0,$n);
    }
}

Actually... I seem to manage fine by just calling asn_decode_tag and maintaining a hash of APPLICATION tags to Macro names in my application.
My original thought was to have decode returned an object blessed to a macro specific class, but I could easily do that in the application as well by just blessing the return hash to something I generate from the macro name.
So I'm not sure this issue actually solves any unsolvable problems in the end. - unless it's really hard for you to maintain the APPLICATION-tag=>Macro-name hash.

I am ok with adding the subs, except they should end with return undef, not croak. It should be up to the application to croak if it wants to

On 2012-06-12 23:26, Graham Barr wrote:

I am ok with adding the subs, except they should end with return undef, not croak. It should be up to the application to croak if it wants to

Of course...
A better implementation of them would probably be to simply maintain a
hash (APPLICATIONTAG => macroname) after parsing and just look it up
instead of iterating over all macros.

The reason I posted the example here and not sumbmitted a patch was that
there's probably so many ways to do this that it's a design choice.

Right now I'm just calling asn_decode_tag on the incoming PDU and then
looking up the macro name by tag in a hash in the application code. -
then calling find().

The problem I wanted to solve was when having an incoming network package with a PDU of unknown type but with an application tag. I think my current solution could be cleaner if I could get the TAG=>macro hash from Convert::ASN1 instead of manually creating and hardcoding it, but apart from that it's working just as well as the above subs.
There's no reason to add API which doesn't solve any real problem and would only end up being deprecated. So I'm a little in doubt here.

/Peter