Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing serial number arithmetic #311

Open
ansd opened this issue Sep 14, 2023 · 1 comment
Open

Missing serial number arithmetic #311

ansd opened this issue Sep 14, 2023 · 1 comment

Comments

@ansd
Copy link

ansd commented Sep 14, 2023

In AMQP 1.0, a delivery-number is a 32-bit RFC-1982 serial number.
The DISPOSITION frame includes fields first and last.
first is the lower bound of delivery-ids to be (n)acked.
last is the upper bound of delivery-ids to be (n)acked.

To provide an example:
Serial number arithmetic defines sequence number 1 to be greater than sequence number 4294967294 (= 2^32-2) .
Therefore my understanding is that a DISPOSITION frame with first = 4294967294 and last = 1 is allowed and refers to delivery-numbers 4294967294, 4294967295, 0, and 1.

Is my understanding correct?

If yes, aren't

for deliveryID := start; deliveryID <= end; deliveryID++ {

and
for deliveryID := start; deliveryID <= end; deliveryID++ {

flawed? These lines need to perform serial number arithmetic instead of integer arithmetic?

@ansd
Copy link
Author

ansd commented Sep 14, 2023

In other words, there are bugs when acking / nacking messages with this client when the delivery ID wraps around.

The .NET client does the right thing in https://github.com/Azure/amqpnetlite/blob/772d0d3b6bbc43cf64e0ccd25911317f011927d7/src/Session.cs#L650 because they overwrite the comparison operators in https://github.com/Azure/amqpnetlite/blob/772d0d3b6bbc43cf64e0ccd25911317f011927d7/src/SequenceNumber.cs#L94-L98 to perform serial number arithmetic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant