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

feat: support variable when rewrite header in proxy rewrite plugin #9112

Conversation

monkeyDluffy6017
Copy link
Contributor

Description

Support for referencing the match result of the regex_uri when rewrite header in the proxy-rewrite plugin, and the match result can be referenced as a variable by other plugins too.

Nginx:

location ~* ^/aaa/(.*)$ {
    ...
    proxy_set_header HDR_TEST $1;
}

Apisix:

"plugins": {
    "proxy-rewrite": {
        "regex_uri": ["^/test/(.*)/(.*)/(.*)", "/$1_$2_$3"],
        "headers": {
            "X-Api:Version": "v2-$1"
        }
    }
},

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

local resolve_var_with_captures
do
local _captures
-- escape is not supported very well, like there si a redundant '\' after escape "$1"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
-- escape is not supported very well, like there si a redundant '\' after escape "$1"
-- escape is not supported very well, like there is a redundant '\' after escape "$1"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if not m then
if err then
core.log.error("match error in proxy-rewrite plugin, please check: ", err)
return
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to return a 503 for this error?

Copy link
Contributor Author

@monkeyDluffy6017 monkeyDluffy6017 Mar 22, 2023

Choose a reason for hiding this comment

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

I prefer 500, it's an unexpected condition


local m, err = re_match(upstream_uri, conf.regex_uri[1], "jo")
if not m then
if err then
Copy link
Member

Choose a reason for hiding this comment

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

We can merge the two conditions above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how? use the re_match first, then use the match result to replace?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

apisix/core/utils.lua Show resolved Hide resolved
@leslie-tsang leslie-tsang self-requested a review March 21, 2023 02:52
@spacewander spacewander merged commit 777c0b7 into apache:master Mar 24, 2023
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.

4 participants