Skip to content

Conversation

@NisanthanNanthakumar
Copy link
Contributor

Fixes SENTRY-S38

@NisanthanNanthakumar NisanthanNanthakumar requested a review from a team October 28, 2021 18:23
trailing_slash_star = pattern[-1] == "*" and len(pattern) > 1 and pattern[-2] == "/"
try:
trailing_slash_star = pattern[-1] == "*" and len(pattern) > 1 and pattern[-2] == "/"
except IndexError:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we just check for a pattern length of 1 and return False for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AniketDas-Tekky this is searching for the /* case. It needs to look for both characters.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if the pattern length is 1, then it can never be /*

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just think we should prevent the exception from happening in the first place instead of doing a try/catch

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AniketDas-Tekky ahh that's what you meant! makes sense!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AniketDas-Tekky hmm we already do a check for that in that expression

@NisanthanNanthakumar NisanthanNanthakumar merged commit b39e9d8 into master Oct 28, 2021
@NisanthanNanthakumar NisanthanNanthakumar deleted the API-2169 branch October 28, 2021 21:50
@github-actions github-actions bot locked and limited conversation to collaborators Nov 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants