[ul] Fix max PDU issues

[pgimeno4d]

Rather than implementing (send/receive)_pdata in the generic traits (Sync/Async)Association, implement them in the specialization (Async?)(Client/Server)Association, which allows specifying which max PDU length to use, the requestor's or the acceptor's.

Fixes #712. Note there are no unit tests for send/ receive_pdata.

[pgimeno4d]

I guess this change could do with unit tests that demonstrate the problem and how the patch fixes it, especially since #712 can no longer be reproduced by that issue's OP. Will work on it, although I haven't ever used send/receive_pdata.

[Enet4]

I guess this change could do with unit tests that demonstrate the problem and how the patch fixes it, especially since #712 can no longer be reproduced by that issue's OP. Will work on it, although I haven't ever used send/receive_pdata.

Would it help if I pushed a branch with a baseline test which covers send_pdata and receive_pdata? I can probably make it available within the next few hours.

For what it's worth, there are tests for the underlying PDataWriter and PDataReader implementations, so this may be a case of passing the wrong PDU size limits to them.

---

Update: see master...chore/test-association_store_pdata

[pgimeno4d]

Would it help if I pushed a branch with a baseline test which covers send_pdata and receive_pdata? I can probably make it available within the next few hours.

I guess it could help me see how to use them and see them integrated in a test. If not, I already have the findscu (for receive_pdata) and storescu (for send_pdata) examples. My plan is to stress the maximum PDUs in a manner similar to what #685 did: choose different maximums for the server and the client, and check that it can send and receive exactly that and not one more byte, and then switch sides, to guarantee that no combination is missed.

For what it's worth, there are tests for the underlying PDataWriter and PDataReader implementations, so this may be a case of passing the wrong PDU size limits to them.

Yeah, the problem affects send_pdata and receive_pdata only, as the implementations of these are passing values of max PDU to the PDataReader/Writer which are adequate from a client's standpoint only, even when called from a ServerAssociation, as the trait is common to both server and client and they use the default implementation from mod.rs instead of a specialized implementation for each.

In fact this patch just removes the default implementation and creates specializations with the correct values (mostly a copy/paste but carefully reviewed case by case because these are very error-prone).

[pgimeno4d]

After examining the code, I think I can test send_pdata from the receiver's side, but I don't see how to test receive_pdata, since it calls read_pdu with strict set to false:

dicom-rs/ul/src/association/pdata.rs

Line 291 in 898a091

match read_pdu(&mut buf, self.max_data_length, false)

Therefore no error is ever produced, and the only consequence is that internal buffers may need reallocation due to short initial capacity, which is not something visible from the outside.

So, working on a patch that tests just send_pdata.

[pgimeno4d]

Update: see master...chore/test-association_store_pdata

I've just noticed your update; unfortunately edits don't send notifications so I missed it earlier, sorry.

It's good for a full protocol test. For testing specific parts, like the exact size of the PDU, I'd rather abuse association_echo.rs, though, which already has the connection establishment infrastructure and is designed to deal with packets of arbitrary lengths, even if the packet contents are not standard. I've based my test on it.

Your test would be a nice addition as an "overall high-level test".

[pgimeno4d]

Added the test that demonstrates the problem in current master. If the patch is not applied, the test fails.