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

fix: the query string is repeated twice when enable http-to-https and append-query-string (#7394) #7433

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions apisix/plugins/redirect.lua
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,14 @@ local schema = {
},
http_to_https = {type = "boolean"},
encode_uri = {type = "boolean", default = false},
append_query_string = {type = "boolean", default = false},
append_query_string = {type = "boolean"},
tokers marked this conversation as resolved.
Show resolved Hide resolved
},
oneOf = {
{required = {"uri"}},
{required = {"regex_uri"}},
{required = {"http_to_https"}}
Copy link
Member

Choose a reason for hiding this comment

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

["not"] = {required = {"client_cert_id"}}

We can use not operation to show the exclusive relationship?

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 have tried, but not succeed

["not"] = {
    allOf = {
        {required = {"append_query_string"}},
        {required = {"http_to_https"}},
    }
}  

and

["not"] = {required = {"append_query_string", "http_to_https"}},

Copy link
Member

Choose a reason for hiding this comment

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

http_to_https = {
    ["not"] = {required = {"uri", "regex_uri", "append_query_string"}}
},

uri = {
    ["not"] = {required = {"regex_uri", "http_to_https"}}
},

regex_uri = {
    ["not"] = {required = {"uri", "http_to_https"}}
},

what about this form?

Copy link
Member

Choose a reason for hiding this comment

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

Give it a try:

    oneOf = {
        {required = {"uri"}},
        {required = {"regex_uri"}},
        {required = {"http_to_https"}, ["not"] = {
           required = {"append_query_string"}
        }}
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Give it a try:

    oneOf = {
        {required = {"uri"}},
        {required = {"regex_uri"}},
        {required = {"http_to_https"}, ["not"] = {
           required = {"append_query_string"}
        }}
    }

It does work, i have updated, please check @spacewander

{required = {"http_to_https"}, ["not"] = {
required = {"append_query_string"}
}}
}
}

Expand Down
6 changes: 3 additions & 3 deletions docs/en/latest/plugins/redirect.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ The `redirect` Plugin can be used to configure redirects.

| Name | Type | Required | Default | Valid values | Description |
|---------------------|---------------|----------|---------|--------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| http_to_https | boolean | False | false | | When set to `true` and the request is HTTP, it will be redirected to HTTPS with the same URI with a 301 status code. |
| http_to_https | boolean | False | false | | When set to `true` and the request is HTTP, it will be redirected to HTTPS with the same URI with a 301 status code. Note the querystring from the raw URI will also be contained in the Location header. |
| uri | string | False | | | URI to redirect to. Can contain Nginx variables. For example, `/test/index.html`, `$uri/index.html`, `${uri}/index.html`. If you refer to a variable name that doesn't exist, instead of throwing an error, it will treat it as an empty variable. |
| regex_uri | array[string] | False | | | Match the URL from client with a regular expression and redirect. If it doesn't match, the request will be forwarded to the Upstream. Only either of `uri` or `regex_uri` can be used at a time. For example, [" ^/iresty/(.*)/(.*)/(.*)", "/$1-$2-$3"], where the first element is the regular expression to match and the second element is the URI to redirect to. |
| ret_code | integer | False | 302 | [200, ...] | HTTP response code. |
Expand All @@ -43,8 +43,8 @@ The `redirect` Plugin can be used to configure redirects.

:::note

Only one of `http_to_https`, `uri` and `regex_uri` can be configured.

* Only one of `http_to_https`, `uri` and `regex_uri` can be configured.
* Only one of `http_to_https` and `append_query_string` can be configured.
* When enabling `http_to_https`, the ports in the redirect URL will pick a value in the following order (in descending order of priority)
* Read `plugin_attr.redirect.https_port` from the configuration file (`conf/config.yaml`).
* If `apisix.ssl` is enabled, read `apisix.ssl.listen_port` first, and if it does not exist, read `apisix.ssl.listen` and select a port randomly from it.
Expand Down
8 changes: 4 additions & 4 deletions docs/zh/latest/plugins/redirect.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,17 @@ description: 本文介绍了关于 Apache APISIX `redirect` 插件的基本信

| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述 |
|---------------------|---------------|-----|-------|--------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| http_to_https | boolean | 否 | false | [true,false] | 当设置为 `true` 并且请求是 HTTP 时,它将被重定向具有相同 URI 和 301 状态码的 HTTPS。 |
| http_to_https | boolean | 否 | false | [true,false] | 当设置为 `true` 并且请求是 HTTP 时,它将被重定向具有相同 URI 和 301 状态码的 HTTPS,原 URI 的查询字符串也将包含在 Location 头中。 |
| uri | string | 否 | | | 要重定向到的 URI,可以包含 NGINX 变量。例如:`/test/index.htm`, `$uri/index.html`,`${uri}/index.html`。如果你引入了一个不存在的变量,它不会报错,而是将其视为一个空变量。 |
| regex_uri | array[string] | 否 | | | 将来自客户端的 URL 与正则表达式匹配并重定向。当匹配成功后使用模板替换发送重定向到客户端,如果未匹配成功会将客户端请求的 URI 转发至上游。 和 `regex_uri` 不可以同时存在。例如:["^/iresty/(.)/(.)/(.*)","/$1-$2-$3"] 第一个元素代表匹配来自客户端请求的 URI 正则表达式,第二个元素代表匹配成功后发送重定向到客户端的 URI 模板。 |
| ret_code | integer | 否 | 302 | [200, ...] | HTTP 响应码 |
| encode_uri | boolean | 否 | false | [true,false] | 当设置为 `true` 时,对返回的 `Location` Header 按照 [RFC3986](https://datatracker.ietf.org/doc/html/rfc3986)的编码格式进行编码。 |
| encode_uri | boolean | 否 | false | [true,false] | 当设置为 `true` 时,对返回的 `Location` Header 按照 [RFC3986](https://datatracker.ietf.org/doc/html/rfc3986) 的编码格式进行编码。 |
| append_query_string | boolean | 否 | false | [true,false] | 当设置为 `true` 时,将原始请求中的查询字符串添加到 `Location` Header。如果已配置 `uri` 或 `regex_uri` 已经包含查询字符串,则请求中的查询字符串将附加一个`&`。如果你已经处理过查询字符串(例如,使用 NGINX 变量 `$request_uri`),请不要再使用该参数以避免重复。 |

:::note

`http_to_https`、`uri` 和 `regex_uri` 只能配置其中一个属性。

* `http_to_https`、`uri` 和 `regex_uri` 只能配置其中一个属性。
* `http_to_https`、和 `append_query_string` 只能配置其中一个属性。
* 当开启 `http_to_https` 时,重定向 URL 中的端口将按如下顺序选取一个值(按优先级从高到低排列)
* 从配置文件(`conf/config.yaml`)中读取 `plugin_attr.redirect.https_port`。
* 如果 `apisix.ssl` 处于开启状态,先读取 `apisix.ssl.listen_port`,如果没有,再读取 `apisix.ssl.listen` 并从中随机选一个 `port`。
Expand Down
35 changes: 35 additions & 0 deletions t/plugin/redirect.t
Original file line number Diff line number Diff line change
Expand Up @@ -1114,3 +1114,38 @@ X-Forwarded-Proto: http
--- error_code: 301
--- response_headers
Location: https://foo.com:9443/hello



=== TEST 47: wrong configure, enable http_to_https with append_query_string
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"uri": "/hello",
"host": "foo.com",
"plugins": {
"redirect": {
"http_to_https": true,
"append_query_string": true
}
}
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body eval
qr/error_msg":"failed to check the configuration of plugin redirect err: value should match only one schema, but matches none/
--- no_error_log
[error]