From 9617d412eef574a9152fbf85fe0c6fd311ec3179 Mon Sep 17 00:00:00 2001 From: Wei Jiang Date: Tue, 17 May 2022 17:55:11 +0800 Subject: [PATCH 01/10] fix: redirect http to https but port not change Signed-off-by: Wei Jiang --- apisix/plugins/redirect.lua | 28 +++++++- conf/config-default.yaml | 1 + docs/en/latest/plugins/redirect.md | 5 +- docs/zh/latest/plugins/redirect.md | 5 +- t/plugin/redirect.t | 110 ++++++++--------------------- 5 files changed, 66 insertions(+), 83 deletions(-) diff --git a/apisix/plugins/redirect.lua b/apisix/plugins/redirect.lua index 104cf9d45d48..99d78188a562 100644 --- a/apisix/plugins/redirect.lua +++ b/apisix/plugins/redirect.lua @@ -24,7 +24,8 @@ local ipairs = ipairs local ngx = ngx local str_find = core.string.find local str_sub = string.sub -local tonumber = tonumber +local type = type +local math_random = math.random local lrucache = core.lrucache.new({ ttl = 300, count = 100 @@ -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, + "plugin_attr", + "redirect_https_port") + + if not ret_port then + 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 + local uri = conf.uri local regex_uri = conf.regex_uri diff --git a/conf/config-default.yaml b/conf/config-default.yaml index bdf445545037..747fbe11347c 100644 --- a/conf/config-default.yaml +++ b/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 diff --git a/docs/en/latest/plugins/redirect.md b/docs/en/latest/plugins/redirect.md index 865ef602d3e6..a3fcd38c82cc 100644 --- a/docs/en/latest/plugins/redirect.md +++ b/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. + * 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. + * Use 443 as the default https port. ::: diff --git a/docs/zh/latest/plugins/redirect.md b/docs/zh/latest/plugins/redirect.md index 75ece0243e4e..674332fba154 100644 --- a/docs/zh/latest/plugins/redirect.md +++ b/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`。 + * 如果 `apisix.ssl` 处于开启状态，先读取 `apisix.ssl.listen_port`，如果没有，再读取 `apisix.ssl.listen` 并从中随机选一个 `port`。 + * 使用 443 作为默认 `https port`。 ::: diff --git a/t/plugin/redirect.t b/t/plugin/redirect.t index 47479a2b468e..2312e65d5908 100644 --- a/t/plugin/redirect.t +++ b/t/plugin/redirect.t @@ -435,59 +435,11 @@ GET /hello 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) ---- 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 - - - -=== TEST 21: redirect(pass number more than 65535 to x-forwarded-port) ---- request -GET /hello ---- more_headers -Host: foo.com -x-forwarded-port: 65536 ---- error_code: 301 ---- response_headers -Location: https://foo.com/hello - - - -=== TEST 22: redirect(pass invalid non-number to x-forwarded-port) ---- request -GET /hello ---- more_headers -Host: foo.com -x-forwarded-port: ok ---- error_code: 301 ---- response_headers -Location: https://foo.com/hello - - - -=== TEST 23: enable http_to_https with ret_code(not take effect) +=== TEST 19: enable http_to_https with ret_code(not take effect) --- config location /t { content_by_lua_block { @@ -521,18 +473,18 @@ passed -=== TEST 24: redirect +=== TEST 20: redirect --- 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 25: wrong configure, enable http_to_https with uri +=== TEST 21: wrong configure, enable http_to_https with uri --- config location /t { content_by_lua_block { @@ -567,7 +519,7 @@ qr/error_msg":"failed to check the configuration of plugin redirect err: value s -=== TEST 26: enable http_to_https with upstream +=== TEST 22: enable http_to_https with upstream --- config location /t { content_by_lua_block { @@ -606,18 +558,18 @@ passed -=== TEST 27: redirect +=== TEST 23: redirect --- request GET /hello --- more_headers Host: test.com --- error_code: 301 --- response_headers -Location: https://test.com:1984/hello +Location: https://test.com:8443/hello -=== TEST 28: set ssl(sni: test.com) +=== TEST 24: set ssl(sni: test.com) --- config location /t { content_by_lua_block { @@ -648,7 +600,7 @@ passed -=== TEST 29: client https request +=== TEST 25: client https request --- config listen unix:$TEST_NGINX_HTML_DIR/nginx.sock ssl; @@ -722,7 +674,7 @@ close: 1 nil} -=== TEST 30: add plugin with new uri: /test/add +=== TEST 26: add plugin with new uri: /test/add --- config location /t { content_by_lua_block { @@ -757,46 +709,46 @@ passed -=== TEST 31: http to https post redirect +=== TEST 27: http to https post redirect --- request POST /hello-https --- more_headers Host: test.com --- response_headers -Location: https://test.com:1984/hello-https +Location: https://test.com:8443/hello-https --- error_code: 308 --- no_error_log [error] -=== TEST 32: http to https get redirect +=== TEST 28: http to https get redirect --- request GET /hello-https --- more_headers Host: test.com --- response_headers -Location: https://test.com:1984/hello-https +Location: https://test.com:8443/hello-https --- error_code: 301 --- no_error_log [error] -=== TEST 33: http to https head redirect +=== TEST 29: http to https head redirect --- request HEAD /hello-https --- more_headers Host: test.com --- response_headers -Location: https://test.com:1984/hello-https +Location: https://test.com:8443/hello-https --- error_code: 301 --- no_error_log [error] -=== TEST 34: add plugin with new regex_uri: /test/1 redirect to http://test.com/1 +=== TEST 30: add plugin with new regex_uri: /test/1 redirect to http://test.com/1 --- config location /t { content_by_lua_block { @@ -835,7 +787,7 @@ passed -=== TEST 35: regex_uri redirect +=== TEST 31: regex_uri redirect --- request GET /test/1 --- response_headers @@ -846,7 +798,7 @@ Location: http://test.com/1 -=== TEST 36: regex_uri not match, get response from upstream +=== TEST 32: regex_uri not match, get response from upstream --- request GET /hello --- error_code: 200 @@ -857,7 +809,7 @@ hello world -=== TEST 37: add plugin with new regex_uri: encode_uri = true +=== TEST 33: add plugin with new regex_uri: encode_uri = true --- config location /t { content_by_lua_block { @@ -897,7 +849,7 @@ passed -=== TEST 38: regex_uri redirect with special characters +=== TEST 34: regex_uri redirect with special characters --- request GET /test/with%20space --- error_code: 200 @@ -909,7 +861,7 @@ Location: http://test.com/with%20space -=== TEST 39: add plugin with new uri: encode_uri = true +=== TEST 35: add plugin with new uri: encode_uri = true --- config location /t { content_by_lua_block { @@ -943,7 +895,7 @@ passed -=== TEST 40: redirect with special characters +=== TEST 36: redirect with special characters --- request GET /hello/with%20space --- response_headers @@ -954,7 +906,7 @@ Location: /hello/with%20space -=== TEST 41: add plugin with new uri: $uri (append_query_string = true) +=== TEST 37: add plugin with new uri: $uri (append_query_string = true) --- config location /t { content_by_lua_block { @@ -988,7 +940,7 @@ passed -=== TEST 42: redirect +=== TEST 38: redirect --- request GET /hello?name=json --- response_headers @@ -999,7 +951,7 @@ Location: /hello?name=json -=== TEST 43: add plugin with new uri: $uri?type=string (append_query_string = true) +=== TEST 39: add plugin with new uri: $uri?type=string (append_query_string = true) --- config location /t { content_by_lua_block { @@ -1033,7 +985,7 @@ passed -=== TEST 44: redirect +=== TEST 40: redirect --- request GET /hello?name=json --- response_headers @@ -1044,7 +996,7 @@ Location: /hello?type=string&name=json -=== TEST 45: enable http_to_https (pass X-Forwarded-Proto) +=== TEST 41: enable http_to_https (pass X-Forwarded-Proto) --- config location /t { content_by_lua_block { @@ -1084,7 +1036,7 @@ passed -=== TEST 46: enable http_to_https (pass X-Forwarded-Proto) +=== TEST 42: enable http_to_https (pass X-Forwarded-Proto) --- request GET /hello --- more_headers @@ -1092,4 +1044,4 @@ Host: foo.com X-Forwarded-Proto: http --- error_code: 301 --- response_headers -Location: https://foo.com:1984/hello +Location: https://foo.com:8443/hello From 096129bf2dd5974bfafb2e5afcbe07abeeb80abc Mon Sep 17 00:00:00 2001 From: Wei Jiang Date: Tue, 17 May 2022 18:51:27 +0800 Subject: [PATCH 02/10] fix: redirect http to https but port not change Signed-off-by: Wei Jiang --- t/plugin/redirect3.t | 107 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 t/plugin/redirect3.t diff --git a/t/plugin/redirect3.t b/t/plugin/redirect3.t new file mode 100644 index 000000000000..2a24d68d38ae --- /dev/null +++ b/t/plugin/redirect3.t @@ -0,0 +1,107 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +use t::APISIX 'no_plan'; + +repeat_each(1); +no_long_string(); +no_root_location(); + +run_tests(); + +__DATA__ + +=== TEST 1: enable http_to_https +--- 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 + } + } + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] + + + +=== TEST 2: 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 +--- error_code: 301 +--- response_headers +Location: https://foo.com:9445/hello + + +=== TEST 3: redirect port using `apisix.ssl.listen` +--- extra_yaml_config +plugin_attr: + redirect_https_port: null +--- request +GET /hello +--- more_headers +Host: foo.com +--- error_code: 301 +--- response_headers +Location: https://foo.com:9443/hello + + + +=== TEST 4: redirect port using `https default port` +--- yaml_config +apisix: + ssl: + enable: null +--- extra_yaml_config +plugin_attr: + redirect_https_port: null +--- request +GET /hello +--- more_headers +Host: foo.com +--- error_code: 301 +--- response_headers +Location: https://foo.com/hello From 06aa4014f198173d14b3ee27c01858a2340118aa Mon Sep 17 00:00:00 2001 From: Wei Jiang Date: Tue, 17 May 2022 19:00:18 +0800 Subject: [PATCH 03/10] fix: redirect http to https but port not change Signed-off-by: Wei Jiang --- t/plugin/redirect.t | 101 ++++++++++++++++++++++++++++++---------- t/plugin/redirect3.t | 107 ------------------------------------------- 2 files changed, 76 insertions(+), 132 deletions(-) delete mode 100644 t/plugin/redirect3.t diff --git a/t/plugin/redirect.t b/t/plugin/redirect.t index 2312e65d5908..ca4a6dee9c5f 100644 --- a/t/plugin/redirect.t +++ b/t/plugin/redirect.t @@ -428,7 +428,7 @@ passed -=== TEST 18: redirect +=== TEST 18: redirect(port using `plugin_attr.redirect_https_port`) --- request GET /hello --- more_headers @@ -439,7 +439,58 @@ Location: https://foo.com:8443/hello -=== TEST 19: enable http_to_https with ret_code(not take effect) +=== 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 +--- error_code: 301 +--- response_headers +Location: https://foo.com:9445/hello + + + +=== 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 +--- error_code: 301 +--- response_headers +Location: https://foo.com:9443/hello + + + +=== TEST 21: redirect(port using `https default port`) +--- yaml_config +apisix: + ssl: + enable: null +--- extra_yaml_config +plugin_attr: + redirect_https_port: null +--- request +GET /hello +--- more_headers +Host: foo.com +--- error_code: 301 +--- response_headers +Location: https://foo.com/hello + + + +=== TEST 22: enable http_to_https with ret_code(not take effect) --- config location /t { content_by_lua_block { @@ -473,7 +524,7 @@ passed -=== TEST 20: redirect +=== TEST 23: redirect --- request GET /hello --- more_headers @@ -484,7 +535,7 @@ Location: https://foo.com:8443/hello -=== TEST 21: wrong configure, enable http_to_https with uri +=== TEST 24: wrong configure, enable http_to_https with uri --- config location /t { content_by_lua_block { @@ -519,7 +570,7 @@ qr/error_msg":"failed to check the configuration of plugin redirect err: value s -=== TEST 22: enable http_to_https with upstream +=== TEST 25: enable http_to_https with upstream --- config location /t { content_by_lua_block { @@ -558,7 +609,7 @@ passed -=== TEST 23: redirect +=== TEST 26: redirect --- request GET /hello --- more_headers @@ -569,7 +620,7 @@ Location: https://test.com:8443/hello -=== TEST 24: set ssl(sni: test.com) +=== TEST 27: set ssl(sni: test.com) --- config location /t { content_by_lua_block { @@ -600,7 +651,7 @@ passed -=== TEST 25: client https request +=== TEST 28: client https request --- config listen unix:$TEST_NGINX_HTML_DIR/nginx.sock ssl; @@ -674,7 +725,7 @@ close: 1 nil} -=== TEST 26: add plugin with new uri: /test/add +=== TEST 29: add plugin with new uri: /test/add --- config location /t { content_by_lua_block { @@ -709,7 +760,7 @@ passed -=== TEST 27: http to https post redirect +=== TEST 30: http to https post redirect --- request POST /hello-https --- more_headers @@ -722,7 +773,7 @@ Location: https://test.com:8443/hello-https -=== TEST 28: http to https get redirect +=== TEST 31: http to https get redirect --- request GET /hello-https --- more_headers @@ -735,7 +786,7 @@ Location: https://test.com:8443/hello-https -=== TEST 29: http to https head redirect +=== TEST 32: http to https head redirect --- request HEAD /hello-https --- more_headers @@ -748,7 +799,7 @@ Location: https://test.com:8443/hello-https -=== TEST 30: add plugin with new regex_uri: /test/1 redirect to http://test.com/1 +=== TEST 33: add plugin with new regex_uri: /test/1 redirect to http://test.com/1 --- config location /t { content_by_lua_block { @@ -787,7 +838,7 @@ passed -=== TEST 31: regex_uri redirect +=== TEST 34: regex_uri redirect --- request GET /test/1 --- response_headers @@ -798,7 +849,7 @@ Location: http://test.com/1 -=== TEST 32: regex_uri not match, get response from upstream +=== TEST 35: regex_uri not match, get response from upstream --- request GET /hello --- error_code: 200 @@ -809,7 +860,7 @@ hello world -=== TEST 33: add plugin with new regex_uri: encode_uri = true +=== TEST 36: add plugin with new regex_uri: encode_uri = true --- config location /t { content_by_lua_block { @@ -849,7 +900,7 @@ passed -=== TEST 34: regex_uri redirect with special characters +=== TEST 37: regex_uri redirect with special characters --- request GET /test/with%20space --- error_code: 200 @@ -861,7 +912,7 @@ Location: http://test.com/with%20space -=== TEST 35: add plugin with new uri: encode_uri = true +=== TEST 38: add plugin with new uri: encode_uri = true --- config location /t { content_by_lua_block { @@ -895,7 +946,7 @@ passed -=== TEST 36: redirect with special characters +=== TEST 39: redirect with special characters --- request GET /hello/with%20space --- response_headers @@ -906,7 +957,7 @@ Location: /hello/with%20space -=== TEST 37: add plugin with new uri: $uri (append_query_string = true) +=== TEST 40: add plugin with new uri: $uri (append_query_string = true) --- config location /t { content_by_lua_block { @@ -940,7 +991,7 @@ passed -=== TEST 38: redirect +=== TEST 41: redirect --- request GET /hello?name=json --- response_headers @@ -951,7 +1002,7 @@ Location: /hello?name=json -=== TEST 39: add plugin with new uri: $uri?type=string (append_query_string = true) +=== TEST 42: add plugin with new uri: $uri?type=string (append_query_string = true) --- config location /t { content_by_lua_block { @@ -985,7 +1036,7 @@ passed -=== TEST 40: redirect +=== TEST 43: redirect --- request GET /hello?name=json --- response_headers @@ -996,7 +1047,7 @@ Location: /hello?type=string&name=json -=== TEST 41: enable http_to_https (pass X-Forwarded-Proto) +=== TEST 44: enable http_to_https (pass X-Forwarded-Proto) --- config location /t { content_by_lua_block { @@ -1036,7 +1087,7 @@ passed -=== TEST 42: enable http_to_https (pass X-Forwarded-Proto) +=== TEST 45: enable http_to_https (pass X-Forwarded-Proto) --- request GET /hello --- more_headers diff --git a/t/plugin/redirect3.t b/t/plugin/redirect3.t deleted file mode 100644 index 2a24d68d38ae..000000000000 --- a/t/plugin/redirect3.t +++ /dev/null @@ -1,107 +0,0 @@ -# -# Licensed to the Apache Software Foundation (ASF) under one or more -# contributor license agreements. See the NOTICE file distributed with -# this work for additional information regarding copyright ownership. -# The ASF licenses this file to You under the Apache License, Version 2.0 -# (the "License"); you may not use this file except in compliance with -# the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# - -use t::APISIX 'no_plan'; - -repeat_each(1); -no_long_string(); -no_root_location(); - -run_tests(); - -__DATA__ - -=== TEST 1: enable http_to_https ---- 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 - } - } - }]] - ) - - if code >= 300 then - ngx.status = code - end - ngx.say(body) - } - } ---- request -GET /t ---- response_body -passed ---- no_error_log -[error] - - - -=== TEST 2: 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 ---- error_code: 301 ---- response_headers -Location: https://foo.com:9445/hello - - -=== TEST 3: redirect port using `apisix.ssl.listen` ---- extra_yaml_config -plugin_attr: - redirect_https_port: null ---- request -GET /hello ---- more_headers -Host: foo.com ---- error_code: 301 ---- response_headers -Location: https://foo.com:9443/hello - - - -=== TEST 4: redirect port using `https default port` ---- yaml_config -apisix: - ssl: - enable: null ---- extra_yaml_config -plugin_attr: - redirect_https_port: null ---- request -GET /hello ---- more_headers -Host: foo.com ---- error_code: 301 ---- response_headers -Location: https://foo.com/hello From 9b159e1767f28ff8f80561bb81c09a82ad044399 Mon Sep 17 00:00:00 2001 From: LetsGO Date: Tue, 17 May 2022 20:53:46 +0800 Subject: [PATCH 04/10] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: 罗泽轩 --- docs/en/latest/plugins/redirect.md | 2 +- docs/zh/latest/plugins/redirect.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/en/latest/plugins/redirect.md b/docs/en/latest/plugins/redirect.md index a3fcd38c82cc..2a2721bc4b5d 100644 --- a/docs/en/latest/plugins/redirect.md +++ b/docs/en/latest/plugins/redirect.md @@ -46,7 +46,7 @@ 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 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. + * 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. * Use 443 as the default https port. diff --git a/docs/zh/latest/plugins/redirect.md b/docs/zh/latest/plugins/redirect.md index 674332fba154..2cdd3bf410cb 100644 --- a/docs/zh/latest/plugins/redirect.md +++ b/docs/zh/latest/plugins/redirect.md @@ -46,7 +46,7 @@ description: 本文介绍了关于 Apache APISIX `redirect` 插件的基本信 `http_to_https`、`uri` 和 `regex_uri` 只能配置其中一个属性。 * 当开启 `http_to_https` 时，重定向 URL 中的端口将按如下顺序选取一个值（按优先级从高到低排列） - * 从配置文件中读取 `plugin_attr.redirect_https_port`。 + * 从配置文件（`conf/config.yaml`）中读取 `plugin_attr.redirect_https_port`。 * 如果 `apisix.ssl` 处于开启状态，先读取 `apisix.ssl.listen_port`，如果没有，再读取 `apisix.ssl.listen` 并从中随机选一个 `port`。 * 使用 443 作为默认 `https port`。 From c8d2f5506f7f365156fbe19a4a3fb5447c24a3c3 Mon Sep 17 00:00:00 2001 From: Wei Jiang Date: Tue, 17 May 2022 21:10:21 +0800 Subject: [PATCH 05/10] fix: review Signed-off-by: Wei Jiang --- apisix/plugins/redirect.lua | 11 +++-- conf/config-default.yaml | 3 +- t/plugin/redirect.t | 93 +++++++++++++++++++++++-------------- 3 files changed, 66 insertions(+), 41 deletions(-) diff --git a/apisix/plugins/redirect.lua b/apisix/plugins/redirect.lua index 99d78188a562..03681991375e 100644 --- a/apisix/plugins/redirect.lua +++ b/apisix/plugins/redirect.lua @@ -15,6 +15,7 @@ -- limitations under the License. -- local core = require("apisix.core") +local plugin = require("apisix.plugin") local tab_insert = table.insert local tab_concat = table.concat local string_format = string.format @@ -149,12 +150,14 @@ function _M.rewrite(conf, ctx) core.log.info("plugin rewrite phase, conf: ", core.json.delay_encode(conf)) local ret_code = conf.ret_code - local local_conf = core.config.local_conf() - local ret_port = core.table.try_read_attr(local_conf, - "plugin_attr", - "redirect_https_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") diff --git a/conf/config-default.yaml b/conf/config-default.yaml index 747fbe11347c..f60c6577fc79 100644 --- a/conf/config-default.yaml +++ b/conf/config-default.yaml @@ -470,4 +470,5 @@ plugin_attr: connect: 60s read: 60s send: 60s - redirect_https_port: 8443 # the default port for use by HTTP redirects to HTTPS +# redirect: +# https_port: 8443 # the default port for use by HTTP redirects to HTTPS diff --git a/t/plugin/redirect.t b/t/plugin/redirect.t index ca4a6dee9c5f..1719bc99418f 100644 --- a/t/plugin/redirect.t +++ b/t/plugin/redirect.t @@ -429,6 +429,10 @@ passed === TEST 18: redirect(port using `plugin_attr.redirect_https_port`) +--- extra_yaml_config +plugin_attr: + redirect: + https_port: 8443 --- request GET /hello --- more_headers @@ -458,10 +462,7 @@ Location: https://foo.com:9445/hello -=== TEST 20: redirect(port using `apisix.ssl.listen`) ---- extra_yaml_config -plugin_attr: - redirect_https_port: null +=== TEST 20: redirect(port using `apisix.ssl.listen` when listen length is one) --- request GET /hello --- more_headers @@ -472,14 +473,34 @@ Location: https://foo.com:9443/hello -=== TEST 21: redirect(port using `https default port`) +=== TEST 21: redirect(port using `apisix.ssl.listen` when listen length more than one) +--- yaml_config +apisix: + ssl: + enable: true + listen: + - 6443 + - 7443 + - port: 8443 + - port: 9443 +--- request +GET /hello +--- more_headers +Host: foo.com +--- error_code: 301 +--- response_headers_like +Location: https://foo.com:[6-9]443/hello + + + +=== TEST 22: redirect(port using `https default port`) --- yaml_config apisix: ssl: enable: null --- extra_yaml_config plugin_attr: - redirect_https_port: null + redirect: null --- request GET /hello --- more_headers @@ -490,7 +511,7 @@ Location: https://foo.com/hello -=== TEST 22: enable http_to_https with ret_code(not take effect) +=== TEST 23: enable http_to_https with ret_code(not take effect) --- config location /t { content_by_lua_block { @@ -524,18 +545,18 @@ passed -=== TEST 23: redirect +=== TEST 24: redirect --- request GET /hello --- more_headers Host: foo.com --- error_code: 301 --- response_headers -Location: https://foo.com:8443/hello +Location: https://foo.com:9443/hello -=== TEST 24: wrong configure, enable http_to_https with uri +=== TEST 25: wrong configure, enable http_to_https with uri --- config location /t { content_by_lua_block { @@ -570,7 +591,7 @@ qr/error_msg":"failed to check the configuration of plugin redirect err: value s -=== TEST 25: enable http_to_https with upstream +=== TEST 26: enable http_to_https with upstream --- config location /t { content_by_lua_block { @@ -609,18 +630,18 @@ passed -=== TEST 26: redirect +=== TEST 27: redirect --- request GET /hello --- more_headers Host: test.com --- error_code: 301 --- response_headers -Location: https://test.com:8443/hello +Location: https://test.com:9443/hello -=== TEST 27: set ssl(sni: test.com) +=== TEST 28: set ssl(sni: test.com) --- config location /t { content_by_lua_block { @@ -651,7 +672,7 @@ passed -=== TEST 28: client https request +=== TEST 29: client https request --- config listen unix:$TEST_NGINX_HTML_DIR/nginx.sock ssl; @@ -725,7 +746,7 @@ close: 1 nil} -=== TEST 29: add plugin with new uri: /test/add +=== TEST 30: add plugin with new uri: /test/add --- config location /t { content_by_lua_block { @@ -760,46 +781,46 @@ passed -=== TEST 30: http to https post redirect +=== TEST 31: http to https post redirect --- request POST /hello-https --- more_headers Host: test.com --- response_headers -Location: https://test.com:8443/hello-https +Location: https://test.com:9443/hello-https --- error_code: 308 --- no_error_log [error] -=== TEST 31: http to https get redirect +=== TEST 32: http to https get redirect --- request GET /hello-https --- more_headers Host: test.com --- response_headers -Location: https://test.com:8443/hello-https +Location: https://test.com:9443/hello-https --- error_code: 301 --- no_error_log [error] -=== TEST 32: http to https head redirect +=== TEST 33: http to https head redirect --- request HEAD /hello-https --- more_headers Host: test.com --- response_headers -Location: https://test.com:8443/hello-https +Location: https://test.com:9443/hello-https --- error_code: 301 --- no_error_log [error] -=== TEST 33: add plugin with new regex_uri: /test/1 redirect to http://test.com/1 +=== TEST 34: add plugin with new regex_uri: /test/1 redirect to http://test.com/1 --- config location /t { content_by_lua_block { @@ -838,7 +859,7 @@ passed -=== TEST 34: regex_uri redirect +=== TEST 35: regex_uri redirect --- request GET /test/1 --- response_headers @@ -849,7 +870,7 @@ Location: http://test.com/1 -=== TEST 35: regex_uri not match, get response from upstream +=== TEST 36: regex_uri not match, get response from upstream --- request GET /hello --- error_code: 200 @@ -860,7 +881,7 @@ hello world -=== TEST 36: add plugin with new regex_uri: encode_uri = true +=== TEST 37: add plugin with new regex_uri: encode_uri = true --- config location /t { content_by_lua_block { @@ -900,7 +921,7 @@ passed -=== TEST 37: regex_uri redirect with special characters +=== TEST 38: regex_uri redirect with special characters --- request GET /test/with%20space --- error_code: 200 @@ -912,7 +933,7 @@ Location: http://test.com/with%20space -=== TEST 38: add plugin with new uri: encode_uri = true +=== TEST 39: add plugin with new uri: encode_uri = true --- config location /t { content_by_lua_block { @@ -946,7 +967,7 @@ passed -=== TEST 39: redirect with special characters +=== TEST 40: redirect with special characters --- request GET /hello/with%20space --- response_headers @@ -957,7 +978,7 @@ Location: /hello/with%20space -=== TEST 40: add plugin with new uri: $uri (append_query_string = true) +=== TEST 41: add plugin with new uri: $uri (append_query_string = true) --- config location /t { content_by_lua_block { @@ -991,7 +1012,7 @@ passed -=== TEST 41: redirect +=== TEST 42: redirect --- request GET /hello?name=json --- response_headers @@ -1002,7 +1023,7 @@ Location: /hello?name=json -=== TEST 42: add plugin with new uri: $uri?type=string (append_query_string = true) +=== TEST 43: add plugin with new uri: $uri?type=string (append_query_string = true) --- config location /t { content_by_lua_block { @@ -1036,7 +1057,7 @@ passed -=== TEST 43: redirect +=== TEST 44: redirect --- request GET /hello?name=json --- response_headers @@ -1047,7 +1068,7 @@ Location: /hello?type=string&name=json -=== TEST 44: enable http_to_https (pass X-Forwarded-Proto) +=== TEST 45: enable http_to_https (pass X-Forwarded-Proto) --- config location /t { content_by_lua_block { @@ -1087,7 +1108,7 @@ passed -=== TEST 45: enable http_to_https (pass X-Forwarded-Proto) +=== TEST 46: enable http_to_https (pass X-Forwarded-Proto) --- request GET /hello --- more_headers @@ -1095,4 +1116,4 @@ Host: foo.com X-Forwarded-Proto: http --- error_code: 301 --- response_headers -Location: https://foo.com:8443/hello +Location: https://foo.com:9443/hello From 6b38bfeb41176379fe1fe9b0ecdd25824aeb9a28 Mon Sep 17 00:00:00 2001 From: LetsGO Date: Wed, 18 May 2022 09:19:39 +0800 Subject: [PATCH 06/10] Apply suggestions from code review Co-authored-by: tzssangglass Co-authored-by: Alex Zhang --- apisix/plugins/redirect.lua | 4 +--- docs/en/latest/plugins/redirect.md | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/apisix/plugins/redirect.lua b/apisix/plugins/redirect.lua index 03681991375e..4ea06bfca6b6 100644 --- a/apisix/plugins/redirect.lua +++ b/apisix/plugins/redirect.lua @@ -158,9 +158,7 @@ function _M.rewrite(conf, ctx) if not ret_port then local local_conf = core.config.local_conf() - local ssl = core.table.try_read_attr(local_conf, - "apisix", - "ssl") + 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 diff --git a/docs/en/latest/plugins/redirect.md b/docs/en/latest/plugins/redirect.md index 2a2721bc4b5d..30721a0072c8 100644 --- a/docs/en/latest/plugins/redirect.md +++ b/docs/en/latest/plugins/redirect.md @@ -47,7 +47,7 @@ Only one of `http_to_https`, `uri` and `regex_uri` 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 enabling, read `apisix.ssl.listen_port` first, and if it does not exist, read `apisix.ssl.listen` and select a random port from it. + * 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. * Use 443 as the default https port. ::: From b9449f944ccf09be033455b49f1dae84ac0d1ffe Mon Sep 17 00:00:00 2001 From: LetsGO Date: Wed, 18 May 2022 09:25:24 +0800 Subject: [PATCH 07/10] Update apisix/plugins/redirect.lua Co-authored-by: tzssangglass --- apisix/plugins/redirect.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apisix/plugins/redirect.lua b/apisix/plugins/redirect.lua index 4ea06bfca6b6..e3bd01262705 100644 --- a/apisix/plugins/redirect.lua +++ b/apisix/plugins/redirect.lua @@ -151,7 +151,7 @@ function _M.rewrite(conf, ctx) local ret_code = conf.ret_code local ret_port = nil - local attr = plugin.plugin_attr("redirect") + local attr = plugin.plugin_attr(plugin_name) if attr then ret_port = attr.https_port end From 1ab81b771ab47e96986d1896682fc6fa01d00e9a Mon Sep 17 00:00:00 2001 From: Wei Jiang Date: Wed, 18 May 2022 09:25:31 +0800 Subject: [PATCH 08/10] fix: review Signed-off-by: Wei Jiang --- docs/en/latest/plugins/redirect.md | 2 +- docs/zh/latest/plugins/redirect.md | 2 +- t/plugin/redirect.t | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/en/latest/plugins/redirect.md b/docs/en/latest/plugins/redirect.md index 30721a0072c8..8a7a034bc934 100644 --- a/docs/en/latest/plugins/redirect.md +++ b/docs/en/latest/plugins/redirect.md @@ -46,7 +46,7 @@ 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 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`). + * 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. * Use 443 as the default https port. diff --git a/docs/zh/latest/plugins/redirect.md b/docs/zh/latest/plugins/redirect.md index 2cdd3bf410cb..276132b48aa5 100644 --- a/docs/zh/latest/plugins/redirect.md +++ b/docs/zh/latest/plugins/redirect.md @@ -46,7 +46,7 @@ description: 本文介绍了关于 Apache APISIX `redirect` 插件的基本信 `http_to_https`、`uri` 和 `regex_uri` 只能配置其中一个属性。 * 当开启 `http_to_https` 时，重定向 URL 中的端口将按如下顺序选取一个值（按优先级从高到低排列） - * 从配置文件（`conf/config.yaml`）中读取 `plugin_attr.redirect_https_port`。 + * 从配置文件（`conf/config.yaml`）中读取 `plugin_attr.redirect.https_port`。 * 如果 `apisix.ssl` 处于开启状态，先读取 `apisix.ssl.listen_port`，如果没有，再读取 `apisix.ssl.listen` 并从中随机选一个 `port`。 * 使用 443 作为默认 `https port`。 diff --git a/t/plugin/redirect.t b/t/plugin/redirect.t index 1719bc99418f..5a691ba1fcf2 100644 --- a/t/plugin/redirect.t +++ b/t/plugin/redirect.t @@ -428,7 +428,7 @@ passed -=== TEST 18: redirect(port using `plugin_attr.redirect_https_port`) +=== TEST 18: redirect(port using `plugin_attr.redirect.https_port`) --- extra_yaml_config plugin_attr: redirect: From d965ff802b9ec47609e2845ca279a20eaf233eaf Mon Sep 17 00:00:00 2001 From: Wei Jiang Date: Wed, 18 May 2022 09:40:44 +0800 Subject: [PATCH 09/10] fix: review Signed-off-by: Wei Jiang --- t/plugin/redirect.t | 3 --- 1 file changed, 3 deletions(-) diff --git a/t/plugin/redirect.t b/t/plugin/redirect.t index 5a691ba1fcf2..3b8d87afd787 100644 --- a/t/plugin/redirect.t +++ b/t/plugin/redirect.t @@ -449,9 +449,6 @@ apisix: ssl: enable: true listen_port: 9445 ---- extra_yaml_config -plugin_attr: - redirect_https_port: null --- request GET /hello --- more_headers From 215e66ba75eb03ac02c978afb912db8b4ea51c88 Mon Sep 17 00:00:00 2001 From: Wei Jiang Date: Wed, 18 May 2022 10:34:30 +0800 Subject: [PATCH 10/10] fix: review Signed-off-by: Wei Jiang --- apisix/plugins/redirect.lua | 56 ++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/apisix/plugins/redirect.lua b/apisix/plugins/redirect.lua index e3bd01262705..6c9a99a1575c 100644 --- a/apisix/plugins/redirect.lua +++ b/apisix/plugins/redirect.lua @@ -145,35 +145,47 @@ local function concat_new_uri(uri, ctx) return tab_concat(tmp, "") end +local function get_port(attr) + local port + if attr then + port = attr.https_port + end -function _M.rewrite(conf, ctx) - core.log.info("plugin rewrite phase, conf: ", core.json.delay_encode(conf)) + if port then + return port + end - local ret_code = conf.ret_code - local ret_port = nil - local attr = plugin.plugin_attr(plugin_name) - if attr then - ret_port = attr.https_port + local local_conf = core.config.local_conf() + local ssl = core.table.try_read_attr(local_conf, "apisix", "ssl") + if not ssl or not ssl["enable"] then + return 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 + 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 + +function _M.rewrite(conf, ctx) + core.log.info("plugin rewrite phase, conf: ", core.json.delay_encode(conf)) + + local ret_code = conf.ret_code + + local attr = plugin.plugin_attr(plugin_name) + local ret_port = get_port(attr) + local uri = conf.uri local regex_uri = conf.regex_uri