search

Azure / go-amqp

AMQP 1.0 client library for Go.
https://github.com/Azure/go-amqp
MIT License
110 stars 58 forks source link

Missing serial number arithmetic #311

Open ansd opened 1 year ago

ansd commented 1 year ago

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 https://github.com/Azure/go-amqp/blob/1c1e489960c8b4cb448bd8a0a3d11adaa13ff77f/session.go#L418 and https://github.com/Azure/go-amqp/blob/1c1e489960c8b4cb448bd8a0a3d11adaa13ff77f/session.go#L708 flawed? These lines need to perform serial number arithmetic instead of integer arithmetic?

ansd commented 1 year ago

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.