From 30d1837421681fe7c57c249b003030c2769f2995 Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Wed, 24 Jun 2026 21:38:48 -0500 Subject: [PATCH] fix(pyamqp): pad decoded performatives to full field count AMQP 1.0 section 1.4 lets a sender omit trailing null fields, so an incoming performative described-list can be shorter than the full field count. The pyAMQP decoder built the field list from the wire count, and consumers then accessed fixed indices (frame[9], OpenFrame(*frame)), raising IndexError/TypeError on a short list. decode_frame now pads the decoded list up to the performative's full field count, so omitted trailing fields read back as None. Applied to both the Event Hubs and Service Bus vendored copies, with regression tests and changelog entries. --- sdk/eventhub/azure-eventhub/CHANGELOG.md | 6 ++ .../azure/eventhub/_pyamqp/_decode.py | 36 +++++++++++ .../azure-eventhub/azure/eventhub/_version.py | 2 +- .../pyamqp_tests/unittest/test_decode.py | 62 ++++++++++++++++++- sdk/servicebus/azure-servicebus/CHANGELOG.md | 1 + .../azure/servicebus/_pyamqp/_decode.py | 36 +++++++++++ .../tests/unittests/test_pyamqp_decode.py | 62 +++++++++++++++++++ 7 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 sdk/servicebus/azure-servicebus/tests/unittests/test_pyamqp_decode.py diff --git a/sdk/eventhub/azure-eventhub/CHANGELOG.md b/sdk/eventhub/azure-eventhub/CHANGELOG.md index f287cb0b8fde..c823a834bdc5 100644 --- a/sdk/eventhub/azure-eventhub/CHANGELOG.md +++ b/sdk/eventhub/azure-eventhub/CHANGELOG.md @@ -1,5 +1,11 @@ # Release History +## 5.15.2 (Unreleased) + +### Bugs Fixed + +- Fixed a bug in the pyAMQP transport where decoding an incoming performative whose trailing null fields were omitted by the sender (permitted by AMQP 1.0 section 1.4) raised `IndexError`/`TypeError`. The decoded field list is now padded to the performative's full field count so omitted trailing fields read back as `None`. + ## 5.15.1 (2025-11-11) ### Bugs Fixed diff --git a/sdk/eventhub/azure-eventhub/azure/eventhub/_pyamqp/_decode.py b/sdk/eventhub/azure-eventhub/azure/eventhub/_pyamqp/_decode.py index afe58f311a0c..3a51324452ac 100644 --- a/sdk/eventhub/azure-eventhub/azure/eventhub/_pyamqp/_decode.py +++ b/sdk/eventhub/azure-eventhub/azure/eventhub/_pyamqp/_decode.py @@ -23,6 +23,7 @@ from .message import Message, Header, Properties +from . import performatives if TYPE_CHECKING: from .message import MessageDict @@ -327,6 +328,36 @@ def decode_payload(buffer: memoryview) -> Message: return Message(**message_properties) +# Number of fields encoded on the wire for each performative, keyed by its +# frame-type code. The AMQP 1.0 spec (section 1.4) lets a sender omit trailing +# null fields, so an incoming performative list can be shorter than the full +# field count. Padding the decoded list up to this count keeps positional +# (frame[N]) access and namedtuple unpacking safe; the missing trailing fields +# read back as None, which is the spec-defined meaning of an omitted field. +# The transfer performative (code 20) carries a trailing payload that is not a +# wire field, so its _definition uses a None sentinel for that slot, which is +# excluded from the count here. +_PERFORMATIVE_FIELD_COUNT: Dict[int, int] = { + performative._code: sum(1 for field in performative._definition if field is not None) + for performative in ( + performatives.OpenFrame, + performatives.BeginFrame, + performatives.AttachFrame, + performatives.FlowFrame, + performatives.TransferFrame, + performatives.DispositionFrame, + performatives.DetachFrame, + performatives.EndFrame, + performatives.CloseFrame, + performatives.SASLMechanism, + performatives.SASLInit, + performatives.SASLChallenge, + performatives.SASLResponse, + performatives.SASLOutcome, + ) +} + + def decode_frame(data: memoryview) -> Tuple[int, List[Any]]: # Ignore the first two bytes, they will always be the constructors for # described type then ulong. @@ -343,6 +374,11 @@ def decode_frame(data: memoryview) -> Tuple[int, List[Any]]: fields: List[Optional[memoryview]] = [None] * count for i in range(count): buffer, fields[i] = _DECODE_BY_CONSTRUCTOR[buffer[0]](buffer[1:]) + # A sender may omit trailing null fields, so pad the decoded list up to the + # performative's full field count before any positional access or unpacking. + full_field_count = _PERFORMATIVE_FIELD_COUNT.get(frame_type) + if full_field_count is not None and count < full_field_count: + fields.extend([None] * (full_field_count - count)) if frame_type == 20: fields.append(buffer) return frame_type, fields diff --git a/sdk/eventhub/azure-eventhub/azure/eventhub/_version.py b/sdk/eventhub/azure-eventhub/azure/eventhub/_version.py index 5bb5c30266da..5d660bbf8ea3 100644 --- a/sdk/eventhub/azure-eventhub/azure/eventhub/_version.py +++ b/sdk/eventhub/azure-eventhub/azure/eventhub/_version.py @@ -3,4 +3,4 @@ # Licensed under the MIT License. # ------------------------------------ -VERSION = "5.15.1" +VERSION = "5.15.2" diff --git a/sdk/eventhub/azure-eventhub/tests/pyamqp_tests/unittest/test_decode.py b/sdk/eventhub/azure-eventhub/tests/pyamqp_tests/unittest/test_decode.py index 05b77014c20c..2cbf9e4ea782 100644 --- a/sdk/eventhub/azure-eventhub/tests/pyamqp_tests/unittest/test_decode.py +++ b/sdk/eventhub/azure-eventhub/tests/pyamqp_tests/unittest/test_decode.py @@ -1,5 +1,6 @@ import pytest -from azure.eventhub._pyamqp._decode import _decode_decimal128 +from azure.eventhub._pyamqp._decode import _decode_decimal128, decode_frame, _PERFORMATIVE_FIELD_COUNT +from azure.eventhub._pyamqp import performatives from decimal import Decimal @@ -18,3 +19,62 @@ def test_decimal_decode(value, expected): assert output[1] == expected +def _list8_frame(code, count, encoded_fields, payload=b""): + # Build a described performative frame using a list8 (0xc0) body: + # described-type ctor (0x00), ulong ctor (0x53), descriptor code, list8 (0xc0), + # size, count, then the encoded field bytes and any trailing payload. + header = bytes([0x00, 0x53, code, 0xC0, len(encoded_fields) + 1, count]) + return memoryview(header + encoded_fields + payload) + + +# A sender may omit trailing null fields (AMQP 1.0 section 1.4), so an incoming +# performative list can be shorter than the full field count. The decoder must +# pad it back to the full count so positional access and namedtuple unpacking +# stay safe and omitted fields read back as None. +def test_short_open_is_padded_to_full_field_count(): + # Open with only container_id set ("x"), 1 field on the wire out of 10. + frame = _list8_frame(performatives.OpenFrame._code, 1, bytes([0xA1, 0x01, 0x78])) + frame_type, fields = decode_frame(frame) + assert frame_type == performatives.OpenFrame._code + assert len(fields) == 10 + # Unpacking and fixed-index access must not raise on the omitted fields. + open_frame = performatives.OpenFrame(*fields) + assert open_frame.container_id == b"x" + assert open_frame.properties is None + assert fields[9] is None + + +def test_short_transfer_pads_fields_and_preserves_payload(): + # Transfer with only handle (0) set, plus a message payload. The payload is + # appended after the fields and must survive the padding. + frame = _list8_frame(performatives.TransferFrame._code, 1, bytes([0x52, 0x00]), payload=b"\xde\xad") + frame_type, fields = decode_frame(frame) + assert frame_type == performatives.TransferFrame._code + # 11 wire fields padded out, then the trailing payload appended (12 total). + assert len(fields) == 12 + transfer = performatives.TransferFrame(*fields) + assert transfer.handle == 0 + assert transfer.batchable is None + assert bytes(transfer.payload) == b"\xde\xad" + + +@pytest.mark.parametrize( + "frame_cls,expected_count", + [ + (performatives.OpenFrame, 10), + (performatives.BeginFrame, 8), + (performatives.AttachFrame, 14), + (performatives.FlowFrame, 11), + (performatives.TransferFrame, 11), + (performatives.DispositionFrame, 6), + (performatives.DetachFrame, 3), + (performatives.EndFrame, 1), + (performatives.CloseFrame, 1), + ], +) +def test_performative_field_count_matches_spec(frame_cls, expected_count): + # The padding target is the number of wire fields defined for each + # performative (the trailing transfer payload slot is excluded). + assert _PERFORMATIVE_FIELD_COUNT[frame_cls._code] == expected_count + + diff --git a/sdk/servicebus/azure-servicebus/CHANGELOG.md b/sdk/servicebus/azure-servicebus/CHANGELOG.md index 3b9e9d0ac980..cbdde7c99221 100644 --- a/sdk/servicebus/azure-servicebus/CHANGELOG.md +++ b/sdk/servicebus/azure-servicebus/CHANGELOG.md @@ -5,6 +5,7 @@ ### Bugs Fixed - Read `com.microsoft:max-message-batch-size` vendor property from the AMQP sender link to correctly limit batch size on Premium large-message entities, where `max-message-size` can be up to 100 MB but the batch limit is 1 MB. +- Fixed a bug in the pyAMQP transport where decoding an incoming performative whose trailing null fields were omitted by the sender (permitted by AMQP 1.0 section 1.4) raised `IndexError`/`TypeError`. The decoded field list is now padded to the performative's full field count so omitted trailing fields read back as `None`. ## 7.14.3 (2025-11-11) diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp/_decode.py b/sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp/_decode.py index afe58f311a0c..3a51324452ac 100644 --- a/sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp/_decode.py +++ b/sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp/_decode.py @@ -23,6 +23,7 @@ from .message import Message, Header, Properties +from . import performatives if TYPE_CHECKING: from .message import MessageDict @@ -327,6 +328,36 @@ def decode_payload(buffer: memoryview) -> Message: return Message(**message_properties) +# Number of fields encoded on the wire for each performative, keyed by its +# frame-type code. The AMQP 1.0 spec (section 1.4) lets a sender omit trailing +# null fields, so an incoming performative list can be shorter than the full +# field count. Padding the decoded list up to this count keeps positional +# (frame[N]) access and namedtuple unpacking safe; the missing trailing fields +# read back as None, which is the spec-defined meaning of an omitted field. +# The transfer performative (code 20) carries a trailing payload that is not a +# wire field, so its _definition uses a None sentinel for that slot, which is +# excluded from the count here. +_PERFORMATIVE_FIELD_COUNT: Dict[int, int] = { + performative._code: sum(1 for field in performative._definition if field is not None) + for performative in ( + performatives.OpenFrame, + performatives.BeginFrame, + performatives.AttachFrame, + performatives.FlowFrame, + performatives.TransferFrame, + performatives.DispositionFrame, + performatives.DetachFrame, + performatives.EndFrame, + performatives.CloseFrame, + performatives.SASLMechanism, + performatives.SASLInit, + performatives.SASLChallenge, + performatives.SASLResponse, + performatives.SASLOutcome, + ) +} + + def decode_frame(data: memoryview) -> Tuple[int, List[Any]]: # Ignore the first two bytes, they will always be the constructors for # described type then ulong. @@ -343,6 +374,11 @@ def decode_frame(data: memoryview) -> Tuple[int, List[Any]]: fields: List[Optional[memoryview]] = [None] * count for i in range(count): buffer, fields[i] = _DECODE_BY_CONSTRUCTOR[buffer[0]](buffer[1:]) + # A sender may omit trailing null fields, so pad the decoded list up to the + # performative's full field count before any positional access or unpacking. + full_field_count = _PERFORMATIVE_FIELD_COUNT.get(frame_type) + if full_field_count is not None and count < full_field_count: + fields.extend([None] * (full_field_count - count)) if frame_type == 20: fields.append(buffer) return frame_type, fields diff --git a/sdk/servicebus/azure-servicebus/tests/unittests/test_pyamqp_decode.py b/sdk/servicebus/azure-servicebus/tests/unittests/test_pyamqp_decode.py new file mode 100644 index 000000000000..326449a9a164 --- /dev/null +++ b/sdk/servicebus/azure-servicebus/tests/unittests/test_pyamqp_decode.py @@ -0,0 +1,62 @@ +import pytest +from azure.servicebus._pyamqp._decode import decode_frame, _PERFORMATIVE_FIELD_COUNT +from azure.servicebus._pyamqp import performatives + + +def _list8_frame(code, count, encoded_fields, payload=b""): + # Build a described performative frame using a list8 (0xc0) body: + # described-type ctor (0x00), ulong ctor (0x53), descriptor code, list8 (0xc0), + # size, count, then the encoded field bytes and any trailing payload. + header = bytes([0x00, 0x53, code, 0xC0, len(encoded_fields) + 1, count]) + return memoryview(header + encoded_fields + payload) + + +# A sender may omit trailing null fields (AMQP 1.0 section 1.4), so an incoming +# performative list can be shorter than the full field count. The decoder must +# pad it back to the full count so positional access and namedtuple unpacking +# stay safe and omitted fields read back as None. +def test_short_open_is_padded_to_full_field_count(): + # Open with only container_id set ("x"), 1 field on the wire out of 10. + frame = _list8_frame(performatives.OpenFrame._code, 1, bytes([0xA1, 0x01, 0x78])) + frame_type, fields = decode_frame(frame) + assert frame_type == performatives.OpenFrame._code + assert len(fields) == 10 + # Unpacking and fixed-index access must not raise on the omitted fields. + open_frame = performatives.OpenFrame(*fields) + assert open_frame.container_id == b"x" + assert open_frame.properties is None + assert fields[9] is None + + +def test_short_transfer_pads_fields_and_preserves_payload(): + # Transfer with only handle (0) set, plus a message payload. The payload is + # appended after the fields and must survive the padding. + frame = _list8_frame(performatives.TransferFrame._code, 1, bytes([0x52, 0x00]), payload=b"\xde\xad") + frame_type, fields = decode_frame(frame) + assert frame_type == performatives.TransferFrame._code + # 11 wire fields padded out, then the trailing payload appended (12 total). + assert len(fields) == 12 + transfer = performatives.TransferFrame(*fields) + assert transfer.handle == 0 + assert transfer.batchable is None + assert bytes(transfer.payload) == b"\xde\xad" + + +@pytest.mark.parametrize( + "frame_cls,expected_count", + [ + (performatives.OpenFrame, 10), + (performatives.BeginFrame, 8), + (performatives.AttachFrame, 14), + (performatives.FlowFrame, 11), + (performatives.TransferFrame, 11), + (performatives.DispositionFrame, 6), + (performatives.DetachFrame, 3), + (performatives.EndFrame, 1), + (performatives.CloseFrame, 1), + ], +) +def test_performative_field_count_matches_spec(frame_cls, expected_count): + # The padding target is the number of wire fields defined for each + # performative (the trailing transfer payload slot is excluded). + assert _PERFORMATIVE_FIELD_COUNT[frame_cls._code] == expected_count