From 8b1ae6617dc99dbbcc79a093fc329030d89db464 Mon Sep 17 00:00:00 2001 From: Joel Bender Date: Tue, 19 Mar 2019 21:56:07 -0400 Subject: [PATCH] try putting low and high limits on Unsigned (#258) --- py25/bacpypes/basetypes.py | 3 +- py25/bacpypes/primitivedata.py | 44 ++++++++++++++-------- py27/bacpypes/basetypes.py | 3 +- py27/bacpypes/primitivedata.py | 38 +++++++++++++------ py34/bacpypes/basetypes.py | 3 +- py34/bacpypes/primitivedata.py | 28 +++++++++++--- tests/test_primitive_data/test_unsigned.py | 18 ++++++++- 7 files changed, 101 insertions(+), 36 deletions(-) diff --git a/py25/bacpypes/basetypes.py b/py25/bacpypes/basetypes.py index dcbd5ed..c5419a1 100755 --- a/py25/bacpypes/basetypes.py +++ b/py25/bacpypes/basetypes.py @@ -1730,7 +1730,8 @@ class AccessRule(Sequence): ] class AccessThreatLevel(Unsigned): - pass + _low_limit = 0 + _high_limit = 100 class AccumulatorRecord(Sequence): sequenceElements = \ diff --git a/py25/bacpypes/primitivedata.py b/py25/bacpypes/primitivedata.py index bf3bf12..6718c61 100755 --- a/py25/bacpypes/primitivedata.py +++ b/py25/bacpypes/primitivedata.py @@ -594,34 +594,34 @@ class Boolean(Atomic): class Unsigned(Atomic): _app_tag = Tag.unsignedAppTag + _low_limit = 0 + _high_limit = None - def __init__(self,arg = None): - self.value = 0L + def __init__(self, arg=None): + self.value = 0 if arg is None: pass elif isinstance(arg, Tag): self.decode(arg) elif isinstance(arg, int): - if (arg < 0): - raise ValueError("unsigned integer required") - self.value = long(arg) - elif isinstance(arg, long): - if (arg < 0): - raise ValueError("unsigned integer required") + if not self.is_valid(arg): + raise ValueError("value out of range") self.value = arg elif isinstance(arg, Unsigned): + if not self.is_valid(arg.value): + raise ValueError("value out of range") self.value = arg.value else: raise TypeError("invalid constructor datatype") def encode(self, tag): # rip apart the number - data = struct.pack('>L', self.value) + data = bytearray(struct.pack('>L', self.value)) # reduce the value to the smallest number of octets - while (len(data) > 1) and (data[0] == '\x00'): - data = data[1:] + while (len(data) > 1) and (data[0] == 0): + del data[0] # encode the tag tag.set_app_data(Tag.unsignedAppTag, data) @@ -633,9 +633,9 @@ class Unsigned(Atomic): raise InvalidTag("invalid tag length") # get the data - rslt = 0L + rslt = 0 for c in tag.tagData: - rslt = (rslt << 8) + ord(c) + rslt = (rslt << 8) + c # save the result self.value = rslt @@ -643,10 +643,24 @@ class Unsigned(Atomic): @classmethod def is_valid(cls, arg): """Return True if arg is valid value for the class.""" - return isinstance(arg, (int, long)) and (not isinstance(arg, bool)) and (arg >= 0) + if not isinstance(arg, int) or isinstance(arg, bool): + return False + if (arg < cls._low_limit): + return False + if (cls._high_limit is not None) and (arg > cls._high_limit): + return False + return True def __str__(self): - return "Unsigned(%s)" % (self.value, ) + return "%s(%s)" % (self.__class__.__name__, self.value) + +class Unsigned8(Unsigned): + _low_limit = 0 + _high_limit = 255 + +class Unsigned16(Unsigned): + _low_limit = 0 + _high_limit = 65535 # # Integer diff --git a/py27/bacpypes/basetypes.py b/py27/bacpypes/basetypes.py index dcbd5ed..c5419a1 100755 --- a/py27/bacpypes/basetypes.py +++ b/py27/bacpypes/basetypes.py @@ -1730,7 +1730,8 @@ class AccessRule(Sequence): ] class AccessThreatLevel(Unsigned): - pass + _low_limit = 0 + _high_limit = 100 class AccumulatorRecord(Sequence): sequenceElements = \ diff --git a/py27/bacpypes/primitivedata.py b/py27/bacpypes/primitivedata.py index 8d00be6..53034e0 100755 --- a/py27/bacpypes/primitivedata.py +++ b/py27/bacpypes/primitivedata.py @@ -598,23 +598,23 @@ class Boolean(Atomic): class Unsigned(Atomic): _app_tag = Tag.unsignedAppTag + _low_limit = 0 + _high_limit = None - def __init__(self,arg = None): - self.value = 0L + def __init__(self, arg=None): + self.value = 0 if arg is None: pass elif isinstance(arg, Tag): self.decode(arg) elif isinstance(arg, int): - if (arg < 0): - raise ValueError("unsigned integer required") - self.value = long(arg) - elif isinstance(arg, long): - if (arg < 0): - raise ValueError("unsigned integer required") + if not self.is_valid(arg): + raise ValueError("value out of range") self.value = arg elif isinstance(arg, Unsigned): + if not self.is_valid(arg.value): + raise ValueError("value out of range") self.value = arg.value else: raise TypeError("invalid constructor datatype") @@ -637,9 +637,9 @@ class Unsigned(Atomic): raise InvalidTag("invalid tag length") # get the data - rslt = 0L + rslt = 0 for c in tag.tagData: - rslt = (rslt << 8) + ord(c) + rslt = (rslt << 8) + c # save the result self.value = rslt @@ -647,10 +647,24 @@ class Unsigned(Atomic): @classmethod def is_valid(cls, arg): """Return True if arg is valid value for the class.""" - return isinstance(arg, (int, long)) and (not isinstance(arg, bool)) and (arg >= 0) + if not isinstance(arg, int) or isinstance(arg, bool): + return False + if (arg < cls._low_limit): + return False + if (cls._high_limit is not None) and (arg > cls._high_limit): + return False + return True def __str__(self): - return "Unsigned(%s)" % (self.value, ) + return "%s(%s)" % (self.__class__.__name__, self.value) + +class Unsigned8(Unsigned): + _low_limit = 0 + _high_limit = 255 + +class Unsigned16(Unsigned): + _low_limit = 0 + _high_limit = 65535 # # Integer diff --git a/py34/bacpypes/basetypes.py b/py34/bacpypes/basetypes.py index dcbd5ed..c5419a1 100755 --- a/py34/bacpypes/basetypes.py +++ b/py34/bacpypes/basetypes.py @@ -1730,7 +1730,8 @@ class AccessRule(Sequence): ] class AccessThreatLevel(Unsigned): - pass + _low_limit = 0 + _high_limit = 100 class AccumulatorRecord(Sequence): sequenceElements = \ diff --git a/py34/bacpypes/primitivedata.py b/py34/bacpypes/primitivedata.py index bb46f08..d3b341c 100755 --- a/py34/bacpypes/primitivedata.py +++ b/py34/bacpypes/primitivedata.py @@ -598,8 +598,10 @@ class Boolean(Atomic): class Unsigned(Atomic): _app_tag = Tag.unsignedAppTag + _low_limit = 0 + _high_limit = None - def __init__(self,arg = None): + def __init__(self, arg=None): self.value = 0 if arg is None: @@ -607,10 +609,12 @@ class Unsigned(Atomic): elif isinstance(arg, Tag): self.decode(arg) elif isinstance(arg, int): - if (arg < 0): - raise ValueError("unsigned integer required") + if not self.is_valid(arg): + raise ValueError("value out of range") self.value = arg elif isinstance(arg, Unsigned): + if not self.is_valid(arg.value): + raise ValueError("value out of range") self.value = arg.value else: raise TypeError("invalid constructor datatype") @@ -643,10 +647,24 @@ class Unsigned(Atomic): @classmethod def is_valid(cls, arg): """Return True if arg is valid value for the class.""" - return isinstance(arg, int) and (not isinstance(arg, bool)) and (arg >= 0) + if not isinstance(arg, int) or isinstance(arg, bool): + return False + if (arg < cls._low_limit): + return False + if (cls._high_limit is not None) and (arg > cls._high_limit): + return False + return True def __str__(self): - return "Unsigned(%s)" % (self.value, ) + return "%s(%s)" % (self.__class__.__name__, self.value) + +class Unsigned8(Unsigned): + _low_limit = 0 + _high_limit = 255 + +class Unsigned16(Unsigned): + _low_limit = 0 + _high_limit = 65535 # # Integer diff --git a/tests/test_primitive_data/test_unsigned.py b/tests/test_primitive_data/test_unsigned.py index 65d6935..e33fc51 100644 --- a/tests/test_primitive_data/test_unsigned.py +++ b/tests/test_primitive_data/test_unsigned.py @@ -12,7 +12,7 @@ import unittest from bacpypes.debugging import bacpypes_debugging, ModuleLogger, xtob from bacpypes.errors import InvalidTag -from bacpypes.primitivedata import Unsigned, Tag +from bacpypes.primitivedata import Unsigned, Unsigned8, Unsigned16, Tag # some debugging _debug = 0 @@ -104,6 +104,22 @@ class TestUnsigned(unittest.TestCase): with self.assertRaises(ValueError): Unsigned(-1) + def test_unsigned8(self): + if _debug: TestUnsigned._debug("test_unsigned8") + + with self.assertRaises(ValueError): + Unsigned8(-1) + with self.assertRaises(ValueError): + Unsigned8(256) + + def test_unsigned16(self): + if _debug: TestUnsigned._debug("test_unsigned16") + + with self.assertRaises(ValueError): + Unsigned16(-1) + with self.assertRaises(ValueError): + Unsigned16(65536) + def test_unsigned_tag(self): if _debug: TestUnsigned._debug("test_unsigned_tag")