* SNMP table row consistency check added. This change may break

  valid SNMP SET operations on tables if RowStatus column is not
  passed at the very end of var-binds.

5 files changed, 66 insertions, 39 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 0add53f9..111e6df5 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -5,6 +5,9 @@ Revision 4.3.2, work in progress
 Repository `tarball download <http://pysnmp.cvs.sourceforge.net/viewvc/pysnmp/pysnmp/?view=tar>`_
 
 - Copyright notice added to non-trivial source code files.
+- SNMP table row consistency check added. This change may break
+  valid SNMP SET operations on tables if RowStatus column is not
+  passed at the very end of var-binds.
 
 Revision 4.3.1, released 12-11-2015
 -----------------------------------
diff --git a/TODO.txt b/TODO.txt
index 455412c9..c7a4fae3 100644
--- a/TODO.txt
+++ b/TODO.txt
@@ -52,3 +52,9 @@ Sparse notes on major existing problems/plans
   may need a means to deal only with specific node types.
 
 * redesign proto.errind.ErrorIndication
+
+* redesign PYSNMP MIB tables to better handle optional values (keys)
+
+* add RowStatus checks when reading MIB tables (LCD)
+
+* Disallow empty SET value reaching scalar MIB object
diff --git a/pysnmp/entity/config.py b/pysnmp/entity/config.py
index d1de809a..deecae12 100644
--- a/pysnmp/entity/config.py
+++ b/pysnmp/entity/config.py
@@ -71,14 +71,14 @@ def addV1System(snmpEngine, communityIndex, communityName,
         ((snmpCommunityEntry.name + (8,) + tblIdx, 'destroy'),)
     )
     snmpEngine.msgAndPduDsp.mibInstrumController.writeVars(
-        ((snmpCommunityEntry.name + (8,) + tblIdx, 'createAndGo'),
-         (snmpCommunityEntry.name + (1,) + tblIdx, communityIndex),
+        ((snmpCommunityEntry.name + (1,) + tblIdx, communityIndex),
          (snmpCommunityEntry.name + (2,) + tblIdx, communityName),
          (snmpCommunityEntry.name + (3,) + tblIdx, securityName is not None and securityName or communityIndex),
          (snmpCommunityEntry.name + (4,) + tblIdx, contextEngineId),
          (snmpCommunityEntry.name + (5,) + tblIdx, contextName),
          (snmpCommunityEntry.name + (6,) + tblIdx, transportTag),
-         (snmpCommunityEntry.name + (7,) + tblIdx, 'nonVolatile'))
+         (snmpCommunityEntry.name + (7,) + tblIdx, 'nonVolatile'),
+         (snmpCommunityEntry.name + (8,) + tblIdx, 'createAndGo'))
     )
 
 def delV1System(snmpEngine, communityIndex):
@@ -127,12 +127,12 @@ def addV3User(snmpEngine, userName,
         ((usmUserEntry.name + (13,) + tblIdx1, 'destroy'),)
     )
     snmpEngine.msgAndPduDsp.mibInstrumController.writeVars(
-        ((usmUserEntry.name + (13,) + tblIdx1, 'createAndGo'),
-         (usmUserEntry.name + (2,) + tblIdx1, userName),
+        ((usmUserEntry.name + (2,) + tblIdx1, userName),
          (usmUserEntry.name + (3,) + tblIdx1, securityName),
          (usmUserEntry.name + (4,) + tblIdx1, zeroDotZero.name),
          (usmUserEntry.name + (5,) + tblIdx1, authProtocol),
-         (usmUserEntry.name + (8,) + tblIdx1, privProtocol))
+         (usmUserEntry.name + (8,) + tblIdx1, privProtocol),
+         (usmUserEntry.name + (13,) + tblIdx1, 'createAndGo'))
     )
 
     # Localize keys
