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.

Sign up for GitHub

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

Jump to bottom

bugfix: symbol would be encoded to 0x3f when using proxy-rewrite directive #1816

Merged
merged 12 commits into from Jul 9, 2020
Merged

bugfix: symbol would be encoded to 0x3f when using proxy-rewrite directive #1816

merged 12 commits into from Jul 9, 2020

Conversation

wrl96
Copy link
Contributor

@wrl96 wrl96 commented Jul 8, 2020

Summary

bugfix: symbol would be encoded to 0x3f when using proxy-rewrite directive

Full changelog

Issues resolved

Fix #1801

spacewander
spacewander requested changes Jul 9, 2020
View reviewed changes
apisix/plugins/proxy-rewrite.lua Outdated Show resolved Hide resolved
membphis
membphis approved these changes Jul 9, 2020
View reviewed changes
Copy link
Member

@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.

LGTM

@membphis membphis requested a review from spacewander July 9, 2020 05:41
moonming
moonming reviewed Jul 9, 2020
View reviewed changes
apisix/plugins/proxy-rewrite.lua

if ctx.var.is_args == "?" then
ctx.var.upstream_uri = upstream_uri .. "?" .. (ctx.var.args or "")
if index then
ctx.var.upstream_uri = upstream_uri .. "&" .. (ctx.var.args or "")
Copy link
Member

@moonming moonming Jul 9, 2020 •
edited

Choose a reason for hiding this comment

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

I'm wondering do we need to keep the request args?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm wondering do we need to keep the request args?

I think we need to keep it.
If someone needn't it, he can remove args by regex_uri.

@membphis
Copy link
Member

membphis commented Jul 9, 2020

@wrl96 https://travis-ci.org/github/apache/incubator-apisix/jobs/706394034#L991

you need to rebase your branch, that is the bug of Travis CI. we can not merge this PR if the CI is failed.

@wrl96
Copy link
Contributor Author

wrl96 commented Jul 9, 2020

@wrl96 https://travis-ci.org/github/apache/incubator-apisix/jobs/706394034#L991

you need to rebase your branch, that is the bug of Travis CI. we can not merge this PR if the CI is failed.

Fixed

@membphis membphis merged commit c41abab into apache:master Jul 9, 2020
@membphis
Copy link
Member

membphis commented Jul 9, 2020

@wrl96 many thx, merged

BTW, you should use git rebase

@wrl96
Copy link
Contributor Author

wrl96 commented Jul 9, 2020

@wrl96 many thx, merged

BTW, you should use git rebase

ok, I used Rebase my Github fork in idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moonming moonming moonming left review comments

@spacewander spacewander spacewander approved these changes

@membphis membphis membphis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

request help: how to rewrite query parameters using proxy-rewrite
6 participants
@wrl96 @membphis @spacewander @moonming @yongboy @EnableAsync