[MRG] Add better handling of invalid A-ASSOCIATE (RJ) parameters during ACSE negotiation

docs/changelog/v2.0.0.rst

@@ -48,6 +48,8 @@ Enhancements
   option (:issue:`620`)
 * Updated to meet the 2021b version of the DICOM Standard
 * Added type hints
+* Handle non-conformant A-ASSOCIATE (RJ) 'Result', 'Source' and 'Diagnostic'
+  values during ACSE negotiation (:issue:`633`)
 
 Changes
 .......

pynetdicom/pdu.py

@@ -1225,16 +1225,14 @@ def reason_str(self) -> str:
         }
 
         if self.source not in _reasons:
-            LOGGER.error('Invalid value in Source field in '
-                         'A-ASSOCIATE-RJ PDU')
-            raise ValueError('Invalid value in Source field in '
-                             'A-ASSOCIATE-RJ PDU')
+            msg = 'Invalid value in Source field in A-ASSOCIATE-RJ PDU'
+            LOGGER.error(msg)
+            raise ValueError(msg)
 
         if self.reason_diagnostic not in _reasons[self.source]:
-            LOGGER.error('Invalid value in Reason field in '
-                         'A-ASSOCIATE-RJ PDU')
-            raise ValueError('Invalid value in Reason field in '
-                             'A-ASSOCIATE-RJ PDU')
+            msg = 'Invalid value in Reason field in A-ASSOCIATE-RJ PDU'
+            LOGGER.error(msg)
+            raise ValueError(msg)
 
         return _reasons[self.source][self.reason_diagnostic]
 
@@ -1247,10 +1245,9 @@ def result_str(self) -> str:
         }
 
         if self.result not in _results:
-            LOGGER.error('Invalid value in Result field in '
-                         'A-ASSOCIATE-RJ PDU')
-            raise ValueError('Invalid value in Result field in '
-                             'A-ASSOCIATE-RJ PDU')
+            msg = 'Invalid value in Result field in A-ASSOCIATE-RJ PDU'
+            LOGGER.error(msg)
+            raise ValueError(msg)
 
         return _results[self.result]
 
@@ -1264,10 +1261,9 @@ def source_str(self) -> str:
         }
 
         if self.source not in _sources:
-            LOGGER.error('Invalid value in Source field in '
-                         'A-ASSOCIATE-RJ PDU')
-            raise ValueError('Invalid value in Source field in '
-                             'A-ASSOCIATE-RJ PDU')
+            msg = 'Invalid value in Source field in A-ASSOCIATE-RJ PDU'
+            LOGGER.error(msg)
+            raise ValueError(msg)
 
         return _sources[self.source]
 

pynetdicom/pdu_primitives.py

@@ -345,8 +345,17 @@ def result(self, value: Optional[int]) -> None:
     @property
     def result_str(self) -> str:
         """Return the result as str."""
-        results = {1: "Rejected Permanent", 2: "Rejected Transient"}
-        return results[cast(int, self.result)]
+        results = {
+            0: "Accepted", 1: "Rejected Permanent", 2: "Rejected Transient"
+        }
+
+        if self.result not in results:
+            LOGGER.error(
+                f"Invalid A-ASSOCIATE 'Result' {self.result}"
+            )
+            return "(no value available)"
+
+        return results[self.result]
 
     @property
     def result_source(self) -> Optional[int]:
@@ -387,7 +396,13 @@ def source_str(self) -> str:
             2: 'Service Provider (ACSE)',
             3: 'Service Provider (Presentation)'
         }
-        return sources[cast(int, self.result_source)]
+        if self.result_source not in sources:
+            LOGGER.error(
+                f"Invalid A-ASSOCIATE 'Result Source' {self.result_source}"
+            )
+            return "(no value available)"
+
+        return sources[self.result_source]
 
     @property
     def diagnostic(self) -> Optional[int]:
@@ -463,7 +478,14 @@ def reason_str(self) -> str:
         }
         result = cast(int, self.result_source)
         diagnostic = cast(int, self.diagnostic)
-        return reasons[result][diagnostic]
+        try:
+            return reasons[result][diagnostic]
+        except KeyError:
+            LOGGER.error(
+                f"Invalid A-ASSOCIATE 'Result Source' {result} and/or "
+                f"'Diagnostic' {diagnostic} values"
+            )
+            return "(no value available)"
 
     @property
     def calling_presentation_address(self) -> Optional[Tuple[str, int]]:

pynetdicom/tests/test_primitives.py

@@ -1012,6 +1012,35 @@ def test_conversion(self):
             b"\x43\x4f\x4d\x5f\x30\x39\x30"
         )
 
+    def test_invalid_result_str(self, caplog):
+        """Test an invalid result value gets logged and doesn't raise."""
+        pdu = A_ASSOCIATE()
+        pdu.result = None
+        with caplog.at_level(logging.ERROR, logger='pynetdicom'):
+            assert pdu.result_str == '(no value available)'
+            assert "Invalid A-ASSOCIATE 'Result' None" in caplog.text
+
+    def test_invalid_source_str(self, caplog):
+        """Test an invalid source value gets logged and doesn't raise."""
+        pdu = A_ASSOCIATE()
+        pdu.result_source = None
+        with caplog.at_level(logging.ERROR, logger='pynetdicom'):
+            assert pdu.source_str == '(no value available)'
+            assert "Invalid A-ASSOCIATE 'Result Source' None" in caplog.text
+
+    def test_invalid_reason_str(self, caplog):
+        """Test an invalid diagnostic value gets logged and doesn't raise."""
+        pdu = A_ASSOCIATE()
+        pdu.result = 1
+        pdu.result_source = 2
+        pdu.diagnostic = 7
+        with caplog.at_level(logging.ERROR, logger='pynetdicom'):
+            assert pdu.reason_str == '(no value available)'
+            assert (
+                "Invalid A-ASSOCIATE 'Result Source' 2 and/or "
+                "'Diagnostic' 7 values"
+            ) in caplog.text
+
 
 class TestPrimitive_A_RELEASE:
     def test_assignment(self):