Fixing wire evolution - #357
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to improve wire evolution interoperability by making newer receivers automatically tolerate older senders that omit trailing extension bytes (and ignore extra trailing bytes from newer senders), so tests no longer need to manually pad payloads before decoding.
Changes:
- Updated multi-language code generators (Rust/Python/C#/C/C++) to internally zero-fill missing extension bytes and ignore unknown trailing bytes during deserialization/unpack.
- Simplified wire-evolution interop tests across languages by removing manual payload padding and relying on the generated deserializers.
- Added extension-field tolerant reads in variable-length deserializers (skip reads when the extension field is entirely absent).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_wire_evolution_interop.py | Removes Python-side manual zero-fill helper and relies on message deserialize() handling. |
| tests/rust/src/main.rs | Removes manual padding in Rust interop tests; relies on generated unpack() behavior. |
| tests/csharp/test_wire_evolution_interop.cs | Removes manual padding helper; uses message Deserialize(info) directly. |
| tests/cpp/test_wire_evolution_interop.cpp | Removes manual zero-fill decode helper; calls generated deserialize() directly. |
| tests/c/test_wire_evolution_interop.c | Removes manual padding buffers; calls generated *_deserialize() directly. |
| src/struct_frame/rust_gen.py | Changes Rust generator to pad/truncate buffers for wire evolution during unpack. |
| src/struct_frame/py_gen.py | Changes Python generator fixed deserialization to zero-fill short buffers; adds extension-field short-buffer tolerance for variable decode. |
| src/struct_frame/csharp_gen.py | Changes C# generator to pad/truncate for fixed messages; adds extension-field short-buffer tolerance for variable decode. |
| src/struct_frame/cpp_gen.py | Changes C++ generator fixed-message deserialize() to zero-fill short buffers; adds extension-field short-buffer tolerance for variable decode. |
| src/struct_frame/c_gen.py | Changes C generator fixed-message *_deserialize() to zero-fill short buffers; adds extension-field short-buffer tolerance for variable decode. |
Comment on lines
+1038
to
+1041
| result += ' let mut _padded = [0u8; Self::SIZE];\n' | ||
| result += ' let _n = buf.len().min(Self::SIZE);\n' | ||
| result += ' _padded[.._n].copy_from_slice(&buf[.._n]);\n' | ||
| result += ' let buf = &_padded[..];\n' |
Comment on lines
+736
to
+739
| result += f' size_t copy_len = buffer_size < MAX_SIZE ? buffer_size : MAX_SIZE;\n' | ||
| result += f' std::memset(this, 0, sizeof(*this));\n' | ||
| result += f' if (copy_len > 0) std::memcpy(this, buffer, copy_len);\n' | ||
| result += f' return copy_len;\n' |
Comment on lines
+831
to
+835
| result += f' /* Fixed-size message - zero-fill any bytes the sender omitted (wire evolution) */\n' | ||
| result += f' size_t copy_len = buffer_size < {defineName}_MAX_SIZE ? buffer_size : {defineName}_MAX_SIZE;\n' | ||
| result += f' memset(msg, 0, sizeof({structName}));\n' | ||
| result += f' if (copy_len > 0) memcpy(msg, buffer, copy_len);\n' | ||
| result += f' return copy_len;\n' |
Comment on lines
+436
to
+437
| result += ' if len(data) < cls.MAX_SIZE:\n' | ||
| result += ' data = data + b"\\x00" * (cls.MAX_SIZE - len(data))\n' |
Comment on lines
+666
to
+672
| result += ' if (data.Length != MaxSize)\n' | ||
| result += ' {\n' | ||
| result += ' byte[] _padded = new byte[MaxSize];\n' | ||
| result += ' int _n = Math.Min(data.Length, MaxSize);\n' | ||
| result += ' data.Slice(0, _n).CopyTo(_padded);\n' | ||
| result += ' data = _padded;\n' | ||
| result += ' }\n' |
Comment on lines
+831
to
+835
| result += f' /* Fixed-size message - zero-fill any bytes the sender omitted (wire evolution) */\n' | ||
| result += f' size_t copy_len = buffer_size < {defineName}_MAX_SIZE ? buffer_size : {defineName}_MAX_SIZE;\n' | ||
| result += f' memset(msg, 0, sizeof({structName}));\n' | ||
| result += f' if (copy_len > 0) memcpy(msg, buffer, copy_len);\n' | ||
| result += f' return copy_len;\n' |
Comment on lines
+736
to
+739
| result += f' size_t copy_len = buffer_size < MAX_SIZE ? buffer_size : MAX_SIZE;\n' | ||
| result += f' std::memset(this, 0, sizeof(*this));\n' | ||
| result += f' if (copy_len > 0) std::memcpy(this, buffer, copy_len);\n' | ||
| result += f' return copy_len;\n' |
Comment on lines
+666
to
+672
| result += ' if (data.Length != MaxSize)\n' | ||
| result += ' {\n' | ||
| result += ' byte[] _padded = new byte[MaxSize];\n' | ||
| result += ' int _n = Math.Min(data.Length, MaxSize);\n' | ||
| result += ' data.Slice(0, _n).CopyTo(_padded);\n' | ||
| result += ' data = _padded;\n' | ||
| result += ' }\n' |
Comment on lines
+1036
to
+1045
| # Fixed message: simple unpack. | ||
| # Wire evolution: zero-fill any bytes an older sender omitted (a shorter | ||
| # buffer); ignore any trailing bytes a newer sender appended (a longer | ||
| # buffer). Padding into a fixed Self::SIZE-length array up front means | ||
| # every field read below always has the bytes it expects, so callers | ||
| # never need to pad or truncate the buffer themselves. | ||
| result += ' let mut _padded = [0u8; Self::SIZE];\n' | ||
| result += ' let _n = buf.len().min(Self::SIZE);\n' | ||
| result += ' _padded[.._n].copy_from_slice(&buf[.._n]);\n' | ||
| result += ' let buf = &_padded[..];\n' |
Comment on lines
+436
to
+437
| result += ' if len(data) < cls.MAX_SIZE:\n' | ||
| result += ' data = data + b"\\x00" * (cls.MAX_SIZE - len(data))\n' |
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
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.
No description provided.