Skip to content

Commit

Permalink
Add support for endOfMibView and noSuchInstance
Browse files Browse the repository at this point in the history
References #66 and #54

The values are now supported but this brings microscopic changes to the
API. I will keep this a *minor* change because the previous behaviour of
the API was incorrect, and code relying on that behaviour is hence
incorrect as well. The biggest impact this could have stems from the
fact that code which previously incorrectly raised a "NoSuchOID"
exception, now returns ``None`` (in the pythonic API) and either
``NoSuchInstance()`` or ``NoSuchObject()`` in the "raw" API.

So any code catching ``NoSuchOID`` might require changes.
  • Loading branch information
exhuma committed Nov 26, 2019
1 parent 1548101 commit c70bf83
Show file tree
Hide file tree
Showing 17 changed files with 300 additions and 112 deletions.
54 changes: 30 additions & 24 deletions puresnmp/aio/api/raw.py
Original file line number Diff line number Diff line change
@@ -1,33 +1,31 @@
from __future__ import unicode_literals
from collections import OrderedDict
from typing import TYPE_CHECKING

import logging
import sys
from collections import OrderedDict
from typing import TYPE_CHECKING

from ...x690.types import (
Integer,
ObjectIdentifier,
OctetString,
Sequence,
Type,
)
from ...x690.util import to_bytes, tablify
from ...exc import SnmpError, NoSuchOID, FaultySNMPImplementation
from ...const import ERRORS_STRICT, ERRORS_WARN, Version
from ...exc import FaultySNMPImplementation, NoSuchOID, SnmpError
from ...pdu import (
BulkGetRequest,
GetNextRequest,
GetRequest,
SetRequest,
VarBind,
END_OF_MIB_VIEW,
VarBind
)
from ...const import Version, ERRORS_WARN, ERRORS_STRICT
from ..transport import Transport
from ...util import (
BulkResult, # NOQA (must be here for type detection)
get_unfinished_walk_oids,
group_varbinds,
from ...types import EndOfMibView
from ...util import BulkResult # NOQA (must be here for type detection)
from ...util import get_unfinished_walk_oids, group_varbinds
from ...x690.types import (
Integer,
ObjectIdentifier,
OctetString,
Sequence,
Type
)
from ...x690.util import tablify, to_bytes
from ..transport import Transport

if TYPE_CHECKING: # pragma: no cover
# pylint: disable=unused-import, invalid-name, ungrouped-imports
Expand Down Expand Up @@ -141,11 +139,12 @@ async def multigetnext(ip, community, oids, port=161, timeout=6):
'but got %d' % (len(oids), len(response_object.varbinds)))

output = []
for oid, value in response_object.varbinds:
if value is END_OF_MIB_VIEW:
for varbind in response_object.varbinds:
if varbind.value == EndOfMibView():
# TODO: Should this not be "continue" in case that other listings
# follow?
break
output.append(VarBind(oid, value))

output.append(varbind)

