-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Case insensitive host rule #3931
Conversation
It's normal that tests are failing because the fix is in containous/mux#5. |
Thanks you for your PR. I made a PR on containous/mux#6. Do you think you can add a test to verify that the rule Moreover, this a bug fix so I think we can merge it in the next 1.7. Can you make this PR on the 1.7 branch? |
254558d
to
3d5ec96
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
437ce38
to
f735001
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…dled (host patterns instead of plain hostnames)
f735001
to
dd528e5
Compare
Thank you guys for merging! |
What does this PR do?
This PR introduces a failing test for issue #3930.
Here we also rename parameters and variables in the
hostRegexp()
function to better reflect what is actually handled. Indeed, this function is written as if it was manipulating plain hostnames, whereas it actually handles host patterns that might include regular expressions.Motivation
I wanted to demonstrate the issue #3930, and also address a misnaming that was pointed out while analyzing that issue.
More
Additional Notes
Relates to #3930.