Limit QR C-MOVE and C-GET to 65535 suboperations (#983)

.github/workflows/merge-pytest.yml

@@ -13,7 +13,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ['3.10', '3.11', '3.12']
+        python-version: ['3.10', '3.11', '3.12', '3.13']
 
     steps:
     - uses: actions/checkout@v4
@@ -42,7 +42,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ['3.10', '3.11', '3.12']
+        python-version: ['3.10', '3.11', '3.12', '3.13']
 
     steps:
       - uses: actions/checkout@v4
@@ -105,7 +105,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ['3.10', '3.11', '3.12']
+        python-version: ['3.10', '3.11', '3.12', '3.13']
 
     steps:
     - uses: actions/checkout@v4

.github/workflows/pr-pytest.yml

@@ -107,7 +107,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ['3.10', '3.11', '3.12']
+        python-version: ['3.10', '3.11', '3.12', '3.13']
 
     steps:
     - uses: actions/checkout@v4

docs/changelog/v2.2.0.rst

@@ -0,0 +1,11 @@
+.. _v2.2.0:
+
+2.2.0
+=====
+
+Changes
+.......
+
+* Move and Get SCPs are now limited to a maximum of 65535 matches (:issue:`982`)
+* Minimum supported pydicom version is 3.0 (:issue:`981`)
+* Added support for Python 3.13

docs/examples/qr_get.rst

@@ -3,7 +3,7 @@ Query/Retrieve (Get) Service Examples
 
 The DICOM :dcm:`Query/Retrieve Service <part04/chapter_C.html>`
 provides a mechanism for a service user to query and retrieve the SOP Instances
-managed by a QR SCP. The QR (Get) SOP classes allow an SCU to receive SOP
+managed by a QR SCP. The QR (Get) SOP classes allow an SCU to receive up to 65535 SOP
 Instances that match the requested query. This is accomplished through the
 DIMSE C-GET and C-STORE services. Both query and
 retrieval occur over the same association, with the SCP of the Query/Retrieve

docs/examples/qr_move.rst

@@ -4,7 +4,7 @@ Query/Retrieve (Move) Service Examples
 The DICOM :dcm:`Query/Retrieve Service <part04/chapter_C.html>`
 provides a mechanism for a service user to query and retrieve the SOP Instances
 managed by a QR SCP. The QR (Move) SOP classes allow an SCU to request an SCP
-send matching SOP Instances to a known Storage SCP over a new association.
+send up to 65535 matching SOP Instances to a known Storage SCP over a new association.
 This is accomplished through the DIMSE C-MOVE and C-STORE services.
 
 One limitation of the C-MOVE service is that the Move SCP/Storage SCU must

docs/service_classes/query_retrieve_service_class.rst

@@ -236,6 +236,9 @@ pynetdicom Query/Retrieve (Get) Statuses
 | 0xC413           | Failure  | The handler bound to ``evt.EVT_C_GET``        |
 |                  |          | yielded an invalid number of sub-operations   |
 +------------------+----------+-----------------------------------------------+
+| 0xC416           | Failure  | The handler bound to ``evt.EVT_C_GET``        |
+|                  |          | yielded more than 65535 matches               |
++------------------+----------+-----------------------------------------------+
 
 
 .. _qr_move_statuses:
@@ -328,3 +331,6 @@ pynetdicom Query/Retrieve (Move) Statuses
 | 0xC515           | Failure  | The handler bound to ``evt.EVT_C_MOVE``       |
 |                  |          | failed to yield a valid (address, port) pair  |
 +------------------+----------+-----------------------------------------------+
+| 0xC516           | Failure  | The handler bound to ``evt.EVT_C_MOVE``       |
+|                  |          | yielded more than 65535 matches               |
++------------------+----------+-----------------------------------------------+

pynetdicom/service_class.py

@@ -1694,23 +1694,31 @@ def _get_scp(self, req: C_GET, context: "PresentationContext") -> None:
                 "sub-operations value"
             )
             ctx.error_status = 0xC413
