to_h bug with choice element

[adfoster-r7]

Hi there 👋

I revived my LDAP branch from #31 - and spotted what appears to be a to_h bug with choice element

Replication code

require 'rasn1'

# Implements the model defined by LDAPv3
# https://www.rfc-editor.org/rfc/rfc4511
module Ldap
  class LdapModel < RASN1::Model
    def self.model_name
      name.split('::').last.to_sym
    end

    def self.message_id(name, options = {})
      custom_primitive_type_for(name, MessageId, options)
    end

    def self.ldap_string(name, options = {})
      custom_primitive_type_for(name, LdapString, options)
    end

    def self.ldap_dn(name, options = {})
      custom_primitive_type_for(name, LdapDN, options)
    end

    def self.ldap_relative_dn(name, options = {})
      custom_primitive_type_for(name, LdapString, options)
    end

    def self.custom_primitive_type_for(name, clazz, options = {})
      options.merge!(name: name)
      proc = proc do |opts|
        clazz.new(options.merge(opts))
      end
      @root = BaseElem.new(name, proc, nil)
    end

    private_class_method :custom_primitive_type_for
  end

  #
  # 4.1.2.  String Types
  #

  #        LDAPString ::= OCTET STRING -- UTF-8 encoded,
  #                                     -- [ISO10646] characters
  class LdapString < RASN1::Types::OctetString
  end

  #        LDAPDN ::= LDAPString
  #                    -- Constrained to <distinguishedName> [RFC4514]
  class LdapDN < LdapString
  end

  #
  # 4.2.  Bind Operation
  #

  #        SaslCredentials ::= SEQUENCE {
  #              mechanism               LDAPString,
  #              credentials             OCTET STRING OPTIONAL }
  class SaslCredentials < LdapModel
    sequence model_name,
             content: [
               ldap_string(:mechanism),
               octet_string(:credentials, optional: true)
             ]
  end

  #        AuthenticationChoice ::= CHOICE {
  #              simple                  [0] OCTET STRING,
  #                                      -- 1 and 2 reserved
  #              sasl                    [3] SaslCredentials,
  #              ...  }
  class AuthenticationChoice < LdapModel
    choice model_name,
             content: [
               octet_string(:simple, implicit: 0),
               wrapper(model(:sasl, SaslCredentials), implicit: 3)
             ]
  end

  #        BindRequest ::= [APPLICATION 0] SEQUENCE {
  #              version                 INTEGER (1 ..  127),
  #              name                    LDAPDN,
  #              authentication          AuthenticationChoice }
  class BindRequest < LdapModel
    sequence model_name,
             class: :application,
             implicit: 0,
             content: [
               integer(:version),
               ldap_dn(:name),
               model(:authentication, AuthenticationChoice)
             ]
  end

  #
  # 4.1.1.  Message Envelope
  #

  #        MessageID ::= INTEGER (0 ..  maxInt)
  #        maxInt INTEGER ::= 2147483647 -- (2^^31 - 1) --
  class MessageId < RASN1::Types::Integer
    # XXX: Add constrained types in accordance to the specification
  end

  #              protocolOp      CHOICE {
  #                   bindRequest           BindRequest,
  #                   bindResponse          BindResponse,
  #                   unbindRequest         UnbindRequest,
  #                   searchRequest         SearchRequest,
  #                   searchResEntry        SearchResultEntry,
  #                   searchResDone         SearchResultDone,
  #                   searchResRef          SearchResultReference,
  #                   modifyRequest         ModifyRequest,
  #                   modifyResponse        ModifyResponse,
  #                   addRequest            AddRequest,
  #                   addResponse           AddResponse,
  #                   delRequest            DelRequest,
  #                   delResponse           DelResponse,
  #                   modDNRequest          ModifyDNRequest,
  #                   modDNResponse         ModifyDNResponse,
  #                   compareRequest        CompareRequest,
  #                   compareResponse       CompareResponse,
  #                   abandonRequest        AbandonRequest,
  #                   extendedReq           ExtendedRequest,
  #                   extendedResp          ExtendedResponse,
  #                   ...,
  #                   intermediateResponse  IntermediateResponse },
  class ProtocolOp < LdapModel
    choice model_name,
           content: [
             model(:bind_request, BindRequest),
           ]
  end

  #        LDAPMessage ::= SEQUENCE {
  #              messageID       MessageID,
  #              protocolOp      CHOICE {
  #                   bindRequest           BindRequest,
  #                   bindResponse          BindResponse,
  #                   ...,
  #                   intermediateResponse  IntermediateResponse },
  #              controls       [0] Controls OPTIONAL }
  class LdapMessage < LdapModel
    sequence model_name,
             content: [
               message_id(:message_id),
               model(:protocol_op, ProtocolOp)
             ]
  end
