Fix ReactorSender treating absent max-message-size on ATTACH as a zero-byte limit#49848
Open
thomhurst wants to merge 2 commits into
Open
Fix ReactorSender treating absent max-message-size on ATTACH as a zero-byte limit#49848thomhurst wants to merge 2 commits into
thomhurst wants to merge 2 commits into
Conversation
Per AMQP 1.0 section 2.7.3, an absent or zero max-message-size on the remote ATTACH frame means the peer imposes no message size limit. The sender previously left linkSize at 0 in that case, so every send failed with "Size of the payload exceeded maximum message size: 0 kb" against brokers that do not advertise the field (e.g. ActiveMQ Artemis). Fall back to ClientConstants.MAX_MESSAGE_LENGTH_BYTES (256 KB) when the field is absent or zero, and clamp advertised values larger than Integer.MAX_VALUE instead of truncating them to negative ints. Fixes Azure#49847
|
Azure Pipelines: Successfully started running 1 pipeline(s). 31 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Extract the unsigned-long-to-int saturation into a clampToInt helper and use it in both places that read the remote max-message-size, so a peer advertising a value above Integer.MAX_VALUE cannot truncate the batch size fallback to a negative int either. getMaxBatchSize still returns 0 when the field is absent, preserving its documented contract. Also fixes the vendor-property comment that claimed intValue() matched getLinkSize.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #49847.
ReactorSender.getLinkSize()reads the remotemax-message-sizefrom the broker's ATTACH frame. When the peer omits the field,linkSizewas left at its initial value of0, so every send failed immediately with:Per the AMQP 1.0 spec (OASIS AMQP v1.0, section 2.7.3 "Attach", field
max-message-size):The real Azure Service Bus / Event Hubs service always advertises the field, so this never fires against Azure itself — but spec-compliant brokers that omit it (e.g. ActiveMQ Artemis, which never sets it on its receiving links) are completely unusable for sending: the client rejects even a 1-byte payload before any bytes reach the wire.
Changes
getLinkSize()now treats an absent or zero remotemax-message-sizeas "no limit imposed by the peer" and falls back to the pre-existingClientConstants.MAX_MESSAGE_LENGTH_BYTES(256 KB) so a bounded encode buffer can still be allocated. The previous warning log is replaced with an informational message stating the default being applied.Integer.MAX_VALUE(e.g. peers advertising the maximum unsigned long to signal an effectively unlimited link) are now clamped toInteger.MAX_VALUEinstead of being truncated byUnsignedLong.intValue()into a negativelinkSize, which would have produced aNegativeArraySizeExceptionon the send path.clampToInthelper used by bothgetLinkSize()and themax-message-sizefallback ingetMaxBatchSize(), so the two call sites cannot diverge.getMaxBatchSize()s documented contract is preserved: it still returns 0 when both the vendor property andmax-message-sizeare absent, with downstream clients substituting their own default.Tests
Added to
ReactorSenderTest(all 622 module tests pass; checkstyle and spotbugs clean):testLinkSizeUsesDefaultWhenRemoteMaxMessageSizeAbsent— absent field falls back to 256 KB and the result is cached.testLinkSizeUsesDefaultWhenRemoteMaxMessageSizeZero— explicit zero is treated as "no limit" per spec.testLinkSizeClampsRemoteMaxMessageSizeAboveIntegerMax— 2^64-1 clamps toInteger.MAX_VALUE.testSendWhenRemoteMaxMessageSizeAbsent— a message sends successfully when the field is absent.All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines