undefined method `[]' for nil:NilClass during decode ?

[thechile]

Hi

I am using this proto https://github.com/PowerDNS/pdns/blob/master/pdns/dnsmessage.proto which i compiled to give me the following .rb.

#!/usr/bin/env ruby
# Generated by the protocol buffer compiler. DO NOT EDIT!

require 'protocol_buffers'

# forward declarations
class PBDNSMessage < ::ProtocolBuffers::Message; end

class PBDNSMessage < ::ProtocolBuffers::Message
  # forward declarations
  class DNSQuestion < ::ProtocolBuffers::Message; end
  class DNSResponse < ::ProtocolBuffers::Message; end

  # enums
  module Type
    include ::ProtocolBuffers::Enum

    set_fully_qualified_name "PBDNSMessage.Type"

    DNSQueryType = 1
    DNSResponseType = 2
    DNSOutgoingQueryType = 3
    DNSIncomingResponseType = 4
  end

  module SocketFamily
    include ::ProtocolBuffers::Enum

    set_fully_qualified_name "PBDNSMessage.SocketFamily"

    INET = 1
    INET6 = 2
  end

  module SocketProtocol
    include ::ProtocolBuffers::Enum

    set_fully_qualified_name "PBDNSMessage.SocketProtocol"

    UDP = 1
    TCP = 2
  end

  set_fully_qualified_name "PBDNSMessage"

  # nested messages
  class DNSQuestion < ::ProtocolBuffers::Message
    set_fully_qualified_name "PBDNSMessage.DNSQuestion"

    optional :string, :qName, 1
    optional :uint32, :qType, 2
    optional :uint32, :qClass, 3
  end

  class DNSResponse < ::ProtocolBuffers::Message
    # forward declarations
    class DNSRR < ::ProtocolBuffers::Message; end

    set_fully_qualified_name "PBDNSMessage.DNSResponse"

    # nested messages
    class DNSRR < ::ProtocolBuffers::Message
      set_fully_qualified_name "PBDNSMessage.DNSResponse.DNSRR"

      optional :string, :name, 1
      optional :uint32, :type, 2
      optional :uint32, :class, 3
      optional :uint32, :ttl, 4
      optional :bytes, :rdata, 5
    end

    optional :uint32, :rcode, 1
    repeated ::PBDNSMessage::DNSResponse::DNSRR, :rrs, 2
    optional :string, :appliedPolicy, 3
    repeated :string, :tags, 4
    optional :uint32, :queryTimeSec, 5
    optional :uint32, :queryTimeUsec, 6
  end

  required ::PBDNSMessage::Type, :type, 1
  optional :bytes, :messageId, 2
  optional :bytes, :serverIdentity, 3
  optional ::PBDNSMessage::SocketFamily, :socketFamily, 4
  optional ::PBDNSMessage::SocketProtocol, :socketProtocol, 5
  optional :bytes, :from, 6
  optional :bytes, :to, 7
  optional :uint64, :inBytes, 8
  optional :uint32, :timeSec, 9
  optional :uint32, :timeUsec, 10
  optional :uint32, :id, 11
  optional ::PBDNSMessage::DNSQuestion, :question, 12
  optional ::PBDNSMessage::DNSResponse, :response, 13
  optional :bytes, :originalRequestorSubnet, 14
  optional :string, :requestorId, 15
  optional :bytes, :initialRequestId, 16
end

But when i receive DNSResponse events from the DNS server it get errors e.g.

/opt/chef/embedded/lib/ruby/gems/2.3.0/gems/ruby-protocol-buffers-1.6.1/lib/protocol_buffers/runtime/
field.rb:141:in `class': undefined method `[]' for nil:NilClass (NoMethodError)

This only happens when i receive an event with one or more DNSRR entries.. when there are none it works OK.

For example a message produced on the source that works is

type: DNSResponseType
messageId: "\213\344sy\314\003OX\234\014\204\220{7\r\337"
socketFamily: INET
socketProtocol: UDP
from: "\n\204\020\017"
to: "\n\204\020\017"
inBytes: 89
timeSec: 1496380819
timeUsec: 865757
id: 55841
question {
  qName: "google.com."
  qType: 6
  qClass: 1
}
response {
  rcode: 0
  queryTimeSec: 1496380819
  queryTimeUsec: 865365
}

And one that fails is

type: DNSResponseType
messageId: "\254\251v\304\223`IV\236\245#y\003\375<\214"
socketFamily: INET
socketProtocol: UDP
from: "\n\204\020\017"
to: "\n\204\020\017"
inBytes: 87
timeSec: 1496380805
timeUsec: 897818
id: 64238
question {
  qName: "google.com."
  qType: 1
  qClass: 1
}
response {
  rcode: 0
  rrs {
    name: "google.com."
    type: 1
    class: 1
    ttl: 151
    rdata: "\254\331\013."
  }
  rrs {
    name: "google.com."
    type: 1
    class: 1
    ttl: 151
    rdata: "\254\331\013."
  }
  rrs {
    name: "google.com."
    type: 1
    class: 1
    ttl: 151
    rdata: "\254\331\013."
  }
  queryTimeSec: 1496380805
  queryTimeUsec: 896443
}

Can you suggest please if this is a problem with the compiled protobuf definition or maybe a bug ?

thank you.

[codekitchen]

This looks like mostly likely a bug in this library -- there is an internal data structure on each Message called @set_fields which looks like it is somehow not getting initialized. I'll see if I can come up with a small repro case.

[codekitchen]

Ah actually I tracked it down but it's not what I stated above. The DNSRR message has a field defined as optional uint32 class = 3;, which ends up overriding the ruby built-in method class causing havoc at runtime.

I think ideally this library would detect when a conflict like this occurs... I'm not sure what the best way to handle it is, though. Maybe just rename the field to add a prefix underscore, so it ends up being _class or something like that? That seems potentially confusing but I'm not sure what a better option would be.

In the meantime, can you rename this field in your proto definition? That will work around the problem.

[thechile]

OK, many thanks for identifying the problem. I can create issue for DNS server repo and suggest change of name for that field but i don't know if they will be interested in fixing or accepting a PR if i send.

I think it would be quicker to update this library. If you can point me in direction of culprit code i can try for PR ?

[codekitchen]

Thinking about it more, Message is really a value type and we should treat it as such, so we should allow methods like .class to be defined without mangling them. We should also inherit from BasicObject now that ruby has such a concept (this library was originally written for ruby 1.8).

That said, there's quite a few places where code will have to change -- I see around 18 calls to self.class in message.rb, along with a few calls in text_parser.ry and field.rb too. That'd be awesome if you're up for tackling it, but it'll be a larger change. It will probably be a week or two before I can tackle it myself.

One other workaround for now would be to rename the field in just your local .proto or just the .pb.rb file -- the field names don't actually matter in serialized data, so you'll still be able to interact with code that uses the unchanged definition.

[thechile]

OK thanks.. for quick win i deleted the class field from proto since it's not really useful to me and everything now works. Perfect.

I don't know if my Ruby knowledge is sufficient to realise the needed changes but if i get time i'll take a punt.. if you don't get PR within a week then know i failed miserably and will need your help.

[codekitchen]

haha, totally understand