Fix a pair of related bugs in Server/ClientAssociation::receive#686
Fix a pair of related bugs in Server/ClientAssociation::receive#686pgimeno4d wants to merge 1 commit intoEnet4:masterfrom
Conversation
The maximum PDU's length was being checked against the wrong value in the sync version of the client, and in the async version of the server.
|
I think this should be fixed in #679 see here I changed the Association structs to store a single Edit: Link doesn't show up in diff view very well, linking the code on my branch here and here for client, similar for server as well |
|
I agree that #679 fixes it. I'm not sure to fully agree with the conflation of our and the peer's max_pdu_length, but it's not wrong either, so no objections. I'm not sure whether to keep this open. I guess it depends on whether there will be a 0.8.3. If not, please feel free to close. |
The next version planned is 0.9.0, as we've already started merging breaking changes. On the other hand, I have yet to to make the complete assessment of which PRs will enter. Only those on the 0.9 milestone are practically guaranteed to be included, but I might add more in the meantime. As for the release proper, I understand that it's been a while since the last one, so hopefully I can drop it before the end of the month. Let's see how things go. :) |
|
I will consider #689 instead, which fixes this bug and improves a few other things around max PDU length negotiation. Thank you for detecting the bug and working on it nevertheless! |
The problem can be quickly spotted by comparing the sync and async versions of ClientAssociation::receive() in the source:
Sync version:
dicom-rs/ul/src/association/client.rs
Line 988 in 1f16e9b
Async version:
dicom-rs/ul/src/association/client.rs
Line 1498 in 1f16e9b
One says "requestor" and the other "acceptor"? That can't be right; both sync and async should use the same criteria. So which one is right? It's checking what "we" are able to receive, and since the client is the one that makes requests, and thus the requestor, requestor is right and acceptor is wrong. That is, the sync version is wrong.
The exact same problem happens in ServerAssociation::receive:
Sync version:
dicom-rs/ul/src/association/server.rs
Line 741 in 1f16e9b
Async version:
dicom-rs/ul/src/association/server.rs
Line 1158 in 1f16e9b
The server is the acceptor, so in this case the async version is wrong.