# Verify that the OIDs we retrieved are successors of the requested OIDs.
for requested, retrieved in zip(oids, output):
Expand Down Expand Up @@ -439,7 +438,14 @@ async def bulkget(
# prepare output for listing
repeating_out = OrderedDict() # type: Dict[str, Type]
for oid, value in repeating_tmp:
if value is END_OF_MIB_VIEW:
if isinstance(value, EndOfMibView):
# An SNMP agent behaving correctly, will return the OIDs ordered,
# and running into an EndOfMibView marker means we have reached the
# end of an accessible tree.
# TODO: What if one of the trees walking over is a restricted
# MIB-View, but there are valid values *after* that view? Should we
# not use "continue" here for that? But what about non-compliant
# SNMP servers which return values unordered?
break
repeating_out[unicode(oid)] = value

Expand Down
41 changes: 20 additions & 21 deletions puresnmp/api/raw.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,28 @@
case it's recommended to use :py:mod:`puresnmp.api.raw`.
'''
from __future__ import unicode_literals
from collections import OrderedDict
from typing import TYPE_CHECKING

import logging
import sys
from collections import OrderedDict
from typing import TYPE_CHECKING

from ..x690.types import (
Integer,
ObjectIdentifier,
OctetString,
Sequence,
Type,
)
from ..x690.util import to_bytes, tablify
from ..exc import SnmpError, NoSuchOID, FaultySNMPImplementation
from ..types import EndOfMibView
from ..const import ERRORS_STRICT, ERRORS_WARN, Version
from ..exc import FaultySNMPImplementation, NoSuchOID, SnmpError
from ..pdu import (
END_OF_MIB_VIEW,
BulkGetRequest,
GetNextRequest,
GetRequest,
SetRequest,
VarBind,
END_OF_MIB_VIEW,
VarBind
)
from ..const import Version, ERRORS_WARN, ERRORS_STRICT
from ..transport import Transport
from ..util import (
BulkResult, # NOQA (must be here for type detection)
get_unfinished_walk_oids,
group_varbinds,
)
from ..types import EndOfMibView, NoSuchInstance, NoSuchObject
from ..util import BulkResult # NOQA (must be here for type detection)
from ..util import get_unfinished_walk_oids, group_varbinds
from ..x690.types import Integer, ObjectIdentifier, OctetString, Sequence, Type
from ..x690.util import tablify, to_bytes

if TYPE_CHECKING: # pragma: no cover
# pylint: disable=unused-import, invalid-name, ungrouped-imports
Expand Down Expand Up @@ -448,7 +440,14 @@ def bulkget(
# prepare output for listing
repeating_out = OrderedDict() # type: Dict[str, Type]
for oid, value in repeating_tmp:
if value is END_OF_MIB_VIEW:
if isinstance(value, EndOfMibView):
# An SNMP agent behaving correctly, will return the OIDs ordered,
# and running into an EndOfMibView marker means we have reached the
# end of an accessible tree.
# TODO: What if one of the trees walking over is a restricted
# MIB-View, but there are valid values *after* that view? Should we
# not use "continue" here for that? But what about non-compliant
# SNMP servers which return values unordered?
break
repeating_out[unicode(oid)] = value

Expand Down
2 changes: 2 additions & 0 deletions puresnmp/pdu.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ class PDU(Type):
The superclass for SNMP Messages (GET, SET, GETNEXT, ...)
"""
TYPECLASS = TypeInfo.CONTEXT
ENCODINGS = {TypeInfo.CONSTRUCTED}
TAG = 0x10

@classmethod
def decode(cls, data):
Expand Down
3 changes: 3 additions & 0 deletions puresnmp/test/data/get_non_existing_80.hex
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
0000: 30 2A 02 01 01 04 0A 72 65 73 74 72 69 63 74 65 0*.....restricte
0016: 64 A2 19 02 04 74 E6 91 AD 02 01 00 02 01 00 30 d....t.........0
0032: 0B 30 09 06 05 2A 03 04 05 06 80 00 .0...*......
3 changes: 3 additions & 0 deletions puresnmp/test/data/get_non_existing_81.hex
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
0000: 30 28 02 01 01 04 06 70 75 62 6C 69 63 A2 1B 02 0(.....public...
0016: 04 6B B5 C4 12 02 01 00 02 01 00 30 0D 30 0B 06 .k.........0.0..
0032: 07 2B 06 01 02 01 01 02 81 00 .+........
5 changes: 5 additions & 0 deletions puresnmp/test/data/gh-issues/66-nosuchinstance.hex
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
30 42 02 01 01 04 06 30 30 30 30 30 30 a2 35 02 0B.....000000.5.
04 5d 9d 59 c9 02 01 00 02 01 00 30 27 30 12 06 .].Y.......0'0..
0d 2b 06 01 04 01 81 8a 31 15 01 02 0a 00 02 01 .+......1.......
01 30 11 06 0d 2b 06 01 04 01 81 8a 31 15 01 03 .0...+......1...
01 00 81 00 ....
18 changes: 18 additions & 0 deletions puresnmp/test/data/multiget_nosuchinstance.hex
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# -*- ascii-cols: 6-56 -*-
#
# The following dump relates to GitHub issue #66 and contains a "multiget"
# response with the 4 results:
#
#
# .1.2.3.4.5.6.7.8.9 = No Such Object available on this agent at this OID
# .1.3.6.1.2.1.1.1.2.0 = No Such Instance currently exists at this OID
# .1.3.6.1.2.1.1.2.0 = OID: .1.3.6.1.4.1.8072.3.2.10
# .1.3.6.1.2.1.2.1.0 = INTEGER: 2

0000: 30 60 02 01 01 04 07 70 72 69 76 61 74 65 A2 52 0`.....private.R
0016: 02 04 1F A9 F4 E3 02 01 00 02 01 00 30 44 30 0C ............0D0.
0032: 06 08 2A 03 04 05 06 07 08 09 80 00 30 0D 06 09 ..*.........0...
0048: 2B 06 01 02 01 01 01 02 00 81 00 30 16 06 08 2B +..........0...+
0064: 06 01 02 01 01 02 00 06 0A 2B 06 01 04 01 BF 08 .........+......
0080: 03 02 0A 30 0D 06 08 2B 06 01 02 01 02 01 00 02 ...0...+........
0096: 01 02 ..
18 changes: 18 additions & 0 deletions puresnmp/test/data/multiget_nosuchobject.hex
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# -*- ascii-cols: 6-56 -*-
#
# The following dump relates to GitHub issue #66 and contains a "multiget"
# response with the 4 results:
#
#
# .1.2.3.4.5.6.7.8.9 = No Such Object available on this agent at this OID
# .1.3.6.1.2.1.1.1.2.0 = No Such Object available on this agent at this OID
# .1.3.6.1.2.1.1.2.0 = No Such Object available on this agent at this OID
# .1.3.6.1.2.1.2.1.0 = INTEGER: 2

0000: 30 59 02 01 01 04 0A 72 65 73 74 72 69 63 74 65 0Y.....restricte
0016: 64 A2 48 02 04 1C AD 79 DC 02 01 00 02 01 00 30 d.H....y.......0
0032: 3A 30 0C 06 08 2A 03 04 05 06 07 08 09 80 00 30 :0...*.........0
0048: 0D 06 09 2B 06 01 02 01 01 01 02 00 80 00 30 0C ...+..........0.
0064: 06 08 2B 06 01 02 01 01 02 00 80 00 30 0D 06 08 ..+.........0...
0080: 2B 06 01 02 01 02 01 00 02 01 02 +..........

18 changes: 17 additions & 1 deletion puresnmp/test/gh-issues/test_54_endofmibview.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

from puresnmp.api.raw import bulkget
from puresnmp.test import readbytes
from puresnmp.types import EndOfMibView
from puresnmp.x690.types import Integer, OctetString

try:
Expand All @@ -12,6 +11,23 @@


def test_54_endofmibview():
"""
The dump in ``gh-issues/54-endofmibview.hex`` contains an "endOfMibView"
marker on the OID '1.3.6.1.6.3.10.2.1.4.0'. So that should be the last OID
we see.
NOTE: The return-value of ``bulkget`` for listings is a dictionary which
cannot represent the ``endOfMibView`` marker as-is as it would require to
add the OID twice. For the "pythonic" API this is always correct as it is
supposed to hide the SNMP internals. If is debatable whether this behaviour
is wanted in the "raw" API though. For now this is not supported as it
would require a non backwards-compatible change in the API (the data-type
would change). Using something that "behaves like" a dict is also
questionable acceptable as it would be non-deterministic what happens when
accessing the key which caused the ``endOfMibView`` as this is the one that
is duplicated.
"""
data = readbytes('gh-issues/54-endofmibview.hex')
with patch('puresnmp.api.raw.Transport') as ptch:
ptch().send.return_value = data
Expand Down
63 changes: 63 additions & 0 deletions puresnmp/test/gh-issues/test_66_nosuchinstance.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# type: ignore
"""
When an SNMP agent returns the type-value 0x81 a NULL value should be reported.
By definition in RFC-1905 the "value" part in a VarBind can either be a value,
or one of:
"noSuchObject": the requested OID does not exist
"noSuchInstance": the OID exists but has no value
"endOfMibView": A marker to signify we have seen all that we are allowed to
see in an operation based on "get-next".
This is (at the time of this writing, 2019-10-10) not handled properly. See
also gihub issue #54. This module defines a test verifyng that we get a correct
value from a noSuchInstance marker.
"""
from puresnmp.api.pythonic import multiget as pyget
from puresnmp.api.raw import multiget
from puresnmp.test import readbytes
from puresnmp.types import NoSuchInstance
from puresnmp.x690.types import Integer

try:
from unittest.mock import patch
except ImportError:
from mock import patch


def test_data_type():
"""
We want to have a special data-type for noSuchInstance which behaves
properly (like a varbind & implicit null).
"""
value = NoSuchInstance('1.2.3.4.5')
assert bool(value) is False
assert value.oid == '1.2.3.4.5'
assert value.value is None


def test_66_nosuchinstance_raw():
"""
If we get a noSuchInstance VarBind, we want to report this as an
appropriate value.
"""
data = readbytes('gh-issues/66-nosuchinstance.hex')
with patch('puresnmp.api.raw.Transport') as ptch:
ptch().send.return_value = data
ptch().get_request_id.return_value = 123
result = multiget('192.0.2.1', 'private', ['1.2.3', '2.3.4'])
assert result == [Integer(1), NoSuchInstance('1.2.3')]


def test_66_nosuchinstance_pythonic():
"""
Pure Python has no data-type to represent the same concept as
"noSuchInstance" so we generalise it as ``None``
"""
data = readbytes('gh-issues/66-nosuchinstance.hex')
with patch('puresnmp.api.raw.Transport') as ptch:
ptch().send.return_value = data
ptch().get_request_id.return_value = 123
result = pyget('192.0.2.1', 'private', ['1.2.3', '2.3.4'])
assert result == [1, None]
21 changes: 11 additions & 10 deletions puresnmp/test/test_aio_raw.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from puresnmp.const import Version
from puresnmp.exc import NoSuchOID, SnmpError
from puresnmp.pdu import BulkGetRequest, GetNextRequest, GetRequest, VarBind
from puresnmp.types import Counter, Gauge, IpAddress, TimeTicks
from puresnmp.types import NoSuchInstance, Counter, Gauge, IpAddress, TimeTicks, NoSuchObject
from puresnmp.util import BulkResult
from puresnmp.x690.types import (Integer, ObjectIdentifier, OctetString,
Sequence, to_bytes)
Expand Down Expand Up @@ -86,24 +86,24 @@ async def test_get_non_existing_oid_80(self):
mck().send = AsyncMock()
mck().send.return_value = data
mck().get_request_id.return_value = 0
with pytest.raises(NoSuchOID):
await get('::1', 'private', '1.2.3')
result = await get('::1', 'private', '1.2.3')
assert not bool(result)
assert isinstance(result, NoSuchObject)

@pytest.mark.asyncio
async def test_get_non_existing_oid_81(self):
async def test_get_nosuchinstance(self):
"""
A "GET" response on a non-existing OID should raise an appropriate
exception.
This tests the byte-marker 0x81 (TODO explain 0x81)
If we get a "noSuchInstance" (0x81) on an OID, we should get the
appropriate result.
"""
data = readbytes('get_non_existing_81.hex')
with patch('puresnmp.aio.api.raw.Transport') as mck:
mck().send = AsyncMock()
mck().send.return_value = data
mck().get_request_id.return_value = 0
with pytest.raises(NoSuchOID):
await get('::1', 'private', '1.2.3')
result = await get('::1', 'private', '1.2.3')
assert not bool(result)
assert isinstance(result, NoSuchInstance)


class TestWalk(object):
Expand Down Expand Up @@ -228,6 +228,7 @@ async def test_eom(self):
expected = [
(root+'0', 1),
(root+'1', 1),
(root+'2', 1),
]

simplified_result = [
Expand Down
Loading

0 comments on commit c70bf83

Please sign in to comment.