-            no_suboperations = int(next(generator))
+            nr_suboperations = int(next(generator))
 
         if not ctx.success:
             return
 
-        if no_suboperations < 1:
+        if nr_suboperations < 1:
             rsp.Status = 0x0000
-            rsp.NumberOfRemainingSuboperations = 0
             rsp.NumberOfFailedSuboperations = 0
             rsp.NumberOfWarningSuboperations = 0
             rsp.NumberOfCompletedSuboperations = 0
             self.dimse.send_msg(rsp, cx_id)
             return
 
+        if nr_suboperations > 65535:
+            # Since Number of * Suboperations are US and have a maximum value of 65535,
+            #   and Sections C.4.3.2 and C.4.3.3 require them in the final response,
+            #   that implies no more than 65535 matches
+            LOGGER.error("The C-GET request handler yielded more than 65535 matches")
+            rsp.Status = 0xC416
+            self.dimse.send_msg(rsp, cx_id)
+            return
+
         # Track the sub operation results
         #   [remaining, failed, warning, complete]
-        store_results = [no_suboperations, 0, 0, 0]
+        store_results = [nr_suboperations, 0, 0, 0]
 
         # Store the SOP Instance UIDs from any failed C-STORE sub-operations
         failed_instances = []
@@ -1797,7 +1805,6 @@ def _add_failed_instance(ds: Dataset) -> None:
                     f"Get SCP Result {ii + 1}: 0x{rsp.Status:04X} "
                     f"({status[0]} - {status[1]})"
                 )
-                rsp.NumberOfRemainingSuboperations = None
                 rsp.NumberOfFailedSuboperations = store_results[1] + store_results[0]
                 rsp.NumberOfWarningSuboperations = store_results[2]
                 rsp.NumberOfCompletedSuboperations = store_results[3]
@@ -1838,7 +1845,6 @@ def _add_failed_instance(ds: Dataset) -> None:
                     LOGGER.info(f"Get SCP Response {ii + 1}: 0x0000 (Success)")
                     rsp.Identifier = None
 
-                rsp.NumberOfRemainingSuboperations = None
                 rsp.NumberOfFailedSuboperations = store_results[1]
                 rsp.NumberOfWarningSuboperations = store_results[2]
                 rsp.NumberOfCompletedSuboperations = store_results[3]
@@ -1857,6 +1863,7 @@ def _add_failed_instance(ds: Dataset) -> None:
                     rsp.NumberOfFailedSuboperations = store_results[1]
                     rsp.NumberOfWarningSuboperations = store_results[2]
                     rsp.NumberOfCompletedSuboperations = store_results[3]
+
                     self.dimse.send_msg(rsp, cx_id)
                     continue
 
@@ -1961,7 +1968,7 @@ def _add_failed_instance(ds: Dataset) -> None:
             rsp.Status = 0x0000
             rsp.Identifier = None
         else:
-            if no_suboperations == store_results[1]:
+            if nr_suboperations == store_results[1]:
                 # Failure response - all sub-operations failed
                 LOGGER.info(f"Get SCP Response {ii + 2}: 0xA702 (Failure)")
                 rsp.Status = 0xA702  # Unable to perform sub-ops
@@ -1982,7 +1989,6 @@ def _add_failed_instance(ds: Dataset) -> None:
             )
             rsp.Identifier = BytesIO(cast(bytes, bytestream))
 
-        rsp.NumberOfRemainingSuboperations = None
         rsp.NumberOfFailedSuboperations = store_results[1]
         rsp.NumberOfWarningSuboperations = store_results[2]
         rsp.NumberOfCompletedSuboperations = store_results[3]
@@ -2085,21 +2091,29 @@ def _move_scp(self, req: C_MOVE, context: "PresentationContext") -> None:
                 "sub-operations value"
             )
             ctx.error_status = 0xC513
