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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
