Skip to content

Commit

Permalink
Fix failure decoding rejected contexts with empty transfer syntax (#344)
Browse files Browse the repository at this point in the history
  • Loading branch information
scaramallion authored Apr 22, 2019
1 parent c7e7004 commit bb1f9d1
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 6 deletions.
11 changes: 11 additions & 0 deletions docs/changelog/v1.3.1.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
.. _v1.3.1:

1.3.1
=====


Fixes
.....

* Fixed association being aborted due to a rejected presentation context
performing a validation check on the transfer syntax value (:issue:`342`)
1 change: 1 addition & 0 deletions docs/release_notes.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
.. currentmodule:: pynetdicom

.. include:: changelog/v1.3.1.rst
.. include:: changelog/v1.3.0.rst
.. include:: changelog/v1.2.0.rst
.. include:: changelog/v1.1.0.rst
Expand Down
2 changes: 1 addition & 1 deletion pynetdicom/_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import re


__version__ = '1.3.0'
__version__ = '1.3.1'


VERSION_PATTERN = r"""
Expand Down
31 changes: 27 additions & 4 deletions pynetdicom/pdu_items.py
Original file line number Diff line number Diff line change
Expand Up @@ -961,8 +961,9 @@ def __str__(self):
s += " Context ID: {0:d}\n".format(self.presentation_context_id)
s += " Result/Reason: {0!s}\n".format(self.result_str)

item_str = '{0!s}'.format(self.transfer_syntax.name)
s += ' + {0!s}\n'.format(item_str)
if self.transfer_syntax:
item_str = '{0!s}'.format(self.transfer_syntax.name)
s += ' + {0!s}\n'.format(item_str)

return s

Expand All @@ -979,6 +980,20 @@ def transfer_syntax(self):

return None

def _wrap_generate_items(self, bytestream):
"""Return a list of decoded PDU items generated from `bytestream`."""
item_list = []
for item_type, item_bytes in self._generate_items(bytestream):
item = PDU_ITEM_TYPES[item_type]()
# Transfer Syntax items shall not have their value tested if
# not accepted
if item_type == 0x40 and self.result != 0x00:
item._skip_validation = True
item.decode(item_bytes)
item_list.append(item)

return item_list


class UserInformationItem(PDUItem):
"""A User Information Item.
Expand Down Expand Up @@ -1492,6 +1507,8 @@ class TransferSyntaxSubItem(PDUItem):

def __init__(self):
"""Initialise a new Abstract Syntax Item."""
# Should not be validated if Presentation Context was rejected
self._skip_validation = False
self.transfer_syntax_name = None

@property
Expand Down Expand Up @@ -1547,8 +1564,10 @@ def __str__(self):
s = "Transfer syntax sub item\n"
s += " Item type: 0x{0:02x}\n".format(self.item_type)
s += " Item length: {0:d} bytes\n".format(self.item_length)
s += ' Transfer syntax name: ={0!s}\n'.format(
self.transfer_syntax_name.name)
if self.transfer_syntax_name:
s += ' Transfer syntax name: ={0!s}\n'.format(
self.transfer_syntax_name.name
)

return s

Expand Down Expand Up @@ -1584,6 +1603,10 @@ def transfer_syntax_name(self, value):
raise TypeError('Transfer syntax must be a pydicom.uid.UID, '
'bytes or str')

if self._skip_validation:
self._transfer_syntax_name = value or None
return

if value is not None and not validate_uid(value):
LOGGER.error("Transfer Syntax Name is an invalid UID")
raise ValueError("Transfer Syntax Name is an invalid UID")
Expand Down
40 changes: 40 additions & 0 deletions pynetdicom/tests/encoded_pdu_items.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,46 @@
b'\x59\x00\x00\x0a\x00\x08\x41\x63\x63\x65' \
b'\x70\x74\x65\x64'


# Issue 342
# Called AET: ANY-SCP
# Calling AET: PYNETDICOM
# Application Context Name: 1.2.840.10008.3.1.1.1
# Presentation Context Items:
# Presentation Context ID: 1
# Abstract Syntax: Verification SOP Class
# SCP/SCU Role: Default
# Result: Accepted
# Transfer Syntax: 1.2.840.10008.1.2.1 Explicit VR Little Endian
# Presentation Context ID: 3
# Abstract Syntax: Basic Grayscale Print Management Meta SOP Class
# SCP/SCU Role: Default
# Result: Abstract Syntax Not Supported
# Transfer Syntax: None
# User Information
# Max Length Received: 28672
# Implementation Class UID: 2.16.840.1
# Implementation Version Name: MergeCOM3_390IB2
# Extended Negotiation
# SOP Extended: None
# Async Ops: None
# User ID: None
a_associate_ac_zero_ts = (
b'\x02\x00\x00\x00\x00\xb6\x00\x01\x00\x00\x41\x4e\x59\x2d\x53\x43'
b'\x50\x20\x20\x20\x20\x20\x20\x20\x20\x20\x50\x59\x4e\x45\x54\x44'
b'\x49\x43\x4f\x4d\x20\x20\x20\x20\x20\x20\x00\x00\x00\x00\x00\x00'
b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x10\x00\x00\x15\x31\x2e'
b'\x32\x2e\x38\x34\x30\x2e\x31\x30\x30\x30\x38\x2e\x33\x2e\x31\x2e'
b'\x31\x2e\x31\x21\x00\x00\x1b\x01\x00\x00\x00\x40\x00\x00\x13\x31'
b'\x2e\x32\x2e\x38\x34\x30\x2e\x31\x30\x30\x30\x38\x2e\x31\x2e\x32'
b'\x2e\x31\x21\x00\x00\x08\x03\x00\x03\x00\x40\x00\x00\x00\x50\x00'
b'\x00\x2a\x51\x00\x00\x04\x00\x00\x70\x00\x52\x00\x00\x0a\x32\x2e'
b'\x31\x36\x2e\x38\x34\x30\x2e\x31\x55\x00\x00\x10\x4d\x65\x72\x67'
b'\x65\x43\x4f\x4d\x33\x5f\x33\x39\x30\x49\x42\x32'
)


############################# A-ASSOCIATE-RJ PDU ###############################
# Result: Rejected (Permanent)
# Source: DUL service-user
Expand Down
53 changes: 52 additions & 1 deletion pynetdicom/tests/test_pdu_items.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
maximum_length_received, implementation_class_uid,
implementation_version_name, role_selection, role_selection_odd,
user_information, extended_negotiation, common_extended_negotiation,
p_data_tf
p_data_tf, a_associate_ac_zero_ts
)