-            no_suboperations = int(next(generator))
+            nr_suboperations = int(next(generator))
 
         # Exception in context or handler aborted/released - second yield
         if not ctx.success or not self.assoc.is_established:
             return
 
-        if no_suboperations < 1:
+        if nr_suboperations < 1:
             rsp.Status = 0x0000
-            rsp.NumberOfRemainingSuboperations = 0
             rsp.NumberOfFailedSuboperations = 0
             rsp.NumberOfWarningSuboperations = 0
             rsp.NumberOfCompletedSuboperations = 0
             self.dimse.send_msg(rsp, cx_id)
             return
 
+        if nr_suboperations > 65535:
+            # Since Number of * Suboperations are US and have a maximum value of 65535,
+            #   and Sections C.4.2.2 and C.4.2.3 require them in the final response,
+            #   that implies no more than 65535 matches
+            LOGGER.error("The C-MOVE request handler yielded more than 65535 matches")
+            rsp.Status = 0xC516
+            self.dimse.send_msg(rsp, cx_id)
+            return
+
         # Try to request new association with Move Destination
         with attempt(rsp, self.dimse, cx_id) as ctx:
             ctx.error_msg = (
@@ -2131,7 +2145,7 @@ def _move_scp(self, req: C_MOVE, context: "PresentationContext") -> None:
 
         # Track the sub operation results
         #   [remaining, failed, warning, complete]
-        store_results = [no_suboperations, 0, 0, 0]
+        store_results = [nr_suboperations, 0, 0, 0]
 
         # Store the SOP Instance UIDs from any failed C-STORE sub-operations
         failed_instances = []
@@ -2243,7 +2257,6 @@ def _add_failed_instance(ds: Dataset) -> None:
                 )
 
                 rsp.Identifier = BytesIO(cast(bytes, bytestream))
-                rsp.NumberOfRemainingSuboperations = None
                 rsp.NumberOfFailedSuboperations = store_results[1] + store_results[0]
                 rsp.NumberOfWarningSuboperations = store_results[2]
                 rsp.NumberOfCompletedSuboperations = store_results[3]
@@ -2275,7 +2288,6 @@ def _add_failed_instance(ds: Dataset) -> None:
                     LOGGER.info(f"Move SCP Response {ii + 1}: 0x0000 (Success)")
                     rsp.Identifier = None
 
-                rsp.NumberOfRemainingSuboperations = None
                 rsp.NumberOfFailedSuboperations = store_results[1]
                 rsp.NumberOfWarningSuboperations = store_results[2]
                 rsp.NumberOfCompletedSuboperations = store_results[3]
@@ -2367,7 +2379,7 @@ def _add_failed_instance(ds: Dataset) -> None:
             rsp.Status = 0x0000
             rsp.Identifier = None
         else:
-            if no_suboperations == store_results[1]:
+            if nr_suboperations == store_results[1]:
                 # Failure response - all sub-operations failed
                 LOGGER.info(f"Move SCP Response {ii + 2}: 0xA702 (Failure)")
                 rsp.Status = 0xA702  # Unable to perform sub-ops
@@ -2388,7 +2400,6 @@ def _add_failed_instance(ds: Dataset) -> None:
             )
             rsp.Identifier = BytesIO(cast(bytes, bytestream))
 
-        rsp.NumberOfRemainingSuboperations = None
         rsp.NumberOfFailedSuboperations = store_results[1]
         rsp.NumberOfWarningSuboperations = store_results[2]
         rsp.NumberOfCompletedSuboperations = store_results[3]

pynetdicom/tests/test_service_qr.py