end

input = "\x30\x2c\x02\x01\x01\x60\x27\x02\x01\x03\x04\x18\x41\x64\x6d\x69" \
  "\x6e\x69\x73\x74\x72\x61\x74\x6f\x72\x40\x61\x64\x66\x33\x2e\x6c" \
  "\x6f\x63\x61\x6c\x80\x08\x70\x34\x24\x24\x77\x30\x72\x64".b

result = RASN1.trace do
  Ldap::LdapMessage.parse(input)
end
expected = {
  LdapMessage: {
    message_id: 1,
    protocol_op: {
      version: 3,
      name: 'Administrator@adf3.local',
      authentication: {
        simple: 'p4$$w0rd'.b
      }
    }
  }
}
puts
puts "result="
pp result
puts "result.to_h="
pp result.to_h

The model's inspect is:

(LdapMessage) LdapMessage SEQUENCE:
  message_id MESSAGE ID: 1
  (ProtocolOp) ProtocolOp CHOICE:
(BindRequest) BindRequest APPLICATION [0] IMPLICIT SEQUENCE:
  version INTEGER: 3
  name LDAP DN: "Administrator@adf3.local"
  (AuthenticationChoice) AuthenticationChoice CHOICE:
simple CONTEXT [0] IMPLICIT OCTET STRING: "p4$$w0rd"

But when caling to_h - it looks like the AuthenticationChoice is going wrong:

{
  :LdapMessage=>{
    :message_id=>1,
    :protocol_op=>{
      :version=>3,
      :name=>"Administrator@adf3.local",
      # XXX: This has gone wrong:
      nil=>"p4$$w0rd"
    }
  }
}

Expected (or something closer to this):

expected = {
  LdapMessage: {
    message_id: 1,
    protocol_op: {
      version: 3,
      name: 'Administrator@adf3.local',
      authentication: {
        simple: 'p4$$w0rd'.b
      }
    }
  }
}

Which would align with wireshark's view:

[lemontree55]

Expected output is:

expected = {
  LdapMessage: {
    message_id: 1,
    protocol_op: {
      version: 3,
      name: 'Administrator@adf3.local',
      authentication: 'p4$$w0rd'.b
    }
  }
}

as to_h output set choice name instead of chosen item name. I am not sure this is the best choice I made...

But I do not understandf why you have a nil as key. Could paste you code here, as simpler as possible ?

[adfoster-r7]

Could paste you code here, as simpler as possible ?

Does pressing this button show the code for you? 👀

If not I can link to a branch with the code 👍

[adfoster-r7]

Just created a new branch with the smallest replicatable failing test to help out: #45

Failure:

       -:LdapMessage => {:message_id=>1, :protocol_op=>{:authentication=>"p4$$w0rd", :name=>"Administrator@adf3.local", :version=>3}},
       +:LdapMessage => {:message_id=>1, :protocol_op=>{:version=>3, :name=>"Administrator@adf3.local", nil=>"p4$$w0rd"}},

[adfoster-r7]

Expected output is:

expected = {
 LdapMessage: {
   message_id: 1,
   protocol_op: {
     version: 3,
     name: 'Administrator@adf3.local',
     authentication: 'p4$$w0rd'.b
   }
 }
}

That makes sense to me - thanks for the clarification! 💯

[lemontree55]

Should be fixed in #44. This branch also adds some changes on Model#to_h about choice elements.

Now, you should have:

expected = {
  LdapMessage: {
    message_id: 1,
    protocol_op: {
      bind_request: {
        version: 3,
        name: "Administrator@adf3.local",
        authentication: { simple: "p4$$w0rd"}
      }
    }
  }
}