fix: Fixes regression in backend logic for argon2 and bcrypt#29
Open
Daverball wants to merge 1 commit into
Open
fix: Fixes regression in backend logic for argon2 and bcrypt#29Daverball wants to merge 1 commit into
argon2 and bcrypt#29Daverball wants to merge 1 commit into
Conversation
For derived backends like `bcrypt_sha256` we were getting incorrect results for the first call to `_calc_checksum`, since we were starting over the inheritance chain from the top when arriving at `_NoBackend`, instead of from the spot where `_NoBackend` previously resided in the chain.
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
1 participant
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.
For derived backends like
bcrypt_sha256we were getting incorrect results for the first call to_calc_checksum, since we were starting over the inheritance chain from the top when arriving at_NoBackend, instead of from the spot where_NoBackendpreviously resided in the chain.Fixes #27
I would've liked to add test cases, to ensure this doesn't regress again in the future, but there's unfortunately currently no reliable way to reset a handler back to having no backends, so the only way we could feasible test this, without changing the code and potentially introduce more regressions, is with a subprocess, so we can compare the output from a fresh instance of the handler against a fully initialized version. But that would only really work for the default backend, so there's a whole bunch of plumbing in the test cases that would need to change in order to accommodate this. So the cure currently seems worse than the disease, but maybe someone can think of a somewhat reliable way to test this, without causing a whole bunch of extra complication in the test case machinery.