Skip to content

Improve handling of maximum PDU lengths in scpproxy and lower the negotiable size#708

Merged
Enet4 merged 3 commits intomasterfrom
imp/ul/min-max-pdu
Oct 14, 2025
Merged

Improve handling of maximum PDU lengths in scpproxy and lower the negotiable size#708
Enet4 merged 3 commits intomasterfrom
imp/ul/min-max-pdu

Conversation

@Enet4
Copy link
Owner

@Enet4 Enet4 commented Oct 11, 2025

Summary

Enet4 added 2 commits October 11, 2025 20:07
- [findscu][storescp][storescu][scpproxy] Update max_pdu_length option accordingly
- modify association requests and acknowledgements
  so that they are not higher than
  the proxy's maximum PDU length
- protect buffer allocations
@Enet4 Enet4 self-assigned this Oct 11, 2025
@Enet4 Enet4 added enhancement A-lib Area: library A-tool Area: tooling C-ul Crate: dicom-ul C-scpproxy Crate: dicom-scproxy labels Oct 11, 2025
@pgimeno4d
Copy link
Contributor

pgimeno4d commented Oct 13, 2025

The test is failing because it is splitting 9000 bytes in several sends, and expects them to be split in three fragments (4K + 4K + about 1K). Now that the 4K have been reduced to 1K, it's no longer three fragments. So I propose to change 9000 to 2500 which is 1K + 1K + about 0.5K:

diff --git a/ul/src/association/pdata.rs b/ul/src/association/pdata.rs
index 9a770dce..cde40326 100644
--- a/ul/src/association/pdata.rs
+++ b/ul/src/association/pdata.rs
@@ -776,7 +776,7 @@ mod tests {
     fn test_write_large_pdata_and_finish() {
         let presentation_context_id = 32;
 
-        let my_data: Vec<_> = (0..9000).map(|x: u32| x as u8).collect();
+        let my_data: Vec<_> = (0..2500).map(|x: u32| x as u8).collect();
 
         let mut buf = Vec::new();
         {
@@ -822,7 +822,7 @@ mod tests {
                 );
                 assert_eq!(
                     data_3.data.len(),
-                    9000 - (MINIMUM_PDU_SIZE - PDV_HEADER_SIZE) as usize * 2
+                    2500 - (MINIMUM_PDU_SIZE - PDV_HEADER_SIZE) as usize * 2
                 );
 
                 // check data consistency
@@ -834,7 +834,7 @@ mod tests {
                 );
                 assert_eq!(
                     data_1.data.len() + data_2.data.len() + data_3.data.len(),
-                    9000
+                    2500
                 );
 
                 let data_1 = &data_1.data;
@@ -858,7 +858,7 @@ mod tests {
     async fn test_async_write_large_pdata_and_finish() {
         let presentation_context_id = 32;
 
-        let my_data: Vec<_> = (0..9000).map(|x: u32| x as u8).collect();
+        let my_data: Vec<_> = (0..2500).map(|x: u32| x as u8).collect();
 
         let mut buf = Vec::new();
         {
@@ -905,7 +905,7 @@ mod tests {
                 );
                 assert_eq!(
                     data_3.data.len(),
-                    9000 - (MINIMUM_PDU_SIZE - PDV_HEADER_SIZE) as usize * 2
+                    2500 - (MINIMUM_PDU_SIZE - PDV_HEADER_SIZE) as usize * 2
                 );
 
                 // check data consistency
@@ -917,7 +917,7 @@ mod tests {
                 );
                 assert_eq!(
                     data_1.data.len() + data_2.data.len() + data_3.data.len(),
-                    9000
+                    2500
                 );
 
                 let data_1 = &data_1.data;

(edit: full commit here: pgimeno4d@b717aee )
Or alternatively, making it calculated, but not sure it's worth it.

@Enet4
Copy link
Owner Author

Enet4 commented Oct 13, 2025

Thanks for the input @pgimeno4d! I went with that suggestion in order to fix the tests.

@naterichman naterichman mentioned this pull request Oct 14, 2025
@Enet4 Enet4 merged commit 23f2b76 into master Oct 14, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-lib Area: library A-tool Area: tooling C-scpproxy Crate: dicom-scproxy C-ul Crate: dicom-ul enhancement

Projects

None yet

2 participants