@@ -170,9 +170,10 @@ def addV3User(snmpEngine, userName,
         ((pysnmpUsmSecretEntry.name + (4,) + tblIdx2, 'destroy'),)
     )
     snmpEngine.msgAndPduDsp.mibInstrumController.writeVars(
-        ((pysnmpUsmSecretEntry.name + (4,) + tblIdx2, 'createAndGo'),
+        ((pysnmpUsmSecretEntry.name + (1,) + tblIdx2, userName),
          (pysnmpUsmSecretEntry.name + (2,) + tblIdx2, authKey),
-         (pysnmpUsmSecretEntry.name + (3,) + tblIdx2, privKey),)
+         (pysnmpUsmSecretEntry.name + (3,) + tblIdx2, privKey),
+         (pysnmpUsmSecretEntry.name + (4,) + tblIdx2, 'createAndGo'))
     )
 
 def delV3User(snmpEngine,
@@ -231,12 +232,12 @@ def addTargetParams(snmpEngine, name, securityName, securityLevel, mpModel=3):
         ((snmpTargetParamsEntry.name + (7,) + tblIdx, 'destroy'),)
     )
     snmpEngine.msgAndPduDsp.mibInstrumController.writeVars(
-        ((snmpTargetParamsEntry.name + (7,) + tblIdx, 'createAndGo'),
-         (snmpTargetParamsEntry.name + (1,) + tblIdx, name),
+        ((snmpTargetParamsEntry.name + (1,) + tblIdx, name),
          (snmpTargetParamsEntry.name + (2,) + tblIdx, mpModel),
          (snmpTargetParamsEntry.name + (3,) + tblIdx, securityModel),
          (snmpTargetParamsEntry.name + (4,) + tblIdx, securityName),
-         (snmpTargetParamsEntry.name + (5,) + tblIdx, securityLevel))
+         (snmpTargetParamsEntry.name + (5,) + tblIdx, securityLevel),
+         (snmpTargetParamsEntry.name + (7,) + tblIdx, 'createAndGo'))
     )
 
 def delTargetParams(snmpEngine, name):
@@ -274,15 +275,15 @@ def addTargetAddr(snmpEngine, addrName, transportDomain, transportAddress,
         ((snmpTargetAddrEntry.name + (9,) + tblIdx, 'destroy'),)
     )
     snmpEngine.msgAndPduDsp.mibInstrumController.writeVars(
-        ((snmpTargetAddrEntry.name + (9,) + tblIdx, 'createAndGo'),
-         (snmpTargetAddrEntry.name + (1,) + tblIdx, addrName),
+        ((snmpTargetAddrEntry.name + (1,) + tblIdx, addrName),
          (snmpTargetAddrEntry.name + (2,) + tblIdx, transportDomain),
          (snmpTargetAddrEntry.name + (3,) + tblIdx, transportAddress),
          (snmpTargetAddrEntry.name + (4,) + tblIdx, timeout),
          (snmpTargetAddrEntry.name + (5,) + tblIdx, retryCount),
          (snmpTargetAddrEntry.name + (6,) + tblIdx, tagList),
          (snmpTargetAddrEntry.name + (7,) + tblIdx, params),
-         (snmpSourceAddrEntry.name + (1,) + tblIdx, sourceAddress))
+         (snmpSourceAddrEntry.name + (1,) + tblIdx, sourceAddress),
+         (snmpTargetAddrEntry.name + (9,) + tblIdx, 'createAndGo'))
     )
 
 def delTargetAddr(snmpEngine, addrName):
@@ -370,10 +371,10 @@ def addVacmGroup(snmpEngine, groupName, securityModel, securityName):
         ((vacmSecurityToGroupEntry.name + (5,) + tblIdx, 'destroy'),)
     )
     snmpEngine.msgAndPduDsp.mibInstrumController.writeVars(
-        ((vacmSecurityToGroupEntry.name + (5,) + tblIdx, 'createAndGo'),
-         (vacmSecurityToGroupEntry.name + (1,) + tblIdx, securityModel),
+        ((vacmSecurityToGroupEntry.name + (1,) + tblIdx, securityModel),
          (vacmSecurityToGroupEntry.name + (2,) + tblIdx, securityName),
-         (vacmSecurityToGroupEntry.name + (3,) + tblIdx, groupName),)
+         (vacmSecurityToGroupEntry.name + (3,) + tblIdx, groupName),
+         (vacmSecurityToGroupEntry.name + (5,) + tblIdx, 'createAndGo'))
     )
 
 def delVacmGroup(snmpEngine, securityModel, securityName):
