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

bug: Trailing slash with argument is not respected #140

Merged
merged 3 commits into from
Dec 29, 2023

Conversation

boekkooi-lengoo
Copy link
Contributor

Good day.

Thanks for having a look at this PR that is trying to fix an issue where the trailing slash of a path is not respected when a argument is set before it.

Cheers,
Warnar

Avoid unused loops, trailing whitespaces and hiding global vars.
Show the issue where when an argument is specified it's also matching without the trailing slash
As ngx.re.split will `delete empty trailing ones` we need to double check that there is no required `/` at the end of the path.
If there is a extra `/` then add it to the pat regex.

Also see https://github.com/openresty/lua-resty-core/blob/50f73790b6bb1d83f52cb3b4ec7e4ab7f649ab32/lib/ngx/re.lua#L273
@boekkooi-lengoo boekkooi-lengoo changed the title Trailing slash with argument is not respected bug: Trailing slash with argument is not respected Dec 19, 2023
@monkeyDluffy6017
Copy link

Thanks for your contribution! we will check it later

Copy link
Contributor

@membphis membphis left a comment

Choose a reason for hiding this comment

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

High quality PR, LGTM

@membphis
Copy link
Contributor

@monkeyDluffy6017 welcome to take a look at this PR, thx

Copy link
Member

@shreemaan-abhishek shreemaan-abhishek left a comment

Choose a reason for hiding this comment

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

LGTM

@moonming moonming merged commit dd87111 into api7:master Dec 29, 2023
2 checks passed
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.

5 participants