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 2020/09/25 03:55:15 UTC
[GitHub] [apisix] gy09535 opened a new pull request #2311: fix: https://github.com/apache/apisix/issues/2257
gy09535 opened a new pull request #2311:
URL: https://github.com/apache/apisix/pull/2311
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [x] Is this PR backward compatible?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r495458630
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +134,12 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ local method_name = ngx.req.get_method()
Review comment:
It is recommended to put the following link in the comments for easy reading:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494855035
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
Review comment:
I think it would be better to use whitelist (`GET` OR `HEAD`) instead.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liuhengloveyou commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
liuhengloveyou commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494811432
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
User's configuration priority
`
if not ret_code or ret_code == 302 then
if ngx.req.get_method() == "POST" then
ret_code = 308
else
ret_code = 301
end
end
`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gy09535 commented on pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
gy09535 commented on pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#issuecomment-698845720
If have times, please help review another PR, https://github.com/apache/apisix/pull/2177 @spacewander @liuhengloveyou
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494855035
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
Review comment:
I think it would be better to use whitelist (`GET` OR `HEAD`) instead.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gy09535 commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
gy09535 commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494867847
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
I think when you choice http to https, you can not use user ret_code ,because http to https is an features, it means you set the redirect 301 but how handle post method , you can set 308, when the http is become https ,the user code is effect. what do you think @liuhengloveyou ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liuhengloveyou commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
liuhengloveyou commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494811432
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
User's configuration priority
`
if not ret_code or ret_code == 302 then
if ngx.req.get_method() == "POST" then
ret_code = 308
else
ret_code = 301
end
end
`
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
User's configuration priority
`
if not ret_code or ret_code == 302 then
if ngx.req.get_method() == "POST" then
ret_code = 308
else
ret_code = 301
end
end
`
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
User's configuration priority
`
if not ret_code or ret_code == 302 then
if ngx.req.get_method() == "POST" then
ret_code = 308
else
ret_code = 301
end
end
`
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
User's configuration priority
`
if not ret_code or ret_code == 302 then
if ngx.req.get_method() == "POST" then
ret_code = 308
else
ret_code = 301
end
end
`
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
The user's configuration should take precedence. like this:
if not ret_code or ret_code == 302 then
if ngx.req.get_method() == "POST" then
ret_code = 308
else
ret_code = 301
end
end
##########
File path: t/plugin/redirect.t
##########
@@ -676,3 +676,51 @@ close: 1 nil}
--- no_error_log
[error]
[alert]
+
+
+
+=== TEST 26: add plugin with new uri: /test/add
+--- 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,
+ [[{
+ "plugins": {
+ "redirect": {
+ "http_to_https": true,
+ "ret_code": 307
+ }
+ },
+ "methods":["POST"],
+ "host": "test.com",
+ "uri": "/hello-https"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 27: redirect
+--- request
+POST /hello-https
+--- more_headers
+Host: test.com
+--- response_headers
+Location: https://test.com/hello-https
+--- error_code: 308
+--- no_error_log
+[error]
Review comment:
When the user has a configuration, the value of the user configuration should be returned
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
I think,,,user can choose not to configure it。If the user's configuration is wrong, should get the wrong result，
not be ignored。
The role of the user's configuration should be very definite.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gy09535 commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
gy09535 commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494867847
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
I think when you choice http to https, you can not use user ret_code ,because http to https is an features, it means you set the redirect 301 but how handle post method , you can set 308, when the http is become https ,the user code is effect. I think is the reason why @membphis use 301 and not us user ret_code , what do you think @liuhengloveyou ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liuhengloveyou commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
liuhengloveyou commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494814868
##########
File path: t/plugin/redirect.t
##########
@@ -676,3 +676,51 @@ close: 1 nil}
--- no_error_log
[error]
[alert]
+
+
+
+=== TEST 26: add plugin with new uri: /test/add
+--- 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,
+ [[{
+ "plugins": {
+ "redirect": {
+ "http_to_https": true,
+ "ret_code": 307
+ }
+ },
+ "methods":["POST"],
+ "host": "test.com",
+ "uri": "/hello-https"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 27: redirect
+--- request
+POST /hello-https
+--- more_headers
+Host: test.com
+--- response_headers
+Location: https://test.com/hello-https
+--- error_code: 308
+--- no_error_log
+[error]
Review comment:
When the user has a configuration, the value of the user configuration should be returned
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liuhengloveyou commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
liuhengloveyou commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494811432
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
User's configuration priority
`
if not ret_code or ret_code == 302 then
if ngx.req.get_method() == "POST" then
ret_code = 308
else
ret_code = 301
end
end
`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liuhengloveyou commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
liuhengloveyou commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494811432
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
User's configuration priority
`
if not ret_code or ret_code == 302 then
if ngx.req.get_method() == "POST" then
ret_code = 308
else
ret_code = 301
end
end
`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gy09535 commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
gy09535 commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r495037342
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
The http to https is a feature and the ret_code is other feature, when apisix receive http request should change it to https, when apisix receive https we should redirect it to user ret_code, they are not conflict.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liuhengloveyou commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
liuhengloveyou commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494811432
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
The user's configuration should take precedence. like this:
if not ret_code or ret_code == 302 then
if ngx.req.get_method() == "POST" then
ret_code = 308
else
ret_code = 301
end
end
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gy09535 commented on pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
gy09535 commented on pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#issuecomment-698845720
If have times, please help review another PR, https://github.com/apache/apisix/pull/2177 @spacewander @liuhengloveyou
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liuhengloveyou commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
liuhengloveyou commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494889826
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
I think,,,user can choose not to configure it。If the user's configuration is wrong, should get the wrong result，
not be ignored。
The role of the user's configuration should be very definite.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r495361793
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +134,12 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ local method_name = ngx.req.get_method()
+ if method_name == "GET" or method_name == "HEAD" then
+ ret_code = 301
Review comment:
need add test cases for these codes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gy09535 commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
gy09535 commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494870587
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
Review comment:
sure
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gy09535 commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
gy09535 commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494867847
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
I think when you choice http to https, you can not use user ret_code ,because http to https is an features, it means you set the redirect 301 but how handle post method , you can set 308, when the http is become https ,the user code is effect. I think is the reason why @membphis use 301 and not use user ret_code , what do you think @liuhengloveyou ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liuhengloveyou commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
liuhengloveyou commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494811432
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
User's configuration priority
`
if not ret_code or ret_code == 302 then
if ngx.req.get_method() == "POST" then
ret_code = 308
else
ret_code = 301
end
end
`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gy09535 commented on a change in pull request #2311: fix: redirect is not correct
Posted by GitBox <gi...@apache.org>.
gy09535 commented on a change in pull request #2311:
URL: https://github.com/apache/apisix/pull/2311#discussion_r494867847
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
I think when you choice http to https, you can not use user ret_code ,because http to https is an features, it means you set the redirect 301 but how handle post method , you can set 308, when the http is become https ,the user code is effect. what do you think @liuhengloveyou ?
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
I think when you choice http to https, you can not use user ret_code ,because http to https is an features, it means you set the redirect 301 but how handle post method , you can set 308, when the http is become https ,the user code is effect. I think is the reason why @membphis use 301 and not us user ret_code , what do you think @liuhengloveyou ?
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
+ ret_code = 308
+ else
+ ret_code = 301
+ end
Review comment:
I think when you choice http to https, you can not use user ret_code ,because http to https is an features, it means you set the redirect 301 but how handle post method , you can set 308, when the http is become https ,the user code is effect. I think is the reason why @membphis use 301 and not use user ret_code , what do you think @liuhengloveyou ?
##########
File path: apisix/plugins/redirect.lua
##########
@@ -133,7 +135,11 @@ function _M.rewrite(conf, ctx)
-- TODO： add test case
-- PR: https://github.com/apache/apisix/pull/1958
uri = "https://$host$request_uri"
- ret_code = 301
+ if ngx.req.get_method() == http_post then
Review comment:
sure
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming merged pull request #2311: bugfix: return right response code when `http_to_https` enabled in redirect plugin.
Posted by GitBox <gi...@apache.org>.
moonming merged pull request #2311:
URL: https://github.com/apache/apisix/pull/2311
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org