@@ -403,14 +404,14 @@ def addVacmAccess(snmpEngine, groupName, contextName, securityModel,
         ((vacmAccessEntry.name + (9,) + tblIdx, 'destroy'),)
     )
     snmpEngine.msgAndPduDsp.mibInstrumController.writeVars(
-        ((vacmAccessEntry.name + (9,) + tblIdx, 'createAndGo'),
-         (vacmAccessEntry.name + (1,) + tblIdx, contextName),
+        ((vacmAccessEntry.name + (1,) + tblIdx, contextName),
          (vacmAccessEntry.name + (2,) + tblIdx, securityModel),
          (vacmAccessEntry.name + (3,) + tblIdx, securityLevel),
          (vacmAccessEntry.name + (4,) + tblIdx, prefix),
          (vacmAccessEntry.name + (5,) + tblIdx, readView),
          (vacmAccessEntry.name + (6,) + tblIdx, writeView),
-         (vacmAccessEntry.name + (7,) + tblIdx, notifyView),)
+         (vacmAccessEntry.name + (7,) + tblIdx, notifyView),
+         (vacmAccessEntry.name + (9,) + tblIdx, 'createAndGo'))
     )
 
 def delVacmAccess(snmpEngine, groupName, contextName, securityModel,
@@ -436,11 +437,11 @@ def addVacmView(snmpEngine, viewName, viewType, subTree, mask):
         ((vacmViewTreeFamilyEntry.name + (6,) + tblIdx, 'destroy'),)
     )
     snmpEngine.msgAndPduDsp.mibInstrumController.writeVars(
-        ((vacmViewTreeFamilyEntry.name + (6,) + tblIdx, 'createAndGo'),
-         (vacmViewTreeFamilyEntry.name + (1,) + tblIdx, viewName),
+        ((vacmViewTreeFamilyEntry.name + (1,) + tblIdx, viewName),
          (vacmViewTreeFamilyEntry.name + (2,) + tblIdx, subTree),
          (vacmViewTreeFamilyEntry.name + (3,) + tblIdx, mask),
-         (vacmViewTreeFamilyEntry.name + (4,) + tblIdx, viewType),)
+         (vacmViewTreeFamilyEntry.name + (4,) + tblIdx, viewType),
+         (vacmViewTreeFamilyEntry.name + (6,) + tblIdx, 'createAndGo'))
     )
 
 def delVacmView(snmpEngine, viewName, subTree):
@@ -558,17 +559,17 @@ def addNotificationTarget(snmpEngine, notificationName, paramsName,
         ((snmpNotifyEntry.name + (5,) + tblIdx1, 'destroy'),)
     )
     snmpEngine.msgAndPduDsp.mibInstrumController.writeVars(
-        ((snmpNotifyEntry.name + (5,) + tblIdx1, 'createAndGo'),
-         (snmpNotifyEntry.name + (2,) + tblIdx1, transportTag),
-         (snmpNotifyEntry.name + (3,) + tblIdx1, notifyType),)
+        ((snmpNotifyEntry.name + (2,) + tblIdx1, transportTag),
+         (snmpNotifyEntry.name + (3,) + tblIdx1, notifyType),
+         (snmpNotifyEntry.name + (5,) + tblIdx1, 'createAndGo'))
     )
 
     snmpEngine.msgAndPduDsp.mibInstrumController.writeVars(
         ((snmpNotifyFilterProfileEntry.name + (3,) + tblIdx2, 'destroy'),)
     )
     snmpEngine.msgAndPduDsp.mibInstrumController.writeVars(
-        ((snmpNotifyFilterProfileEntry.name + (3,) + tblIdx2, 'createAndGo'),
-         (snmpNotifyFilterProfileEntry.name + (1,) + tblIdx2, profileName),)
+        ((snmpNotifyFilterProfileEntry.name + (1,) + tblIdx2, profileName),
+         (snmpNotifyFilterProfileEntry.name + (3,) + tblIdx2, 'createAndGo'))
     )
 
     if not snmpNotifyFilterEntry:
@@ -578,10 +579,10 @@ def addNotificationTarget(snmpEngine, notificationName, paramsName,
         ((snmpNotifyFilterEntry.name + (5,) + tblIdx3, 'destroy'),)
     )
     snmpEngine.msgAndPduDsp.mibInstrumController.writeVars(
-        ((snmpNotifyFilterEntry.name + (5,) + tblIdx3, 'createAndGo'),
-         (snmpNotifyFilterEntry.name + (1,) + tblIdx3, filterSubtree),
+        ((snmpNotifyFilterEntry.name + (1,) + tblIdx3, filterSubtree),
          (snmpNotifyFilterEntry.name + (2,) + tblIdx3, filterMask),
-         (snmpNotifyFilterEntry.name + (3,) + tblIdx3, filterType),)
+         (snmpNotifyFilterEntry.name + (3,) + tblIdx3, filterType),
+         (snmpNotifyFilterEntry.name + (5,) + tblIdx3, 'createAndGo'))
     )
 
 def delNotificationTarget(snmpEngine, notificationName, paramsName,
diff --git a/pysnmp/smi/mibs/PYSNMP-USM-MIB.py b/pysnmp/smi/mibs/PYSNMP-USM-MIB.py
index 44827fe3..29337596 100644
--- a/pysnmp/smi/mibs/PYSNMP-USM-MIB.py
+++ b/pysnmp/smi/mibs/PYSNMP-USM-MIB.py
@@ -38,9 +38,9 @@ pysnmpUsmSecretEntry = MibTableRow((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 2, 1),
 if mibBuilder.loadTexts: pysnmpUsmSecretEntry.setDescription('Information about a particular USM user credentials.')
 pysnmpUsmSecretUserName = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 2, 1, 1), SnmpAdminString().subtype(subtypeSpec=ValueSizeConstraint(1,32)))
 if mibBuilder.loadTexts: pysnmpUsmSecretUserName.setDescription('The username string for which a row in this table\n         represents a configuration.')
-pysnmpUsmSecretAuthKey = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 2, 1, 2), OctetString().subtype(subtypeSpec=ValueSizeConstraint(8,65535)))
+pysnmpUsmSecretAuthKey = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 2, 1, 2), OctetString('\x00\x00\x00\x00\x00\x00\x00\x00').subtype(subtypeSpec=ValueSizeConstraint(8,65535)))
 if mibBuilder.loadTexts: pysnmpUsmSecretAuthKey.setDescription("User's authentication passphrase used for localized key generation.")
-pysnmpUsmSecretPrivKey = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 2, 1, 3), OctetString().subtype(subtypeSpec=ValueSizeConstraint(8,65535)))
+pysnmpUsmSecretPrivKey = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 2, 1, 3), OctetString('\x00\x00\x00\x00\x00\x00\x00\x00').subtype(subtypeSpec=ValueSizeConstraint(8,65535)))
 if mibBuilder.loadTexts: pysnmpUsmSecretPrivKey.setDescription("User's encryption passphrase used for localized key generation.")
 pysnmpUsmSecretStatus = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 2, 1, 4), RowStatus()).setMaxAccess("readcreate")
 if mibBuilder.loadTexts: pysnmpUsmSecretStatus.setDescription('Table status')
@@ -50,13 +50,13 @@ pysnmpUsmKeyEntry = MibTableRow((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 3, 1), )
 usmUserEntry.registerAugmentions(("PYSNMP-USM-MIB", "pysnmpUsmKeyEntry"))
 pysnmpUsmKeyEntry.setIndexNames(*usmUserEntry.getIndexNames())
 if mibBuilder.loadTexts: pysnmpUsmKeyEntry.setDescription('Information about a particular USM user credentials.')
-pysnmpUsmKeyAuthLocalized = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 3, 1, 1), OctetString().subtype(subtypeSpec=ValueSizeConstraint(8,32)))
+pysnmpUsmKeyAuthLocalized = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 3, 1, 1), OctetString('\x00\x00\x00\x00\x00\x00\x00\x00').subtype(subtypeSpec=ValueSizeConstraint(8,32)))
 if mibBuilder.loadTexts: pysnmpUsmKeyAuthLocalized.setDescription("User's localized key used for authentication.")
-pysnmpUsmKeyPrivLocalized = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 3, 1, 2), OctetString().subtype(subtypeSpec=ValueSizeConstraint(8,32)))
+pysnmpUsmKeyPrivLocalized = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 3, 1, 2), OctetString('\x00\x00\x00\x00\x00\x00\x00\x00').subtype(subtypeSpec=ValueSizeConstraint(8,32)))
 if mibBuilder.loadTexts: pysnmpUsmKeyPrivLocalized.setDescription("User's localized key used for encryption.")
-pysnmpUsmKeyAuth = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 3, 1, 3), OctetString().subtype(subtypeSpec=ValueSizeConstraint(8,32)))
+pysnmpUsmKeyAuth = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 3, 1, 3), OctetString('\x00\x00\x00\x00\x00\x00\x00\x00').subtype(subtypeSpec=ValueSizeConstraint(8,32)))
 if mibBuilder.loadTexts: pysnmpUsmKeyAuth.setDescription("User's non-localized key used for authentication.")
-pysnmpUsmKeyPriv = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 3, 1, 4), OctetString().subtype(subtypeSpec=ValueSizeConstraint(8,32)))
+pysnmpUsmKeyPriv = MibTableColumn((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 1, 3, 1, 4), OctetString('\x00\x00\x00\x00\x00\x00\x00\x00').subtype(subtypeSpec=ValueSizeConstraint(8,32)))
 if mibBuilder.loadTexts: pysnmpUsmKeyPriv.setDescription("User's non-localized key used for encryption.")
 pysnmpUsmMIBCompliances = MibIdentifier((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 2, 1))
 pysnmpUsmMIBGroups = MibIdentifier((1, 3, 6, 1, 4, 1, 20408, 3, 1, 1, 2, 2))
diff --git a/pysnmp/smi/mibs/SNMPv2-SMI.py b/pysnmp/smi/mibs/SNMPv2-SMI.py
index bdc26997..ce7da0ec 100644
--- a/pysnmp/smi/mibs/SNMPv2-SMI.py
+++ b/pysnmp/smi/mibs/SNMPv2-SMI.py
@@ -878,7 +878,11 @@ class MibTableColumn(MibScalar):
             MibScalar.writeTest(self, name, val, idx, acInfo)
         # ...otherwise proceed with creating new column
         except (error.NoSuchInstanceError, error.RowCreationWanted):
-            self.__rowOpWanted[name] = error.RowCreationWanted()
+            excValue = sys.exc_info()[1]
+            if isinstance(excValue, error.RowCreationWanted):
+                self.__rowOpWanted[name] = excValue
+            else:
+                self.__rowOpWanted[name] = error.RowCreationWanted()
             self.createTest(name, val, idx, acInfo)
         except error.RowDestructionWanted:
             self.__rowOpWanted[name] = error.RowDestructionWanted()
@@ -911,6 +915,8 @@ class MibTableColumn(MibScalar):
             raise e
 
     def writeUndo(self, name, val, idx, acInfo):
+        if name in self.__rowOpWanted:
+            self.__rowOpWanted[name] = error.RowDestructionWanted()
         self.__delegateWrite('Undo', name, val, idx, acInfo)
         if name in self.__rowOpWanted:
             e = self.__rowOpWanted[name]
@@ -1076,6 +1082,7 @@ class MibTableRow(MibTree):
 
     def __delegate(self, subAction, name, val, idx, acInfo):
         # Relay operation request to column, expect row operation request.
+        rowIsActive = False
         try:
             getattr(self.getBranch(name, idx), 'write'+subAction)(
                 name, val, idx, acInfo
@@ -1091,6 +1098,9 @@ class MibTableRow(MibTree):
                 'create'+subAction, name, None, idx, acInfo
             )
 
+            # watch for RowStatus == 'stActive'
+            rowIsActive = sys.exc_info()[1].get('syntax', 0) == 1
+
         except error.RowDestructionWanted:
             self.__manageColumns(
                 'destroy'+subAction, name[:len(self.name)+1],
@@ -1101,11 +1111,18 @@ class MibTableRow(MibTree):
                 'destroy'+subAction, name, None, idx, acInfo
             )
 
+        return rowIsActive
+
     def writeTest(self, name, val, idx, acInfo):
         self.__delegate('Test', name, val, idx, acInfo)
 
     def writeCommit(self, name, val, idx, acInfo):
-        self.__delegate('Commit', name, val, idx, acInfo)
+        rowIsActive = self.__delegate('Commit', name, val, idx, acInfo)
+        if rowIsActive:
+            for mibNode in self._vars.values():
+                colNode = mibNode.getNode(mibNode.name + name[len(self.name)+1:])
+                if not colNode.syntax.hasValue():
+                    raise error.InconsistentValueError(msg='Row consistency check failed for %r' % colNode)
 
     def writeCleanup(self, name, val, idx, acInfo):
         self.branchVersionId += 1