Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failing rootpath whitelist test #263

Merged
merged 5 commits into from
Oct 18, 2021
Merged

Conversation

mcshaz
Copy link
Contributor

@mcshaz mcshaz commented Oct 8, 2021

Tests added to demonstrate issue #262 (failing with rootpath "/" in whitelist)

@mcshaz
Copy link
Contributor Author

mcshaz commented Oct 8, 2021

As you can see from the code, a default page (path /) can now be used in the EndpointWhitelist. Fixing this and making all tests run revealed an issue with the regex tester - namely if whitelisting "get:/foo" the path "get:/foo/bar/baz" is a match.

I would advocate that if that behavior were desired, the whitelist should be written as "get:/foo/*" and this was a problem with the prior implementation. The other option of course would be to let the consuming appSettings file specify "get:/foo$", however this would be quite different behavior to the wildcard match, and would (I believe) require clear documentation.

@cristipufu cristipufu merged commit 9b50b72 into stefanprodan:master Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants