-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix: support regex_uri with unsafe_uri in proxy-rewrite #9813
fix: support regex_uri with unsafe_uri in proxy-rewrite #9813
Conversation
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
Please fix the code lint |
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
@monkeyDluffy6017 Done. Please approve CI |
apisix/plugins/proxy-rewrite.lua
Outdated
if conf.use_real_request_uri_unsafe and conf.regex_uri then | ||
local index | ||
if separator_escaped then | ||
index = str_find(upstream_uri,"?") | ||
end | ||
if index then | ||
upstream_uri = sub_str(upstream_uri, 1, index - 1) | ||
..sub_str(upstream_uri,index) | ||
end | ||
ctx.var.upstream_uri = upstream_uri | ||
end |
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.
Why do you add these code?
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.
You're right, in the if block above this when the safe encoding was on then this logic was added to first remove the ?
and then readd after safely encoding. When we're skipping the safe encoding then we don't need to do any of that but I added it by mistake. Removed this.
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
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.
Please add blank around the new code block
…x into revolyssup/fixunsafebug
done |
Description
Fixes #9638
Tests to be added
Checklist