Prevent the use of empty user, service, or target names #87
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
Fixes hwchen#86. The fix, since we can't fail to create entries without modifying the API, is to make sure the created entries with empty strings can't be used to set or get passwords. We do this by introducing an "always invalid" platform credential that we create for these entries. Updates docs to clarify that the service and user names must be non-empty.
This reflects the fix of hwchen#86.
Also test on Windows.
Based on review discussion, this change should really be considered a minor version change rather than a patch release. The docs have been updated to note the change in behavior and the workaround available to anyone who was relying on the prior behavior. In addition, a warning has been issued that the next major release may change the `Entry` creation API to be fallible.
|
Unless someone objects, I'm going to merge this weekend and release a new crate. @hwchen any objection? |
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 PR fixes #86.
Since we can't alter the API to prevent the creation of credentials that have empty strings for target, service, or user names, we instead create "always invalid" credentials that, while well-formed, do no support calling get_password or set_password successfully. (They always return a
NoEntryerror.)The docs (both top-level and code-level) have been updated, and tests added to verify this handling.