Backport #4553 to 0.5.x

[pepone]

Backport of #4553 — reject trailing bytes in unary Protobuf payloads.

Summary

A unary Protobuf payload silently accepting bytes after the framed message is a smuggling primitive (middleware vs. dispatcher parsing divergence). DecodeProtobufMessageAsync now reads once more after extracting the message and rejects any remaining bytes (and any non-completed read) with InvalidDataException.

Test plan

• dotnet test tests/IceRpc.Protobuf.Tests — 53/53 pass (51 + 2 new tests).

Backport notes

Cherry-pick of 80ff266 with one conflict resolution in tests/IceRpc.Protobuf.Tests/PipeReaderExtensionsTests.cs: the file was modified by the already-merged #4624 (backport of #4593, envelope validation), which introduced a richer WriteLengthPrefixedMessage helper. Kept all of #4593's tests + the richer helper from 0.5.x, and appended the two new #4553 tests (Decode_message_throws_invalid_data_exception_when_payload_has_trailing_bytes, Decode_message_throws_invalid_data_exception_when_payload_has_concatenated_messages).

Now unblocked since #4624 merged.

[Copilot]

Pull request overview

Backports #4553 to the 0.5.x branch: rejects trailing bytes in unary Protobuf payloads to close a smuggling/parser-divergence vector. After parsing the single expected length-prefixed message, DecodeProtobufMessageAsync performs an additional ReadAsync and throws InvalidDataException if any bytes remain (and InvalidOperationException if the read was canceled). Two NUnit tests cover trailing-junk and concatenated-message payloads, reusing the WriteLengthPrefixedMessage helper introduced by #4624.

Changes:

• Add a final ReadAsync in DecodeProtobufMessageAsync to enforce one-message-only semantics on unary payloads.
• Add two unit tests for trailing-bytes and concatenated-messages cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/IceRpc.Protobuf/Internal/PipeReaderExtensions.cs	Adds post-decode read/check ensuring no trailing bytes remain in unary payloads.
tests/IceRpc.Protobuf.Tests/PipeReaderExtensionsTests.cs	Adds two NUnit tests verifying trailing-bytes and concatenated-messages produce InvalidDataException.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[bernardnormier]

Do we have a separate backport for this Debug.Assert? It could be mixed-in the AsyncStream work.

[pepone]

Good catch — that's actually the only line in main that meaningfully closes this surface in Release builds (the existing Debug.Assert is a no-op there, so a crafted empty payload currently slips through and returns null to the unary caller on 0.5.x).

The replacement landed in #4590 as part of the AsyncStream rewrite, but the assert → ?? throw new InvalidDataException(...) swap is logically separable. I've folded it into this PR (force-pushed):

• DecodeProtobufMessageAsync<T> now uses main's ?? throw new InvalidDataException("The payload is empty; expected a Protobuf message."), with where T : class, IMessage<T>.
• The class constraint cascades to the public InvokerExtensions / IncomingRequestExtensions entry points (8 sites in each file). Whole solution builds clean; Protobuf-generated messages are always classes so no real caller is affected.
• Added Decode_message_throws_invalid_data_exception_on_empty_payload to lock in the new behaviour (verified passing in both Debug and Release configs).