Ensure TRAP PDU consistency in v1arch

Also, consistency ensuring code unified with v3arch piece what
has the side effect of *requiring* snmpTrapOID to be always
present anywhere among user-supplied variable-bindings.

5 files changed, 186 insertions, 114 deletions

diff --git a/pysnmp/entity/rfc3413/ntforg.py b/pysnmp/entity/rfc3413/ntforg.py
index be0d0dbf..c4e84a53 100644
--- a/pysnmp/entity/rfc3413/ntforg.py
+++ b/pysnmp/entity/rfc3413/ntforg.py
@@ -204,42 +204,60 @@ class NotificationOriginator(object):
             'sendVarBinds: notificationTarget %s, contextEngineId %s, contextName "%s", varBinds %s' % (
                 notificationTarget, contextEngineId or '<default>', contextName, varBinds))
 
+        mibBuilder = snmpEngine.msgAndPduDsp.mibInstrumController.mibBuilder
+
         if contextName:
-            __SnmpAdminString, = snmpEngine.msgAndPduDsp.mibInstrumController.mibBuilder.importSymbols(
+            __SnmpAdminString, = mibBuilder.importSymbols(
                 'SNMP-FRAMEWORK-MIB', 'SnmpAdminString')
             contextName = __SnmpAdminString(contextName)
 
         # 3.3
-        (notifyTag, notifyType) = config.getNotificationInfo(snmpEngine, notificationTarget)
+        notifyTag, notifyType = config.getNotificationInfo(
+            snmpEngine, notificationTarget)
 
         notificationHandle = getNextHandle()
 
         debug.logger & debug.FLAG_APP and debug.logger(
-            'sendVarBinds: notificationHandle %s, notifyTag %s, notifyType %s' % (
-                notificationHandle, notifyTag, notifyType))
+            'sendVarBinds: notificationHandle %s, notifyTag %s, '
+            'notifyType %s' % (notificationHandle, notifyTag, notifyType))
 
         varBinds = [(v2c.ObjectIdentifier(x), y) for x, y in varBinds]
 
         # 3.3.2 & 3.3.3
-        snmpTrapOID, sysUpTime = snmpEngine.msgAndPduDsp.mibInstrumController.mibBuilder.importSymbols('__SNMPv2-MIB',
-                                                                                                       'snmpTrapOID',
-                                                                                                       'sysUpTime')
-
-        for idx in range(len(varBinds)):
-            if idx and varBinds[idx][0] == sysUpTime.getName():
-                if varBinds[0][0] == sysUpTime.getName():
-                    varBinds[0] = varBinds[idx]
+        snmpTrapOID, sysUpTime = mibBuilder.importSymbols(
+            '__SNMPv2-MIB', 'snmpTrapOID', 'sysUpTime')
+
+        snmpTrapOID = snmpTrapOID.getName()
+        sysUpTime, uptime = sysUpTime.getName(), sysUpTime.getSyntax()
+
+        # Add sysUpTime if not present already
+        if not varBinds or varBinds[0][0] != sysUpTime:
+            varBinds.insert(0, (v2c.ObjectIdentifier(sysUpTime), uptime.clone()))
+
+        # Search for and reposition sysUpTime if it's elsewhere
+        for idx, varBind in enumerate(varBinds[1:]):
+            if varBind[0] == sysUpTime:
+                varBinds[0] = varBind
+                del varBinds[idx]
+                break
+
+        if len(varBinds) < 2:
+            raise error.PySnmpError('SNMP notification PDU requires '
+                                    'SNMPv2-MIB::snmpTrapOID.0 to be present')
+
+        # Search for and reposition snmpTrapOID if it's elsewhere
+        for idx, varBind in enumerate(varBinds[2:]):
+            if varBind[0] == snmpTrapOID:
+                del varBinds[idx]
+                if varBinds[1][0] == snmpTrapOID:
+                    varBinds[1] = varBind
                 else:
-                    varBinds.insert(0, varBinds[idx])
-                    del varBinds[idx]
-
-            if varBinds[0][0] != sysUpTime.getName():
-                varBinds.insert(0, (v2c.ObjectIdentifier(sysUpTime.getName()),
-                                    sysUpTime.getSyntax().clone()))
+                    varBinds.insert(1, varBind)
+                break
 