@@ -10,6 +10,7 @@
 """
 
 from io import BytesIO
+import logging
 import os
 import time
 
@@ -1390,6 +1391,50 @@ def handle_store(event):
         assert assoc.is_released
         scp.shutdown()
 
+    def test_get_handler_too_many_subops(self, caplog):
+        """Test handler yielding more than 65535 subops"""
+
+        def handle(event):
+            yield 65536
+            status = Dataset()
+            status.Status = 0xFF00
+            yield status, self.ds
+            yield 0x0000, None
+
+        def handle_store(event):
+            return 0x0000
+
+        handlers = [(evt.EVT_C_GET, handle), (evt.EVT_C_STORE, handle_store)]
+
+        self.ae = ae = AE()
+        ae.add_supported_context(PatientRootQueryRetrieveInformationModelGet)
+        ae.add_supported_context(CTImageStorage, scu_role=False, scp_role=True)
+        ae.add_requested_context(PatientRootQueryRetrieveInformationModelGet)
+        ae.add_requested_context(CTImageStorage)
+        scp = ae.start_server(("localhost", 11112), block=False, evt_handlers=handlers)
+
+        role = build_role(CTImageStorage, scp_role=True)
+
+        ae.acse_timeout = 5
+        ae.dimse_timeout = 5
+        assoc = ae.associate("localhost", 11112, ext_neg=[role])
+        assert assoc.is_established
+        with caplog.at_level(logging.ERROR, logger="pynetdicom"):
+            result = assoc.send_c_get(
+                self.query, PatientRootQueryRetrieveInformationModelGet
+            )
+            status, identifier = next(result)
+            assert status.Status == 0xC416
+            pytest.raises(StopIteration, next, result)
+
+            assoc.release()
+            assert assoc.is_released
+            scp.shutdown()
+
+        assert (
+            "The C-GET request handler yielded more than 65535 matches" in caplog.text
+        )
+
     def test_get_handler_status_dataset(self):
         """Test handler yielding a Dataset status"""
 
@@ -3869,6 +3914,42 @@ def handle(event):
         assoc.release()
         scp.shutdown()
 
+    def test_move_handler_too_many_subops(self, caplog):
+        """Test handler yielding more than 65535 subops"""
+
+        def handle(event):
+            yield ("127.0.0.1", 11112)
+            yield 65536
+            yield 0xFF00, self.ds
+
+        handlers = [(evt.EVT_C_MOVE, handle)]
+
+        self.ae = ae = AE()
+        ae.add_supported_context(PatientRootQueryRetrieveInformationModelMove)
+        ae.add_supported_context(CTImageStorage, scu_role=False, scp_role=True)
+        ae.add_requested_context(PatientRootQueryRetrieveInformationModelMove)
+        ae.add_requested_context(CTImageStorage)
+        scp = ae.start_server(("localhost", 11112), block=False, evt_handlers=handlers)
+
+        ae.acse_timeout = 5
+        ae.dimse_timeout = 5
+        assoc = ae.associate("localhost", 11112)
+        assert assoc.is_established
+        with caplog.at_level(logging.ERROR, logger="pynetdicom"):
+            result = assoc.send_c_move(
+                self.query, "TESTMOVE", PatientRootQueryRetrieveInformationModelMove
+            )
+            status, identifier = next(result)
+            assert status.Status == 0xC516
+            pytest.raises(StopIteration, next, result)
+
+            assoc.release()
+            scp.shutdown()
+
+        assert (
+            "The C-MOVE request handler yielded more than 65535 matches" in caplog.text
+        )
+
     def test_move_handler_bad_aet(self):
         """Test handler yielding a bad move aet"""
 

pyproject.toml

@@ -17,6 +17,8 @@ classifiers=[
     "Programming Language :: Python :: 3.10",
     "Programming Language :: Python :: 3.11",
     "Programming Language :: Python :: 3.12",
+    "Programming Language :: Python :: 3.13",
+    "Programming Language :: Python :: 3.14",
     "Operating System :: OS Independent",
     "Topic :: Scientific/Engineering :: Medical Science Apps.",
     "Topic :: Software Development :: Libraries",