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(redirect): set redirect server port when enable http_to_https #6686

Merged
merged 12 commits into from
May 5, 2022
Merged
Show file tree
Hide file tree
Changes from 6 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
1 change: 1 addition & 0 deletions apisix/core/ctx.lua
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ do
upstream_cache_bypass = true,

var_x_forwarded_proto = true,
var_x_forwarded_port = true,
}

-- sort in alphabetical
Expand Down
6 changes: 3 additions & 3 deletions apisix/plugins/redirect.lua
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ local ipairs = ipairs
local ngx = ngx
local str_find = core.string.find
local str_sub = string.sub
local tonumber = tonumber

local lrucache = core.lrucache.new({
ttl = 300, count = 100
Expand All @@ -38,7 +39,6 @@ local schema = {
type = "object",
properties = {
ret_code = {type = "integer", minimum = 200, default = 302},
ret_port = {type = "integer", minimum = 1, maxmum = 65535, default = 443},
uri = {type = "string", minLength = 2, pattern = reg},
regex_uri = {
description = "params for generating new uri that substitute from client uri, " ..
Expand Down Expand Up @@ -148,7 +148,7 @@ function _M.rewrite(conf, ctx)
core.log.info("plugin rewrite phase, conf: ", core.json.delay_encode(conf))

local ret_code = conf.ret_code
local ret_port = conf.ret_port
local ret_port = tonumber(ctx.var["var_x_forwarded_port"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Recently, I upgraded apisix with the latest code, and found that there was an error in converting http to https. When redirecting, I always bring a port 80. It should be caused by this change, because I am listening on port 80 and port 443, but ngx_tpl There is a piece of code in it that is "set $var_x_forwarded_port $server_port;", which means that the port from http to https is always the same. I don't think this change is very reasonable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, seems I got what you mean. The server port 80 and then enable http_to_https, it'll always redirect to https://host:80 then get 400 bad request. cc @tokers @spacewander @tzssangglass

We need to take more consideration about #6686 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Can we set a default value for this case?
If there is no X-Forwarded-Port in clinet reuqest headers, then the redirect plugin use 443 as the default https port?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has default as the server_port

set $var_x_forwarded_port $server_port;

As mentioned before #6686 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

It has default as the server_port

I mean in the redirect plugin, if someone enable http_to_https and the client request doesn't pass X-Forwarded-Port, use 443 as the default port in Location and don't care about the server_port variable.

Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, it is definitely impossible to know the listening port of https for this domain name if it is not configured from http to https, so it can be considered that the port can be configured, but the default is 443?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As the conversation before #6686 (comment)

The original implement with an extra attribute ret_port, but no acceptable.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can read the correct port from config.yaml?

Choose a reason for hiding this comment

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

把这个set $var_x_forwarded_port $server_port; 改成set $var_x_forwarded_port 443;

local uri = conf.uri
local regex_uri = conf.regex_uri

Expand All @@ -157,7 +157,7 @@ function _M.rewrite(conf, ctx)
if conf.http_to_https and _scheme == "http" then
-- TODO: add test case
-- PR: https://github.com/apache/apisix/pull/1958
if ret_port == 443 then
if ret_port == 443 or ret_port == 0 then
tokers marked this conversation as resolved.
Show resolved Hide resolved
uri = "https://$host$request_uri"
else
uri = "https://$host:" .. ret_port .. "$request_uri"
Expand Down
24 changes: 11 additions & 13 deletions docs/en/latest/plugins/redirect.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,21 @@ The `redirect` Plugin can be used to configure redirects.

## Attributes

| Name | Type | Requirement | Default | Valid | Description |
| ------------- | ------- | ----------- | ------- | ----- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| http_to_https | boolean | optional | false | | When it is set to `true` and the request is HTTP, will be automatically redirected to HTTPS with 301 response code, and the URI will keep the same as client request. |
| uri | string | optional | | | New URL which can contain Nginx variable, eg: `/test/index.html`, `$uri/index.html`. You can refer to variables in a way similar to `${xxx}` to avoid ambiguity, eg: `${uri}foo/index.html`. If you just need the original `$` character, add `\` in front of it, like this one: `/\$foo/index.html`. If you refer to a variable name that does not exist, this will not produce an error, and it will be used as an empty string. |
| regex_uri | array[string] | optional | | | Use regular expression to match URL from client, when the match is successful, the URL template will be redirected to. If the match is not successful, the URL from the client will be forwarded to the upstream. Only one of `uri` and `regex_uri` can be exist. For example: [" ^/iresty/(.*)/(.*)/(.*)", "/$1-$2-$3"], the first element represents the matching regular expression and the second element represents the URL template that is redirected to. |
| ret_code | integer | optional | 302 | [200, ...] | Response code |
| ret_port | integer | optional | 443 | [1, 65535] | Redirect server port, only work when enable `http_to_https`. |
| encode_uri | boolean | optional | false | | When set to `true` the uri in `Location` header will be encoded as per [RFC3986](https://datatracker.ietf.org/doc/html/rfc3986) |
| append_query_string | boolean | optional | false | | When set to `true`, add the query string from the original request to the location header. If the configured `uri` / `regex_uri` already contains a query string, the query string from request will be appended to that after an `&`. Caution: don't use this if you've already handled the query string, e.g. with nginx variable $request_uri, to avoid duplicates. |
| 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. |
| 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. |
| encode_uri | boolean | False | false | | When set to `true` the URI in the `Location` header will be encoded as per [RFC3986](https://datatracker.ietf.org/doc/html/rfc3986). |
| append_query_string | boolean | False | false | | When set to `true`, adds the query string from the original request to the `Location` header. If the configured `uri` or `regex_uri` already contains a query string, the query string from the request will be appended to it with an `&`. Do not use this if you have already handled the query string (for example, with an Nginx variable `$request_uri`) to avoid duplicates. |

:::note

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

* When enable `http_to_https`, you can specify redirect server port with the header `X-Forwarded-Port`.
kwanhur marked this conversation as resolved.
Show resolved Hide resolved

:::

## Enabling the Plugin
Expand Down Expand Up @@ -107,7 +108,6 @@ Content-Type: text/html
Content-Length: 166
Connection: keep-alive
Location: /test/default.html

...
```

Expand All @@ -121,8 +121,7 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f1
"uri": "/hello",
"plugins": {
"redirect": {
"http_to_https": true,
"ret_port": 9443
"http_to_https": true
}
}
}'
Expand All @@ -138,7 +137,6 @@ curl http://127.0.0.1:9080/hello -i
HTTP/1.1 301 Moved Permanently
...
Location: https://127.0.0.1:9443/hello

...
```

Expand Down
22 changes: 11 additions & 11 deletions docs/zh/latest/plugins/redirect.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,21 @@ description: 本文介绍了关于 Apache APISIX `redirect` 插件的基本信

## 属性

| Name | Type | Requirement | Default | Valid | Description |
| ------------- | ------- | ----------- | ------- | ---------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| http_to_https | boolean | 可选 | false | | 当设置为 `true` 并且请求是 http 时,会自动 301 重定向为 https,uri 保持不变 |
| uri | string | 可选 | | | 可以包含 Nginx 变量的 URI,例如:`/test/index.html`, `$uri/index.html`。你可以通过类似于 `$ {xxx}` 的方式引用变量,以避免产生歧义,例如:`${uri}foo/index.html`。若你需要保留 `$` 字符,那么使用如下格式:`/\$foo/index.html` |
| regex_uri | array[string] | 可选 | | | 转发到上游的新 `uri` 地址, 使用正则表达式匹配来自客户端的 `uri`,当匹配成功后使用模板替换发送重定向到客户端, 未匹配成功时将客户端请求的 `uri` 转发至上游。`uri` 和 `regex_uri` 不可以同时存在。例如:["^/iresty/(.*)/(.*)/(.*)","/$1-$2-$3"] 第一个元素代表匹配来自客户端请求的 `uri` 正则表达式,第二个元素代表匹配成功后发送重定向到客户端的 `uri` 模板。 |
| ret_code | integer | 可选 | 302 | [200, ...] | 请求响应码 |
| ret_port | integer | 可选 | 443 | [1, 65535] | 重定向服务器端口,仅在开启 `http_to_https` 有效。|
| encode_uri | boolean | 可选 | false | | 当设置为 `true` 时,对返回的 `Location` header进行编码,编码格式参考 [RFC3986](https://datatracker.ietf.org/doc/html/rfc3986) |
| append_query_string | boolean | optional | false | | 当设置为 `true` 时,将请求url的query部分添加到Location里。如果在 `uri` 或 `regex_uri` 中配置了query, 那么请求的query会被追加在这个query后,以 `&` 分隔。 注意:如果已经处理了query,比如使用了nginx变量 `$request_uri`,那么启用此功能会造成query重复 |
| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述 |
|---------------------|---------------|-----|-------|--------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| http_to_https | boolean | 是 | false | [true,false] | 当设置为 `true` 并且请求是 HTTP 时,它将被重定向具有相同 URI 和 301 状态码的 HTTPS。 |
| 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)的编码格式进行编码。 |
| 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`,你可以设置请求头 `X-Forwarded-Port` 指定重定向的服务器端口。
kwanhur marked this conversation as resolved.
Show resolved Hide resolved

:::

## 启用插件
Expand Down Expand Up @@ -123,8 +124,7 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 \
"uri": "/hello",
"plugins": {
"redirect": {
"http_to_https": true,
"ret_port": 9443
"http_to_https": true
}
}
}'
Expand Down
Loading