consumer_key: trim an explicit /* or / suffix in add_recursive_rules - #150
Open
Occansey wants to merge 1 commit into
Open
consumer_key: trim an explicit /* or / suffix in add_recursive_rules#150Occansey wants to merge 1 commit into
Occansey wants to merge 1 commit into
Conversation
path.rstrip("*/ ") stripped a whole set of characters from the tail, which
could alter the granted pattern in surprising ways (e.g. a trailing space, or
a path ending in one of those characters). Trim exactly one trailing "/*" or
"/" instead.
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.
In
add_recursive_rules,path.rstrip("*/ ")strips a set of characters from the tail, so it also removes a trailing*,/or space wherever it sits at the end. That can change the granted pattern in ways the caller did not intend (for example a path passed as"/vps*", or one with a trailing space).This trims exactly one trailing
/*or/instead, so the granted pattern matches what the caller wrote:Behaviour is unchanged for the documented cases:
"/sms","/sms/"and"/sms/*"all yield/smsplus/sms/*. The existing test suite passes.Happy to adjust if you'd prefer a different approach.
Thanks,
Edem Occansey