Azure / Azure/azure-sdk-for-python
[EventHub][ServiceBus] pyAMQP encoder does not omit trailing null fields (AMQP 1.0 section 1.4)
- Dominant language
- Python
- Stars
- 5.6k
- Forks
- 3.4k
- Avg merge
- 1d 21h
- Merged PRs (30d)
- 193
Description
## Summary
The vendored pyAMQP encoder always emits every field of a described composite, including trailing null fields, instead of omitting them as permitted by AMQP 1.0 section 1.4. A standing TODO marks this for the message Header and Properties sections in `encode_payload`, and the same applies to frame-level performatives via `describe_performative`. Implementing trailing-null omission on the sender side would produce the compact encoding the decoder already accepts and would make an encode/decode roundtrip test of the omission path possible.
## Motivation
AMQP 1.0 section 1.4 ("Composite Type Representation") states that when the trailing elements of a list representation are null, they MAY be omitted. The encoder does not take this allowance: `describe_performative` appends an explicit `{TYPE: null, VALUE: None}` for every `None`-valued field (`sdk/eventhub/azure-eventhub/azure/eventhub/_pyamqp/_encode.py:883-884`), and `encode_payload` carries a TODO noting the Header/Properties encoding "can be optimized to 1. not encoding trailing None fields" with three candidate fixes sketched inline (`_encode.py:922-933`).
Two consequences follow. First, every frame with unset trailing fields (common for Open, Attach, Transfer, and message Header/Properties) is larger on the wire than it needs to be. Second, because the encoder never produces a short list, the receiver-side tolerance added in Azure/azure-sdk-for-python#47660 cannot be exercised by a real encode->decode roundtrip; that fix is verified only with hand-built byte frames. Implementing the sender side closes both gaps and makes the SDK produce the same compact encoding it now decodes.
This is a wire-efficiency and test-completeness enhancement, not a correctness defect: the encoder's full-list output is spec-legal, and the decoder now tolerates both forms.
## Proposal
Trim the contiguous run of trailing `None` fields before encoding, in both the performative path (`describe_performative`, `_encode.py:878-908`) and the message Header/Properties path (`encode_payload`, `_encode.py:919-934`, where the TODO already outlines the approach). Preserve all leading and interior fields; only drop the trailing null run so positional meaning is retained. Apply the change identically to both vendored copies (`sdk/eventhub/azure-eventhub/.../_pyamqp/_encode.py` and `sdk/servicebus/azure-servicebus/.../_pyamqp/_encode.py`), which are kept byte-identical.
Add a roundtrip regression test asserting that a performative (and a message with trailing-null Header/Properties) encodes to a short list and decodes back equal, with omitted fields reading as `None`. This becomes feasible only once the encoder omits trailing nulls.
## Validation
- `pytest sdk/eventhub/azure-eventhub/tests/pyamqp_tests/unittest/test_encode.py` and `test_decode.py`.
- `pytest sdk/servicebus/azure-servicebus/tests/unittests/test_pyamqp_decode.py`.
- New roundtrip test covering a performative with trailing null fields and a message with trailing-null Header/Properties.
- The eventhub and servicebus `_pyamqp/_encode.py` copies remain byte-identical.
Contributor guide
Assessment
This issue has not been assessed yet.