Skip to content

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

Merged
spacewander merged 4 commits into
apache:masterfrom
juststillthinking:support-variable-when-rewrite-header-in-proxy-rewrite-plugin
Mar 24, 2023
Merged

feat: support variable when rewrite header in proxy rewrite plugin#9112
spacewander merged 4 commits into
apache:masterfrom
juststillthinking:support-variable-when-rewrite-header-in-proxy-rewrite-plugin

Conversation

@juststillthinking

@juststillthinking juststillthinking commented Mar 20, 2023

Copy link
Copy Markdown
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)

spacewander
spacewander reviewed Mar 21, 2023
View reviewed changes
Comment thread apisix/core/utils.lua Outdated
local resolve_var_with_captures
do
local _captures
-- escape is not supported very well, like there si a redundant '\' after escape "$1"

@spacewander spacewander Mar 21, 2023

Copy link
Copy Markdown
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"

@juststillthinking juststillthinking Mar 23, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread apisix/plugins/proxy-rewrite.lua Outdated
if not m then
if err then
core.log.error("match error in proxy-rewrite plugin, please check: ", err)
return

@spacewander spacewander Mar 21, 2023

Copy link
Copy Markdown
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?

@juststillthinking juststillthinking Mar 22, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

Comment thread apisix/plugins/proxy-rewrite.lua Outdated

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

@spacewander spacewander Mar 21, 2023

Copy link
Copy Markdown
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?

@juststillthinking juststillthinking Mar 22, 2023

Copy link
Copy Markdown
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?

@spacewander spacewander Mar 23, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See #9112 (comment)

@juststillthinking juststillthinking Mar 23, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread apisix/core/utils.lua
@leslie-tsang
leslie-tsang self-requested a review March 21, 2023 02:52
leslie-tsang
leslie-tsang reviewed Mar 23, 2023
View reviewed changes
Comment thread apisix/plugins/proxy-rewrite.lua Outdated
soulbird
soulbird reviewed Mar 23, 2023
View reviewed changes
Comment thread apisix/plugins/proxy-rewrite.lua
@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

Reviewers

@spacewander spacewander spacewander approved these changes
@soulbird soulbird soulbird approved these changes
@leslie-tsang leslie-tsang leslie-tsang 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.

4 participants

@juststillthinking @spacewander @soulbird @leslie-tsang