Refactorings to the PAM conversation handling #961
Merged
Conversation
This file contains 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
|
I love the fact that this 2fdc5a2 fixes the second higgs-bugson that was caught by the |
…ly sized one Doing so would cause UB. None of the existing Converser impls do this and it is also hard to accidentally do, but it can't hurt to be a bit more strict either.
A Converser doesn't need to distinguish between a single conversation with multiple messages and multiple conversations with a single message each.
This simplifies the code and makes it easier to directly return the response from Converser::handle_message in the future. This will leak some memory when there are multiple messages in a single conversation and handling one of the messages returns an error. This shouldn't matter for sudo-rs however as we would exit shortly after the error anyway.
Rather than storing it in PamMessage.
And pass it's fields directly to Converser::handle_message instead.
And rename SequentialConverser to Converser to reduce the amount of changes in the rest of the code.
Miri complains.
squell
approved these changes
Jan 21, 2025
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.
This removes some unnecessary abstraction as well as fixes what is technically a soundness issue in the api. None of the existing users of the PAM module currently hit this soundness issue.