LOGGER = logging.getLogger('pynetdicom')
Expand Down Expand Up @@ -612,6 +612,28 @@ def test_result_str(self):
item.result_reason = result
assert item.result_str == _result[result]

def test_decode_empty(self):
"""Regression test for #342 (decoding an empty Transfer Syntax Item."""
# When result is not accepted, transfer syntax value must not be tested
item = PresentationContextItemAC()
item.decode(
b'\x21\x00\x00\x08\x01\x00\x01\x00'
b'\x40\x00\x00\x00'
)

assert item.item_type == 0x21
assert item.item_length == 8
assert item.result == 1
assert len(item) == 12
assert item.transfer_syntax is None

assert "Item length: 8 bytes" in item.__str__()

item = item.transfer_syntax_sub_item[0]
assert item.item_length == 0
assert item._skip_validation is True
assert item.transfer_syntax_name is None


class TestAbstractSyntax(object):
def setup(self):
Expand Down Expand Up @@ -880,6 +902,35 @@ def test_decode_padded_odd(self):
assert len(item) == 9
assert item.encode() == b'\x40\x00\x00\x05\x31\x2e\x32\x2e\x33'

def test_decode_empty(self):
"""Regression test for #342 (decoding an empty Transfer Syntax Item."""
pdu = A_ASSOCIATE_AC()
pdu.decode(a_associate_ac_zero_ts)

item = pdu.presentation_context[0]
assert item.item_type == 0x21
assert item.item_length == 27
assert item.result == 0
assert len(item) == 31
assert item.transfer_syntax == UID('1.2.840.10008.1.2.1')

item = pdu.presentation_context[1]
assert item.item_type == 0x21
assert item.item_length == 8
assert item.result == 3
assert len(item) == 12
assert item.transfer_syntax is None

item = TransferSyntaxSubItem()
item._skip_validation = True
item.decode(b'\x40\x00\x00\x00')
assert item.item_type == 0x40
assert item.item_length == 0
assert len(item) == 4
assert item.transfer_syntax is None
assert 'Item length: 0 bytes' in item.__str__()
assert 'Transfer syntax name' not in item.__str__()


class TestPresentationDataValue(object):
def test_init(self):
Expand Down

0 comments on commit bb1f9d1

Please sign in to comment.