You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/05/17 11:03:45 UTC
[GitHub] [apisix] jwrookie opened a new pull request, #7065: fix: redirect http to https but port not change
jwrookie opened a new pull request, #7065:
URL: https://github.com/apache/apisix/pull/7065
### Description
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
Fixes #7011
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jwrookie commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
jwrookie commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r874796701
##########
conf/config-default.yaml:
##########
@@ -470,3 +470,4 @@ plugin_attr:
connect: 60s
read: 60s
send: 60s
+ redirect_https_port: 8443 # the default port for use by HTTP redirects to HTTPS
Review Comment:
done
##########
apisix/plugins/redirect.lua:
##########
@@ -148,7 +149,30 @@ 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 = tonumber(ctx.var["var_x_forwarded_port"])
+ local local_conf = core.config.local_conf()
+ local ret_port = core.table.try_read_attr(local_conf,
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
spacewander merged PR #7065:
URL: https://github.com/apache/apisix/pull/7065
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jwrookie commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
jwrookie commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r875398301
##########
apisix/plugins/redirect.lua:
##########
@@ -148,7 +150,30 @@ 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 = tonumber(ctx.var["var_x_forwarded_port"])
+ local ret_port = nil
+ local attr = plugin.plugin_attr(plugin_name)
Review Comment:
here:
https://github.com/apache/apisix/blob/6ebe02797fb564c84b30df1865adef7f473880bf/apisix/plugins/redirect.lua#L65-L67
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jwrookie commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
jwrookie commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r874800880
##########
t/plugin/redirect.t:
##########
@@ -428,66 +428,69 @@ passed
-=== TEST 18: redirect
+=== TEST 18: redirect(port using `plugin_attr.redirect_https_port`)
--- request
GET /hello
--- more_headers
Host: foo.com
--- error_code: 301
--- response_headers
-Location: https://foo.com:1984/hello
+Location: https://foo.com:8443/hello
-=== TEST 19: redirect(pass well-known port 443 to x-forwarded-port)
+=== TEST 19: redirect(port using `apisix.ssl.listen_port`)
+--- yaml_config
+apisix:
+ ssl:
+ enable: true
+ listen_port: 9445
+--- extra_yaml_config
+plugin_attr:
+ redirect_https_port: null
--- request
GET /hello
--- more_headers
Host: foo.com
-x-forwarded-port: 443
--- error_code: 301
--- response_headers
-Location: https://foo.com/hello
-
-
-
-=== TEST 20: redirect(pass negative number to x-forwarded-port)
---- request
-GET /hello
---- more_headers
-Host: foo.com
-x-forwarded-port: -443
---- error_code: 301
---- response_headers
-Location: https://foo.com/hello
+Location: https://foo.com:9445/hello
-=== TEST 21: redirect(pass number more than 65535 to x-forwarded-port)
+=== TEST 20: redirect(port using `apisix.ssl.listen`)
+--- extra_yaml_config
+plugin_attr:
+ redirect_https_port: null
--- request
GET /hello
--- more_headers
Host: foo.com
-x-forwarded-port: 65536
--- error_code: 301
--- response_headers
-Location: https://foo.com/hello
+Location: https://foo.com:9443/hello
-=== TEST 22: redirect(pass invalid non-number to x-forwarded-port)
+=== TEST 21: redirect(port using `https default port`)
+--- yaml_config
+apisix:
+ ssl:
+ enable: null
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r874704335
##########
apisix/plugins/redirect.lua:
##########
@@ -148,7 +149,30 @@ 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 = tonumber(ctx.var["var_x_forwarded_port"])
+ local local_conf = core.config.local_conf()
+ local ret_port = core.table.try_read_attr(local_conf,
Review Comment:
There is a wrapper to get the attribute of a plugin:
https://github.com/apache/apisix/blob/6ebe02797fb564c84b30df1865adef7f473880bf/apisix/plugins/prometheus.lua#L58
We need to use it instead.
##########
t/plugin/redirect.t:
##########
@@ -428,66 +428,69 @@ passed
-=== TEST 18: redirect
+=== TEST 18: redirect(port using `plugin_attr.redirect_https_port`)
--- request
GET /hello
--- more_headers
Host: foo.com
--- error_code: 301
--- response_headers
-Location: https://foo.com:1984/hello
+Location: https://foo.com:8443/hello
-=== TEST 19: redirect(pass well-known port 443 to x-forwarded-port)
+=== TEST 19: redirect(port using `apisix.ssl.listen_port`)
+--- yaml_config
+apisix:
+ ssl:
+ enable: true
+ listen_port: 9445
+--- extra_yaml_config
+plugin_attr:
+ redirect_https_port: null
--- request
GET /hello
--- more_headers
Host: foo.com
-x-forwarded-port: 443
--- error_code: 301
--- response_headers
-Location: https://foo.com/hello
-
-
-
-=== TEST 20: redirect(pass negative number to x-forwarded-port)
---- request
-GET /hello
---- more_headers
-Host: foo.com
-x-forwarded-port: -443
---- error_code: 301
---- response_headers
-Location: https://foo.com/hello
+Location: https://foo.com:9445/hello
-=== TEST 21: redirect(pass number more than 65535 to x-forwarded-port)
+=== TEST 20: redirect(port using `apisix.ssl.listen`)
+--- extra_yaml_config
+plugin_attr:
+ redirect_https_port: null
--- request
GET /hello
--- more_headers
Host: foo.com
-x-forwarded-port: 65536
--- error_code: 301
--- response_headers
-Location: https://foo.com/hello
+Location: https://foo.com:9443/hello
-=== TEST 22: redirect(pass invalid non-number to x-forwarded-port)
+=== TEST 21: redirect(port using `https default port`)
+--- yaml_config
+apisix:
+ ssl:
+ enable: null
Review Comment:
Let's add two more test:
1. use listen_port
2. use listen and get port in the array
##########
conf/config-default.yaml:
##########
@@ -470,3 +470,4 @@ plugin_attr:
connect: 60s
read: 60s
send: 60s
+ redirect_https_port: 8443 # the default port for use by HTTP redirects to HTTPS
Review Comment:
We should use a structure like this:
```
redirect:
https_port: ...
```
and comment it out as nobody wants to be redirected to 8443 by default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
tokers commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r875397484
##########
apisix/plugins/redirect.lua:
##########
@@ -148,7 +150,30 @@ 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 = tonumber(ctx.var["var_x_forwarded_port"])
+ local ret_port = nil
+ local attr = plugin.plugin_attr(plugin_name)
Review Comment:
Where did you define the variable `plugin_name`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jwrookie commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
jwrookie commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r875398831
##########
apisix/plugins/redirect.lua:
##########
@@ -148,7 +150,30 @@ 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 = tonumber(ctx.var["var_x_forwarded_port"])
+ local ret_port = nil
+ local attr = plugin.plugin_attr(plugin_name)
Review Comment:
Was defined before :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
soulbird commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r875397182
##########
apisix/plugins/redirect.lua:
##########
@@ -148,7 +150,30 @@ 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 = tonumber(ctx.var["var_x_forwarded_port"])
+ local ret_port = nil
+ local attr = plugin.plugin_attr(plugin_name)
+ if attr then
+ ret_port = attr.https_port
+ end
+
+ if not ret_port then
+ local local_conf = core.config.local_conf()
+ local ssl = core.table.try_read_attr(local_conf, "apisix", "ssl")
+ if ssl and ssl["enable"] then
+ ret_port = ssl["listen_port"]
+ if not ret_port then
+ local ret_ports = ssl["listen"]
+ if ret_ports and #ret_ports > 0 then
+ local idx = math_random(1, #ret_ports)
+ ret_port = ret_ports[idx]
+ if type(ret_port) == "table" then
+ ret_port = ret_port.port
+ end
+ end
+ end
+ end
+ end
+
Review Comment:
Personally, I feel that this code is too inelegant. Suggest:
```lua
local function get_port(attr)
local port
if attr then
port = attr.https.port
end
if port then
return port
end
local local_conf = core.config.local_conf()
local ssl = core.table.try_read_attr(local_conf, "apisix", "ssl")
if not ssl or ssl["enable"] then
return port
end
port = ssl["listen_port"]
if port then
return port
end
local ports = ssl["listen"]
if ports and #ports > 0 then
local idx = math_random(1, #ports)
port = ports[idx]
if type(port) == "table" then
port = port.port
end
end
return port
end
local ret_port = get_port(attr)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jwrookie commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
jwrookie commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r875418292
##########
apisix/plugins/redirect.lua:
##########
@@ -148,7 +150,30 @@ 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 = tonumber(ctx.var["var_x_forwarded_port"])
+ local ret_port = nil
+ local attr = plugin.plugin_attr(plugin_name)
+ if attr then
+ ret_port = attr.https_port
+ end
+
+ if not ret_port then
+ local local_conf = core.config.local_conf()
+ local ssl = core.table.try_read_attr(local_conf, "apisix", "ssl")
+ if ssl and ssl["enable"] then
+ ret_port = ssl["listen_port"]
+ if not ret_port then
+ local ret_ports = ssl["listen"]
+ if ret_ports and #ret_ports > 0 then
+ local idx = math_random(1, #ret_ports)
+ ret_port = ret_ports[idx]
+ if type(ret_port) == "table" then
+ ret_port = ret_port.port
+ end
+ end
+ end
+ end
+ end
+
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r874711300
##########
docs/en/latest/plugins/redirect.md:
##########
@@ -45,7 +45,10 @@ The `redirect` Plugin can be used to configure redirects.
Only one of `http_to_https`, `uri` and `regex_uri` can be configured.
-* When enabling `http_to_https`, the port in the redirect URL will be the value of header `X-Forwarded-Port` or the port of the server.
+* 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.
Review Comment:
```suggestion
* Read `plugin_attr.redirect_https_port` from the configuration file (`conf/config.yaml`).
```
##########
docs/zh/latest/plugins/redirect.md:
##########
@@ -45,7 +45,10 @@ description: 本文介绍了关于 Apache APISIX `redirect` 插件的基本信
`http_to_https`、`uri` 和 `regex_uri` 只能配置其中一个属性。
-* 当开启 `http_to_https` 时，重定向 URL 中的端口将是 `X-Forwarded-Port` 请求头的值或服务器的端口。
+* 当开启 `http_to_https` 时，重定向 URL 中的端口将按如下顺序选取一个值（按优先级从高到低排列）
+ * 从配置文件中读取 `plugin_attr.redirect_https_port`。
Review Comment:
```suggestion
* 从配置文件（`conf/config.yaml`）中读取 `plugin_attr.redirect_https_port`。
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
spacewander commented on PR #7065:
URL: https://github.com/apache/apisix/pull/7065#issuecomment-1129523957
Let's merge master to make CI pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r874860925
##########
docs/zh/latest/plugins/redirect.md:
##########
@@ -45,7 +45,10 @@ description: 本文介绍了关于 Apache APISIX `redirect` 插件的基本信
`http_to_https`、`uri` 和 `regex_uri` 只能配置其中一个属性。
-* 当开启 `http_to_https` 时，重定向 URL 中的端口将是 `X-Forwarded-Port` 请求头的值或服务器的端口。
+* 当开启 `http_to_https` 时，重定向 URL 中的端口将按如下顺序选取一个值（按优先级从高到低排列）
+ * 从配置文件（`conf/config.yaml`）中读取 `plugin_attr.redirect_https_port`。
Review Comment:
ditto
##########
docs/en/latest/plugins/redirect.md:
##########
@@ -45,7 +45,10 @@ The `redirect` Plugin can be used to configure redirects.
Only one of `http_to_https`, `uri` and `regex_uri` can be configured.
-* When enabling `http_to_https`, the port in the redirect URL will be the value of header `X-Forwarded-Port` or the port of the server.
+* 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`).
Review Comment:
redirect_https_port should be updated
##########
t/plugin/redirect.t:
##########
@@ -428,59 +428,83 @@ passed
-=== TEST 18: redirect
+=== TEST 18: redirect(port using `plugin_attr.redirect_https_port`)
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r874920709
##########
apisix/plugins/redirect.lua:
##########
@@ -148,7 +150,32 @@ 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 = tonumber(ctx.var["var_x_forwarded_port"])
+ local ret_port = nil
+ local attr = plugin.plugin_attr("redirect")
Review Comment:
```suggestion
local attr = plugin.plugin_attr(plugin_name)
```
##########
apisix/plugins/redirect.lua:
##########
@@ -148,7 +150,32 @@ 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 = tonumber(ctx.var["var_x_forwarded_port"])
+ local ret_port = nil
+ local attr = plugin.plugin_attr("redirect")
+ if attr then
+ ret_port = attr.https_port
+ end
+
+ if not ret_port then
+ local local_conf = core.config.local_conf()
+ local ssl = core.table.try_read_attr(local_conf,
+ "apisix",
+ "ssl")
Review Comment:
```suggestion
local ssl = core.table.try_read_attr(local_conf, "apisix", "ssl")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
soulbird commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r875397182
##########
apisix/plugins/redirect.lua:
##########
@@ -148,7 +150,30 @@ 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 = tonumber(ctx.var["var_x_forwarded_port"])
+ local ret_port = nil
+ local attr = plugin.plugin_attr(plugin_name)
+ if attr then
+ ret_port = attr.https_port
+ end
+
+ if not ret_port then
+ local local_conf = core.config.local_conf()
+ local ssl = core.table.try_read_attr(local_conf, "apisix", "ssl")
+ if ssl and ssl["enable"] then
+ ret_port = ssl["listen_port"]
+ if not ret_port then
+ local ret_ports = ssl["listen"]
+ if ret_ports and #ret_ports > 0 then
+ local idx = math_random(1, #ret_ports)
+ ret_port = ret_ports[idx]
+ if type(ret_port) == "table" then
+ ret_port = ret_port.port
+ end
+ end
+ end
+ end
+ end
+
Review Comment:
Personally, I feel that this code is too inelegant. Suggest:
```lua
local function get_port(attr)
local port
if attr then
port = attr.https.port
end
if port then
return port
end
local local_conf = core.config.local_conf()
local ssl = core.table.try_read_attr(local_conf, "apisix", "ssl")
if not ssl or ssl["enable"] then
return port
end
port = ssl["listen_port"]
if port then
return port
end
local ports = ssl["listen"]
if ports and #ports > 0 then
local idx = math_random(1, #ports)
port = ret_ports[idx]
if type(port) == "table" then
port = port.port
end
end
return port
end
local ret_port = get_port(attr)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jwrookie commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
jwrookie commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r875391716
##########
docs/zh/latest/plugins/redirect.md:
##########
@@ -45,7 +45,10 @@ description: 本文介绍了关于 Apache APISIX `redirect` 插件的基本信
`http_to_https`、`uri` 和 `regex_uri` 只能配置其中一个属性。
-* 当开启 `http_to_https` 时，重定向 URL 中的端口将是 `X-Forwarded-Port` 请求头的值或服务器的端口。
+* 当开启 `http_to_https` 时，重定向 URL 中的端口将按如下顺序选取一个值（按优先级从高到低排列）
+ * 从配置文件（`conf/config.yaml`）中读取 `plugin_attr.redirect_https_port`。
Review Comment:
done
##########
t/plugin/redirect.t:
##########
@@ -428,59 +428,83 @@ passed
-=== TEST 18: redirect
+=== TEST 18: redirect(port using `plugin_attr.redirect_https_port`)
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jwrookie commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
jwrookie commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r875391621
##########
docs/en/latest/plugins/redirect.md:
##########
@@ -45,7 +45,10 @@ The `redirect` Plugin can be used to configure redirects.
Only one of `http_to_https`, `uri` and `regex_uri` can be configured.
-* When enabling `http_to_https`, the port in the redirect URL will be the value of header `X-Forwarded-Port` or the port of the server.
+* 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`).
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a diff in pull request #7065: fix: redirect http to https but port not change
Posted by GitBox <gi...@apache.org>.
tokers commented on code in PR #7065:
URL: https://github.com/apache/apisix/pull/7065#discussion_r875387274
##########
docs/en/latest/plugins/redirect.md:
##########
@@ -45,7 +45,10 @@ The `redirect` Plugin can be used to configure redirects.
Only one of `http_to_https`, `uri` and `regex_uri` can be configured.
-* When enabling `http_to_https`, the port in the redirect URL will be the value of header `X-Forwarded-Port` or the port of the server.
+* 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 enabling, read `apisix.ssl.listen_port` first, and if it does not exist, read `apisix.ssl.listen` and select a random port from it.
Review Comment:
```suggestion
* 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.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org