Repository navigation
Honor negotiated MMS server limits before merge - #67
Conversation
05b64e6
into
agent/complete-server-consolidation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dbb7d083f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const auto selected_nesting_level = std::min( | ||
| initiate.data_structure_nesting_level, | ||
| kServerMaximumNestingLevel); |
There was a problem hiding this comment.
When a client proposes a nesting level below 5, this advertises the smaller value in the InitiateResponse, but the selected limit is never stored or enforced afterward. In particular, static_dispatcher.cpp accepts constructed read values without traversing their depth and passes write values directly to callbacks, so an association negotiated at level 1 can still exchange deeper arrays or structures, violating the newly advertised limit and potentially breaking clients that rely on it.
Useful? React with 👍 / 👎.
| std::array<std::uint8_t, 128U> aare{}; | ||
| const auto aare_result = encode_accept_aare_into(mms_initiate_response, aare); |
There was a problem hiding this comment.
When the new overload is given a valid but larger MMS InitiateResponse, accept_aare_size() and the surrounding session/CPA checks can accept an AARE larger than 128 bytes, yet encoding always uses this fixed staging buffer. For example, valid initiate detail bit strings can make the AARE exceed 128 bytes while the final CPA still fits its 255-byte session field; the method then returns value_out_of_range even when destination has the reported required capacity.
Useful? React with 👍 / 👎.
1 participant
Fixes final PR #51 review findings
Read.specificationWithResultbefore object reads because this bounded profile does not encode the requested specification;Evidence