-        if len(varBinds) < 2 or varBinds[1][0] != snmpTrapOID.getName():
-            varBinds.insert(1, (v2c.ObjectIdentifier(snmpTrapOID.getName()),
-                                snmpTrapOID.getSyntax()))
+        if varBinds[1][0] != snmpTrapOID:
+            raise error.PySnmpError('SNMP notification PDU requires '
+                                    'SNMPv2-MIB::snmpTrapOID.0 to be present')
 
         sendRequestHandle = -1
 
@@ -265,7 +283,7 @@ class NotificationOriginator(object):
                     securityName, securityLevel))
 
             for varName, varVal in varBinds:
-                if varName in (sysUpTime.name, snmpTrapOID.name):
+                if varName in (sysUpTime, snmpTrapOID):
                     continue
                 try:
                     snmpEngine.accessControlModel[self.ACM_ID].isAccessAllowed(
diff --git a/pysnmp/hlapi/v1arch/asyncio/ntforg.py b/pysnmp/hlapi/v1arch/asyncio/ntforg.py
index 25d831e9..96005813 100644
--- a/pysnmp/hlapi/v1arch/asyncio/ntforg.py
+++ b/pysnmp/hlapi/v1arch/asyncio/ntforg.py
@@ -4,18 +4,19 @@
 # Copyright (c) 2005-2019, Ilya Etingof <etingof@gmail.com>
 # License: http://snmplabs.com/pysnmp/license.html
 #
-from pysnmp.hlapi.v1arch.auth import *
-from pysnmp.hlapi.varbinds import *
-from pysnmp.hlapi.v1arch.asyncio.transport import *
-from pysnmp.smi.rfc1902 import *
-from pysnmp.proto import api
-
 try:
     import asyncio
 
 except ImportError:
     import trollius as asyncio
 
+from pysnmp.hlapi.v1arch.auth import *
+from pysnmp.hlapi.varbinds import *
+from pysnmp.hlapi.v1arch.asyncio.transport import *
+from pysnmp.smi.rfc1902 import *
+from pysnmp.proto.api import v2c
+from pysnmp.proto.proxy import rfc2576
+
 __all__ = ['sendNotification']
 
 VB_PROCESSOR = NotificationOriginatorVarBinds()
@@ -54,24 +55,27 @@ def sendNotification(snmpDispatcher, authData, transportTarget,
         or :py:class:`~pysnmp.smi.rfc1902.ObjectType` class instances
         of :py:class:`~pysnmp.smi.rfc1902.NotificationType` objects.
 
-        SNMP Notification PDU places rigid requirement on the ordering of
-        the variable-bindings.
-
-        Mandatory variable-bindings:
+        Besides user variable-bindings, SNMP Notification PDU requires at
+        least two variable-bindings to be present:
 
         0. SNMPv2-MIB::sysUpTime.0 = <agent uptime>
-        1. SNMPv2-SMI::snmpTrapOID.0 = {SNMPv2-MIB::coldStart, ...}
+        1. SNMPv2-SMI::snmpTrapOID.0 = <notification ID>
 
-        Optional variable-bindings (applicable to SNMP v1 TRAP):
+        When sending SNMPv1 TRAP, more variable-bindings could be present:
 
         2. SNMP-COMMUNITY-MIB::snmpTrapAddress.0 = <agent-IP>
         3. SNMP-COMMUNITY-MIB::snmpTrapCommunity.0 = <snmp-community-name>
         4. SNMP-COMMUNITY-MIB::snmpTrapEnterprise.0 = <enterprise-OID>
 
-        Informational variable-bindings:
+        If user does not supply some or any of the above variable-bindings or
+        if they are at the wrong positions, the system will add/reorder the
+        missing ones automatically.
 
-        * SNMPv2-SMI::NOTIFICATION-TYPE
-        * SNMPv2-SMI::OBJECT-TYPE
+        On top of that, some notification types imply including some additional
+        variable-bindings providing additional details on the event being
+        reported. Therefore it is generally easier to use
+        :py:class:`~pysnmp.smi.rfc1902.NotificationType` object which will
+        help adding relevant variable-bindings.
 
     Other Parameters
     ----------------
@@ -125,14 +129,50 @@ def sendNotification(snmpDispatcher, authData, transportTarget,
     >>>
     """
 
+    sysUpTime = v2c.apiTrapPDU.sysUpTime
+    snmpTrapOID = v2c.apiTrapPDU.snmpTrapOID
+
+    def _ensureVarBinds(varBinds):
+        # Add sysUpTime if not present already
+        if not varBinds or varBinds[0][0] != sysUpTime:
+            varBinds.insert(0, (v2c.ObjectIdentifier(sysUpTime), v2c.TimeTicks(0)))
+
+        # Search for and reposition sysUpTime if it's elsewhere
+        for idx, varBind in enumerate(varBinds[1:]):
+            if varBind[0] == sysUpTime:
+                varBinds[0] = varBind
+                del varBinds[idx]
+                break
+
+        if len(varBinds) < 2:
+            raise error.PySnmpError('SNMP notification PDU requires '
+                                    'SNMPv2-MIB::snmpTrapOID.0 to be present')
+
+        # Search for and reposition snmpTrapOID if it's elsewhere
+        for idx, varBind in enumerate(varBinds[2:]):
+            if varBind[0] == snmpTrapOID:
+                del varBinds[idx]
+                if varBinds[1][0] == snmpTrapOID:
+                    varBinds[1] = varBind
+                else:
+                    varBinds.insert(1, varBind)
+                break
+
+        # Fail on missing snmpTrapOID
+        if varBinds[1][0] != snmpTrapOID:
+            raise error.PySnmpError('SNMP notification PDU requires '
+                                    'SNMPv2-MIB::snmpTrapOID.0 to be present')
+
+        return varBinds
+
     def _cbFun(snmpDispatcher, stateHandle, errorIndication, rspPdu, _cbCtx):
         if future.cancelled():
             return
 
-        errorStatus = pMod.apiTrapPDU.getErrorStatus(rspPdu)
-        errorIndex = pMod.apiTrapPDU.getErrorIndex(rspPdu)
+        errorStatus = v2c.apiTrapPDU.getErrorStatus(rspPdu)
+        errorIndex = v2c.apiTrapPDU.getErrorIndex(rspPdu)
 
-        varBinds = pMod.apiTrapPDU.getVarBinds(rspPdu)
+        varBinds = v2c.apiTrapPDU.getVarBinds(rspPdu)
 
         try:
             varBindsUnmade = VB_PROCESSOR.unmakeVarBinds(snmpDispatcher.cache, varBinds,
@@ -154,31 +194,17 @@ def sendNotification(snmpDispatcher, authData, transportTarget,
     if lookupMib:
         varBinds = VB_PROCESSOR.makeVarBinds(snmpDispatcher.cache, varBinds)
 
-    # # make sure required PDU payload is in place
-    # completeVarBinds = []
-    #
-    # # ensure sysUpTime
-    # if len(varBinds) < 1 or varBinds[0][0] != pMod.apiTrapPDU.sysUpTime:
-    #     varBinds.insert(0, (ObjectIdentifier(pMod.apiTrapPDU.sysUpTime), pMod.Integer(0)))
-    #
-    # # ensure sysUpTime
-    # if len(varBinds) < 1 or varBinds[0][0] != pMod.apiTrapPDU.sysUpTime:
-    #     varBinds.insert(0, (ObjectIdentifier(pMod.apiTrapPDU.sysUpTime), pMod.Integer(0)))
-    #
-    # # ensure snmpTrapOID
-    # if len(varBinds) < 2 or varBinds[1][0] != pMod.apiTrapPDU.snmpTrapOID:
-    #     varBinds.insert(0, (ObjectIdentifier(pMod.apiTrapPDU.sysUpTime), pMod.Integer(0)))
-
-    # input PDU is always v2c
-    pMod = api.PROTOCOL_MODULES[api.SNMP_VERSION_2C]
-
     if notifyType == 'trap':
-        reqPdu = pMod.TrapPDU()
+        reqPdu = v2c.TrapPDU()
     else:
-        reqPdu = pMod.InformRequestPDU()
+        reqPdu = v2c.InformRequestPDU()
+
+    v2c.apiTrapPDU.setDefaults(reqPdu)
+    v2c.apiTrapPDU.setVarBinds(reqPdu, varBinds)
+
+    varBinds = v2c.apiTrapPDU.getVarBinds(reqPdu)
 
-    pMod.apiTrapPDU.setDefaults(reqPdu)
-    pMod.apiTrapPDU.setVarBinds(reqPdu, varBinds)
+    v2c.apiTrapPDU.setVarBinds(reqPdu, _ensureVarBinds(varBinds))
 
     if authData.mpModel == 0:
         reqPdu = rfc2576.v2ToV1(reqPdu)
diff --git a/pysnmp/hlapi/v1arch/asyncore/ntforg.py b/pysnmp/hlapi/v1arch/asyncore/ntforg.py
index d68d87bc..b532184a 100644
--- a/pysnmp/hlapi/v1arch/asyncore/ntforg.py
+++ b/pysnmp/hlapi/v1arch/asyncore/ntforg.py
@@ -8,7 +8,7 @@ from pysnmp.hlapi.v1arch.auth import *
 from pysnmp.hlapi.v1arch.asyncore import *
 from pysnmp.hlapi.varbinds import *
 from pysnmp.smi.rfc1902 import *
-from pysnmp.proto import api
+from pysnmp.proto.api import v2c
 from pysnmp.proto.proxy import rfc2576
 from pysnmp import error
 
@@ -48,24 +48,27 @@ def sendNotification(snmpDispatcher, authData, transportTarget,
         or :py:class:`~pysnmp.smi.rfc1902.ObjectType` class instances
         of :py:class:`~pysnmp.smi.rfc1902.NotificationType` objects.
 
-        SNMP Notification PDU places rigid requirement on the ordering of
-        the variable-bindings.
-
-        Mandatory variable-bindings:
+        Besides user variable-bindings, SNMP Notification PDU requires at
+        least two variable-bindings to be present:
 
         0. SNMPv2-MIB::sysUpTime.0 = <agent uptime>
-        1. SNMPv2-SMI::snmpTrapOID.0 = {SNMPv2-MIB::coldStart, ...}
+        1. SNMPv2-SMI::snmpTrapOID.0 = <notification ID>
 
-        Optional variable-bindings (applicable to SNMP v1 TRAP):
+        When sending SNMPv1 TRAP, more variable-bindings could be present:
 
         2. SNMP-COMMUNITY-MIB::snmpTrapAddress.0 = <agent-IP>
         3. SNMP-COMMUNITY-MIB::snmpTrapCommunity.0 = <snmp-community-name>
         4. SNMP-COMMUNITY-MIB::snmpTrapEnterprise.0 = <enterprise-OID>
 
-        Informational variable-bindings:
+        If user does not supply some or any of the above variable-bindings or
+        if they are at the wrong positions, the system will add/reorder the
+        missing ones automatically.
 
-        * SNMPv2-SMI::NOTIFICATION-TYPE
-        * SNMPv2-SMI::OBJECT-TYPE
+        On top of that, some notification types imply including some additional
+        variable-bindings providing additional details on the event being
+        reported. Therefore it is generally easier to use
+        :py:class:`~pysnmp.smi.rfc1902.NotificationType` object which will
+        help adding relevant variable-bindings.
 
     Other Parameters
     ----------------
@@ -116,6 +119,42 @@ def sendNotification(snmpDispatcher, authData, transportTarget,
     >>> snmpDispatcher.transportDispatcher.runDispatcher()
     """
 
+    sysUpTime = v2c.apiTrapPDU.sysUpTime
+    snmpTrapOID = v2c.apiTrapPDU.snmpTrapOID
+
+    def _ensureVarBinds(varBinds):
+        # Add sysUpTime if not present already
+        if not varBinds or varBinds[0][0] != sysUpTime:
+            varBinds.insert(0, (v2c.ObjectIdentifier(sysUpTime), v2c.TimeTicks(0)))
+
+        # Search for and reposition sysUpTime if it's elsewhere
+        for idx, varBind in enumerate(varBinds[1:]):
+            if varBind[0] == sysUpTime:
+                varBinds[0] = varBind
+                del varBinds[idx]
+                break
+
+        if len(varBinds) < 2:
+            raise error.PySnmpError('SNMP notification PDU requires '
+                                    'SNMPv2-MIB::snmpTrapOID.0 to be present')
+
+        # Search for and reposition snmpTrapOID if it's elsewhere
+        for idx, varBind in enumerate(varBinds[2:]):
+            if varBind[0] == snmpTrapOID:
+                del varBinds[idx]
+                if varBinds[1][0] == snmpTrapOID:
+                    varBinds[1] = varBind
+                else:
+                    varBinds.insert(1, varBind)
+                break
+
+        # Fail on missing snmpTrapOID
+        if varBinds[1][0] != snmpTrapOID:
+            raise error.PySnmpError('SNMP notification PDU requires '
+                                    'SNMPv2-MIB::snmpTrapOID.0 to be present')
+
+        return varBinds
+
     def _cbFun(snmpDispatcher, stateHandle, errorIndication, rspPdu, _cbCtx):
         if not cbFun:
             return
@@ -125,10 +164,10 @@ def sendNotification(snmpDispatcher, authData, transportTarget,
                   cbCtx=cbCtx, snmpDispatcher=snmpDispatcher, stateHandle=stateHandle)
             return
 
-        errorStatus = pMod.apiTrapPDU.getErrorStatus(rspPdu)
-        errorIndex = pMod.apiTrapPDU.getErrorIndex(rspPdu)
+        errorStatus = v2c.apiTrapPDU.getErrorStatus(rspPdu)
+        errorIndex = v2c.apiTrapPDU.getErrorIndex(rspPdu)
 
-        varBinds = pMod.apiTrapPDU.getVarBinds(rspPdu)
+        varBinds = v2c.apiTrapPDU.getVarBinds(rspPdu)
 
         if lookupMib:
             varBinds = VB_PROCESSOR.unmakeVarBinds(snmpDispatcher.cache, varBinds)
@@ -144,8 +183,8 @@ def sendNotification(snmpDispatcher, authData, transportTarget,
         if not nextVarBinds:
             return
 
-        pMod.apiTrapPDU.setRequestID(reqPdu, nextStateHandle)
-        pMod.apiTrapPDU.setVarBinds(reqPdu, nextVarBinds)
+        v2c.apiTrapPDU.setRequestID(reqPdu, nextStateHandle)
+        v2c.apiTrapPDU.setVarBinds(reqPdu, _ensureVarBinds(nextVarBinds))
 
         return snmpDispatcher.sendPdu(authData, transportTarget, reqPdu, cbFun=_cbFun)
 
@@ -154,31 +193,17 @@ def sendNotification(snmpDispatcher, authData, transportTarget,
     if lookupMib:
         varBinds = VB_PROCESSOR.makeVarBinds(snmpDispatcher.cache, varBinds)
 
-    # # make sure required PDU payload is in place
-    # completeVarBinds = []
-    #
-    # # ensure sysUpTime
-    # if len(varBinds) < 1 or varBinds[0][0] != pMod.apiTrapPDU.sysUpTime:
-    #     varBinds.insert(0, (ObjectIdentifier(pMod.apiTrapPDU.sysUpTime), pMod.Integer(0)))
-    #
-    # # ensure sysUpTime
-    # if len(varBinds) < 1 or varBinds[0][0] != pMod.apiTrapPDU.sysUpTime:
-    #     varBinds.insert(0, (ObjectIdentifier(pMod.apiTrapPDU.sysUpTime), pMod.Integer(0)))
-    #
-    # # ensure snmpTrapOID
-    # if len(varBinds) < 2 or varBinds[1][0] != pMod.apiTrapPDU.snmpTrapOID:
-    #     varBinds.insert(0, (ObjectIdentifier(pMod.apiTrapPDU.sysUpTime), pMod.Integer(0)))
-
-    # input PDU is always v2c
-    pMod = api.PROTOCOL_MODULES[api.SNMP_VERSION_2C]
-
     if notifyType == 'trap':
-        reqPdu = pMod.TrapPDU()
+        reqPdu = v2c.TrapPDU()
     else:
-        reqPdu = pMod.InformRequestPDU()
+        reqPdu = v2c.InformRequestPDU()
+
+    v2c.apiTrapPDU.setDefaults(reqPdu)
+    v2c.apiTrapPDU.setVarBinds(reqPdu, varBinds)
+
+    varBinds = v2c.apiTrapPDU.getVarBinds(reqPdu)
 
-    pMod.apiTrapPDU.setDefaults(reqPdu)
-    pMod.apiTrapPDU.setVarBinds(reqPdu, varBinds)
+    v2c.apiTrapPDU.setVarBinds(reqPdu, _ensureVarBinds(varBinds))
 
     if authData.mpModel == 0:
         reqPdu = rfc2576.v2ToV1(reqPdu)
diff --git a/pysnmp/hlapi/v1arch/asyncore/sync/ntforg.py b/pysnmp/hlapi/v1arch/asyncore/sync/ntforg.py
index 81de276c..64465809 100644
--- a/pysnmp/hlapi/v1arch/asyncore/sync/ntforg.py
+++ b/pysnmp/hlapi/v1arch/asyncore/sync/ntforg.py
@@ -41,24 +41,27 @@ def sendNotification(snmpDispatcher, authData, transportTarget,
         or :py:class:`~pysnmp.smi.rfc1902.ObjectType` class instances
         of :py:class:`~pysnmp.smi.rfc1902.NotificationType` objects.
 
-        SNMP Notification PDU places rigid requirement on the ordering of
-        the variable-bindings.
-
-        Mandatory variable-bindings:
+        Besides user variable-bindings, SNMP Notification PDU requires at
+        least two variable-bindings to be present:
 
         0. SNMPv2-MIB::sysUpTime.0 = <agent uptime>
-        1. SNMPv2-SMI::snmpTrapOID.0 = {SNMPv2-MIB::coldStart, ...}
+        1. SNMPv2-SMI::snmpTrapOID.0 = <notification ID>
 
-        Optional variable-bindings (applicable to SNMP v1 TRAP):
+        When sending SNMPv1 TRAP, more variable-bindings could be present:
 
         2. SNMP-COMMUNITY-MIB::snmpTrapAddress.0 = <agent-IP>
         3. SNMP-COMMUNITY-MIB::snmpTrapCommunity.0 = <snmp-community-name>
         4. SNMP-COMMUNITY-MIB::snmpTrapEnterprise.0 = <enterprise-OID>
 
-        Informational variable-bindings:
+        If user does not supply some or any of the above variable-bindings or
+        if they are at the wrong positions, the system will add/reorder the
+        missing ones automatically.
 
-        * SNMPv2-SMI::NOTIFICATION-TYPE
-        * SNMPv2-SMI::OBJECT-TYPE
+        On top of that, some notification types imply including some additional
+        variable-bindings providing additional details on the event being
+        reported. Therefore it is generally easier to use
+        :py:class:`~pysnmp.smi.rfc1902.NotificationType` object which will
+        help adding relevant variable-bindings.
 
     Other Parameters
     ----------------
diff --git a/pysnmp/hlapi/v3arch/asyncio/ntforg.py b/pysnmp/hlapi/v3arch/asyncio/ntforg.py
index 72efbfec..66d9fb29 100644
--- a/pysnmp/hlapi/v3arch/asyncio/ntforg.py
+++ b/pysnmp/hlapi/v3arch/asyncio/ntforg.py
@@ -8,6 +8,12 @@
 # Authors: Matt Hooks <me@matthooks.com>
 #          Zachary Lorusso <zlorusso@gmail.com>
 #
+try:
+    import asyncio
+
+except ImportError:
+    import trollius as asyncio
+
 from pysnmp.hlapi.v3arch.auth import *
 from pysnmp.hlapi.v3arch.context import *
 from pysnmp.hlapi.v3arch.lcd import *
@@ -16,12 +22,6 @@ from pysnmp.hlapi.v3arch.asyncio.transport import *
 from pysnmp.entity.rfc3413 import ntforg
 from pysnmp.smi.rfc1902 import *
 
-try:
-    import asyncio
-
-except ImportError:
-    import trollius as asyncio
-
 __all__ = ['sendNotification']
 
 VB_PROCESSOR = NotificationOriginatorVarBinds()