Repository navigation
Fix response framing edge cases - #98
Merged
Merged
Conversation
djc
approved these changes
Oct 5, 2026
djc
left a comment
djc
left a comment
Owner
There was a problem hiding this comment.
LGTM modulo a few nits, please rebase on current main.
Owner
|
Do you want/need a release after this or are you using it from git anyway? |
The registry sets the response size, and the client allocated with no limit. A size above max_read_buf now causes an error.
The buffer could shrink below the read offset, which would cause a panic.
A failed connect kept the old stream but cleared the in-flight requests, so the next command could get the response of the earlier request.
The client polled again with a zero-length buffer, which could result in an EOF or an infinite wait. A frame that arrived in one read kept the zero padding of the 256-byte read buffer. It is now truncated.
The 4-byte header could be split across reads. The length came only from the last read. That gave a wrong length, or a panic when that read had fewer than 4 bytes.
The clTRID went into the request XML without escaping. Markup in the ID could add elements to the EPP command.
Contributor
Author
|
We do not need a release as long as some other PRs here are not merged. I hope I can come back to them soon to finish the work and get them into a state you are happy with. Due to the nature of the issues fixed in this PR I think it would be valuable to cut a release, though. |
Owner
|
Published 0.5.0. |
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
2 participants
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.
This fixes a couple of edge cases I found.
It is stacked on #84, so you can either close that one and merge this (I cherry-picked with correct attribution) or I will rebase once the other one is merged.
set_max_read_buf. We might want to find something more ergonomic, but I would like to land this sooner rather than later.This is a breaking change: it adds the
Error::ResponseTooLargevariant.I made sure the tests fail without the applied fixes.