Merge pull request #31 from n-rodriguez/fix/v1trap-hierarchy

refactor: V1Trap derives from PDU, not the v2 Trap

Author: stakach

spec/notifications_spec.cr

@@ -45,7 +45,7 @@ describe SNMP::Session do
     parsed.version.should eq(SNMP::Version::V1)
     parsed.request.should eq(SNMP::Request::V1_Trap)
     trap = parsed.pdu.as(SNMP::V1Trap)
-    trap.oid.should eq("1.3.6.1.4.1.9")
+    trap.enterprise.should eq("1.3.6.1.4.1.9")
     trap.agent_address.should eq("10.0.0.1")
     trap.generic_trap.should eq(SNMP::GenericTrap::LinkDown)
     trap.specific_trap.should eq(0)

spec/snmp_spec.cr

@@ -16,7 +16,9 @@ describe SNMP do
 
     snmp_pdu = snmp.pdu
     if snmp_pdu.is_a?(SNMP::V1Trap)
-      snmp_pdu.oid.should eq("1.3.6.1.6.3.1.1.5")
+      snmp_pdu.enterprise.should eq("1.3.6.1.6.3.1.1.5")
+      # PDU#oid keeps its standard meaning: the first varbind's OID
+      snmp_pdu.oid.should eq("1.3.6.1.2.1.2.2.1.1.26")
       snmp_pdu.agent_address.should eq("10.230.254.28")
       snmp_pdu.generic_trap.should eq(SNMP::GenericTrap::LinkUp)
       snmp_pdu.specific_trap.should eq(0)

spec/v1_trap_spec.cr

@@ -0,0 +1,20 @@
+require "./helper"
+
+describe SNMP::V1Trap do
+  it "is a PDU but not a v2 Trap (RFC 1157 traps are their own PDU variant)" do
+    trap = SNMP::V1Trap.new("10.0.0.1", SNMP::GenericTrap::LinkDown, 0, enterprise: "1.3.6.1.4.1.9")
+    trap.is_a?(SNMP::PDU).should be_true
+    trap.is_a?(SNMP::Trap).should be_false
+  end
+
+  it "exposes the enterprise OID as #enterprise, keeping PDU#oid for the first varbind" do
+    vb = SNMP::VarBind.new("1.3.6.1.2.1.2.2.1.1.26")
+    vb.value.set_integer(26)
+    trap = SNMP::V1Trap.new("10.0.0.1", SNMP::GenericTrap::LinkUp, 0,
+      enterprise: "1.3.6.1.4.1.9", time_ticks: 42_u32, varbinds: [vb])
+
+    trap.enterprise.should eq("1.3.6.1.4.1.9")
+    trap.oid.should eq("1.3.6.1.2.1.2.2.1.1.26")
+    trap.time_ticks.should eq(42)
+  end
+end

src/snmp/session.cr

@@ -100,7 +100,7 @@ class SNMP::Session
   # enterprise OID, *agent_address* a dotted-quad IPv4 string.
   def trap_v1(enterprise, agent_address, generic_trap : GenericTrap, specific_trap = 0, uptime = 0, varbinds : Array(VarBind) = [] of VarBind, request_id = rand(REQUEST_ID_RANGE))
     pdu = V1Trap.new(agent_address, generic_trap, specific_trap.to_i32,
-      oid: enterprise, time_ticks: uptime.to_u32, varbinds: varbinds, request_id: request_id)
+      enterprise: enterprise, time_ticks: uptime.to_u32, varbinds: varbinds, request_id: request_id)
     SNMP::Message.new(@community, Request::V1_Trap, pdu, version: Version::V1)
   end
 

src/snmp/v1_trap.cr

@@ -1,7 +1,12 @@
-class SNMP::V1Trap < SNMP::Trap
+# An RFC 1157 Trap-PDU. It is its own variant of the protocol's PDU CHOICE —
+# not an SNMPv2 trap — so it derives from PDU directly and carries the v1
+# fields (enterprise, agent address, generic/specific trap, timestamp) itself.
+# The v2-style request-id / error fields inherited from PDU have no meaning in
+# a v1 trap and stay at their defaults.
+class SNMP::V1Trap < SNMP::PDU
   def initialize(ber : ASN1::BER)
     pdu = SNMP.ber_fields(ber, 6, "v1 trap")
-    @oid = pdu[0].get_object_id
+    @enterprise = pdu[0].get_object_id
     @agent_address = pdu[1].payload.join(".")
     @generic_trap = SNMP.decode_enum(GenericTrap, pdu[2].get_integer, "generic-trap")
     @specific_trap = pdu[3].get_integer.to_i32
@@ -10,25 +15,26 @@ class SNMP::V1Trap < SNMP::Trap
       VarBind.new(varbind)
     end
 
-    # Compatibility with regular PDUs
-    # V1 traps are very different: https://tools.ietf.org/html/rfc1157#page-27
+    # The v2-style PDU fields do not exist on the v1 wire
     @request_id = 0
     @error_status = ErrorStatus::NoError
     @error_index = 0
   end
 
+  property enterprise : String
   property agent_address : String
   property generic_trap : GenericTrap
   property specific_trap : Int32
+  property time_ticks : UInt32
 
-  def initialize(@agent_address, @generic_trap, @specific_trap, **args)
-    super(**args)
+  def initialize(@agent_address, @generic_trap, @specific_trap = 0, @enterprise = "", @time_ticks = 0_u32, varbinds : Array(VarBind) = [] of VarBind, request_id = 0)
+    super(request_id, varbinds)
   end
 
   # RFC 1157 Trap-PDU wire structure — distinct from the standard PDU layout, so
-  # this overrides `PDU#to_ber`. `@oid` holds the enterprise OID.
+  # this overrides `PDU#to_ber`.
   def to_ber(tag_number)
-    enterprise = ASN1::BER.new.set_object_id(@oid)
+    enterprise = ASN1::BER.new.set_object_id(@enterprise)
     agent = IpAddress.new(@agent_address).to_ber
     generic = ASN1::BER.new.set_integer(@generic_trap.to_i)
     specific = ASN1::BER.new.set_integer(@specific_trap)