From a3696152151bb3dda70fea248709d5093c237e86 Mon Sep 17 00:00:00 2001 From: scaramallion Date: Thu, 17 Dec 2020 08:57:16 +1100 Subject: [PATCH] Handle reserved abort source (#563) --- docs/changelog/index.rst | 1 + docs/changelog/v1.5.4.rst | 10 ++++++++++ pynetdicom/_version.py | 2 +- pynetdicom/pdu.py | 12 ++++++------ pynetdicom/pdu_primitives.py | 21 ++++++++------------- pynetdicom/tests/test_assoc.py | 28 +++++++++++++++++++++++++++- pynetdicom/tests/test_primitives.py | 4 +++- 7 files changed, 56 insertions(+), 22 deletions(-) create mode 100644 docs/changelog/v1.5.4.rst diff --git a/docs/changelog/index.rst b/docs/changelog/index.rst index d460ef5ee8..f0de81d3e1 100644 --- a/docs/changelog/index.rst +++ b/docs/changelog/index.rst @@ -7,6 +7,7 @@ Release Notes .. toctree:: :maxdepth: 1 + v1.5.4 v1.5.3 v1.5.2 v1.5.1 diff --git a/docs/changelog/v1.5.4.rst b/docs/changelog/v1.5.4.rst new file mode 100644 index 0000000000..7fdc479b09 --- /dev/null +++ b/docs/changelog/v1.5.4.rst @@ -0,0 +1,10 @@ +.. _v1.5.4: + +1.5.4 +===== + +Fixes +..... + +* Fixed not handling an A-ABORT with a reserved (``0x01``) 'source' value + (:issue:`561`) diff --git a/pynetdicom/_version.py b/pynetdicom/_version.py index d17073d0dd..c68982d054 100644 --- a/pynetdicom/_version.py +++ b/pynetdicom/_version.py @@ -10,7 +10,7 @@ import re -__version__ = '1.5.3' +__version__ = '1.5.4' VERSION_PATTERN = r""" diff --git a/pynetdicom/pdu.py b/pynetdicom/pdu.py index af2b6031b8..345fce5742 100644 --- a/pynetdicom/pdu.py +++ b/pynetdicom/pdu.py @@ -1906,15 +1906,15 @@ def to_primitive(self): """ from pynetdicom.pdu_primitives import A_ABORT, A_P_ABORT - # User initiated abort - if self.source == 0x00: + if self.source == 0x02: + # User provider primitive abort + primitive = A_P_ABORT() + primitive.provider_reason = self.reason_diagnostic + else: + # User initiated abort and undefined abort source primitive = A_ABORT() primitive.abort_source = self.source - # User provider primitive abort - elif self.source == 0x02: - primitive = A_P_ABORT() - primitive.provider_reason = self.reason_diagnostic return primitive diff --git a/pynetdicom/pdu_primitives.py b/pynetdicom/pdu_primitives.py index cc61918b7c..e3e8ec07fe 100644 --- a/pynetdicom/pdu_primitives.py +++ b/pynetdicom/pdu_primitives.py @@ -836,13 +836,13 @@ class A_ABORT(object): """ def __init__(self): - self.abort_source = None + self._abort_source = None @property def abort_source(self): """Return the *Abort Source*.""" if self._abort_source is None: - LOGGER.error("A_ABORT.abort_source parameter not set") + LOGGER.error("A_ABORT.abort_source value not set") raise ValueError("A_ABORT.abort_source value not set") return self._abort_source @@ -851,15 +851,12 @@ def abort_source(self): def abort_source(self, value): """Set the Abort Source.""" # pylint: disable=attribute-defined-outside-init - if value in [0, 2]: + if value in [0, 1, 2, None]: self._abort_source = value - elif value is None: - self._abort_source = None else: - LOGGER.error("Attempted to set A_ABORT.abort_source to an " - "invalid value") - raise ValueError("Attempted to set A_ABORT.abort_source to an " - "invalid value") + msg = "Invalid A-ABORT 'source' value '{}'".format(value) + LOGGER.error(msg) + raise ValueError(msg) class A_P_ABORT(object): @@ -899,7 +896,7 @@ class A_P_ABORT(object): * DICOM Standard, Part 8, :dcm:`Section 7.4` """ def __init__(self): - self.provider_reason = None + self._provider_reason = None @property def provider_reason(self): @@ -914,10 +911,8 @@ def provider_reason(self): def provider_reason(self, value): """Set the Provider Reason.""" # pylint: disable=attribute-defined-outside-init - if value in [0, 1, 2, 4, 5, 6]: + if value in [0, 1, 2, 4, 5, 6, None]: self._provider_reason = value - elif value is None: - self._provider_reason = None else: msg = ( "Attempted to set A_P_ABORT.provider_reason to an invalid " diff --git a/pynetdicom/tests/test_assoc.py b/pynetdicom/tests/test_assoc.py index 2e98d347dc..66e9fdea2d 100644 --- a/pynetdicom/tests/test_assoc.py +++ b/pynetdicom/tests/test_assoc.py @@ -40,7 +40,7 @@ from pynetdicom.pdu_primitives import ( UserIdentityNegotiation, SOPClassExtendedNegotiation, SOPClassCommonExtendedNegotiation, SCP_SCU_RoleSelectionNegotiation, - AsynchronousOperationsWindowNegotiation, A_ASSOCIATE, + AsynchronousOperationsWindowNegotiation, A_ASSOCIATE ) from pynetdicom.sop_class import ( VerificationSOPClass, @@ -580,6 +580,32 @@ def handle_req(event): scp.shutdown() + def test_unknown_abort_source(self): + """Test an unknown abort source handled correctly #561""" + def handle_req(event): + pdu = b"\x07\x00\x00\x00\x00\x04\x00\x00\x01\x00" + event.assoc.dul.socket.send(pdu) + # Give the requestor time to process the message before killing + # the connection + time.sleep(0.1) + + self.ae = ae = AE() + ae.acse_timeout = 5 + ae.dimse_timeout = 5 + ae.network_timeout = 5 + ae.add_supported_context(VerificationSOPClass) + + hh = [(evt.EVT_REQUESTED, handle_req)] + + scp = ae.start_server(('', 11112), block=False, evt_handlers=hh) + + ae.add_requested_context(VerificationSOPClass) + assoc = ae.associate('localhost', 11112) + assert not assoc.is_established + assert assoc.is_aborted + + scp.shutdown() + class TestCStoreSCP(object): """Tests for Association._c_store_scp().""" diff --git a/pynetdicom/tests/test_primitives.py b/pynetdicom/tests/test_primitives.py index 620dd4eb70..52da529fd5 100644 --- a/pynetdicom/tests/test_primitives.py +++ b/pynetdicom/tests/test_primitives.py @@ -1013,6 +1013,8 @@ def test_assignment(self): primitive = A_ABORT() primitive.abort_source = 0 assert primitive.abort_source == 0 + primitive.abort_source = 1 + assert primitive.abort_source == 1 primitive.abort_source = 2 assert primitive.abort_source == 2 @@ -1021,7 +1023,7 @@ def test_exceptions(self): primitive = A_ABORT() with pytest.raises(ValueError): - primitive.abort_source = 1 + primitive.abort_source = 3 with pytest.raises(ValueError): primitive.abort_source