Decoding empty tag should raise DecodingError about bacpypes

[ChristianTremblay]

@JoelBender

Just to be sure I understand it right. Is this code would need to raise the error ?

from bacpypes.primitivedata import Tag, Enumerated
from bacpypes.debugging import bacpypes_debugging, ModuleLogger, xtob

tag = Tag(Tag.applicationTagClass, Tag.enumeratedAppTag, 1, xtob(''))
obj = Enumerated(tag)
# ! Bang -> Decoding Error

Is a specific call to decode() necessary ?

[JoelBender]

That's one form that will generate an error. Your example shows a tag that is inconsistent because the LVT field (in this case interpreted as "length") is one (1) but the length of the tag data is zero (0). The current code doesn't have test for this error, so adding something like this is a good idea:

if (tag.tagLVT != len(tag.tagData)):
    raise DecodingError("inconsistent tag")

The sample test is equivalent to this, which should also raise an error:

tag = Tag(Tag.applicationTagClass, Tag.enumeratedAppTag, 0, xtob(''))

because there must be at least one octet of data (and four octets for Real, Date, and Time, etc.) The test code is already in tests/test_primitive_data/test_enumerated.py, it's just commented out:

#       with self.assertRaises(DecodingError):
#           obj = Enumerated(enumerated_tag(''))

Your mission, now that this is assigned to you :-), is to uncomment the test and fix the decode() function.

It's not necessary to explicitly call decode() because having it implicitly called by __init__() is part of the "signature" of the function.

[JoelBender]

You are going to be making (or you have already made) a new branch issue-50 from stage, correct? If I'm thinking correctly, it should contain the 49f990c commit and go forward from there.

[JoelBender]

After reviewing 18.9 Reject Reason in the standard, perhaps the exception should be called InvalidTag:

INVALID_TAG - While parsing a message, an invalid tag was encountered. Since an invalid tag could confuse the parsing logic, any of the following Reject Reasons may also be generated in response to a confirmed request containing an invalid tag: INCONSISTENT_PARAMETERS, INVALID_PARAMETER_DATA_TYPE, MISSING_REQUIRED_PARAMETER, and TOO_MANY_ARGUMENTS.

When this exception is raised during the decoding of an APDU before it is sent upstream for processing, it will need to be trapped and a RejectPDU sent to the client (I'll make a separate issue).

[ChristianTremblay]

Create an InvalidTagError exception that inherit from DecodingError (to be confirmed... if ok, if not, just change it to inherit from ValueError

This line is added to primitivedata encode function :

    if tag.tagData == b'':
        raise InvalidTagError('empty tag')

Did it for :

• Enumerated
• Date
• Double
• Integer
• object_identifier
• object_type
• real
• time
• unsigned

Seems not appropriate for :

• Null
• Boolean
• BitString
• octet_string

[JoelBender]

This has been subsumed by #58, which has all the reject and abort exceptions, includes all three distributions, and I included the updates to the tests as well. I'm closing this issue and abandoning the branch.