docs: add curl test for redirect plugin's `http_to_https`. #6378

mangoGoForward · 2022-02-18T09:28:33Z

What this PR does / why we need it:

Add curl test for redirect plugin's http_to_https, please see #3931.

Pre-submission checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

Signed-off-by: mango <xu.weiKyrie@foxmail.com>

tokers · 2022-02-19T02:23:12Z

docs/en/latest/plugins/redirect.md

+$ curl http://127.0.0.1:9080/hello -i
+HTTP/1.1 301 Moved Permanently
+...
+Location: https://127.0.0.1/hello


Since we use 9080 to accept HTTP traffic, should we use 9443 here?

Why should be 9443? and the default port of https is 443.
When http_to_https is set to true and the request is HTTP, it will be automatically redirected to HTTPS with 301 response code, and the URI will keep the same as client request.

apisix/apisix/plugins/redirect.lua

Lines 155 to 166 in 9b98f1d

if conf.http_to_https and _scheme == "http" then

-- TODO： add test case

-- PR: https://github.com/apache/apisix/pull/1958

uri = "https://$host$request_uri"

local method_name = ngx.req.get_method()

if method_name == "GET" or method_name == "HEAD" then

ret_code = 301

else

-- https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308

ret_code = 308

end

end

Why should be 9443

The default port for the APISIX proxy https protocol is 9443.

@mangoGoForward Just for the consistence of the documentation, look through the whole doc you'll find we don't use the default 80 port.

The default port for the APISIX proxy https protocol is 9443.

@tokers See the Location header of response, it redirected to https. And I found some test cases in t/plugin/redirect.t:

apisix/t/plugin/redirect.t

Lines 431 to 438 in 42e84e4

=== TEST 18: redirect

--- request

GET /hello

--- more_headers

Host: foo.com

--- error_code: 301

--- response_headers

Location: https://foo.com/hello

Seems we don't need to update?

Personally I think there is a bug, The generated Location header should respect the setting of SSL port, we cannot assume the SSL port is 443.

Also, my original meaning is quite simple, just because we use 9080 as the sample port to accept HTTP traffic in the doc, for the sake for consistence, I think using 9443 is better. If we use 80 to accept HTTP traffic, then using 443 is better.

Sorry, I don't know how to use 9443 in this test case.
Do we need create an upstream?

This PR does not need to be modified in the test case to 9443, just use 9443 in the documentation? cc @tokers

Ping @tokers

@tzssangglass is correct, just use 9443 in documentation is enough.

docs/en/latest/plugins/redirect.md

docs/zh/latest/plugins/redirect.md

Co-authored-by: tzssangglass <tzssangglass@gmail.com>

mangoGoForward and others added 3 commits February 18, 2022 17:12

docs: add curl test for redirect plugin's http_to_https

26b907a

add zh docs for redirect plugin's http_to_https.

2afefb5

Signed-off-by: mango <xu.weiKyrie@foxmail.com>

delete blank line.

bd1ab35

tokers reviewed Feb 19, 2022

View reviewed changes

spacewander requested a review from tzssangglass March 8, 2022 01:05

tzssangglass requested changes Mar 8, 2022

View reviewed changes

docs/en/latest/plugins/redirect.md Outdated Show resolved Hide resolved

tzssangglass requested changes Mar 8, 2022

View reviewed changes

docs/zh/latest/plugins/redirect.md Outdated Show resolved Hide resolved

mangoGoForward and others added 2 commits March 8, 2022 09:40

Update docs/zh/latest/plugins/redirect.md

6e5672e

Co-authored-by: tzssangglass <tzssangglass@gmail.com>

Update docs/en/latest/plugins/redirect.md

d5043fc

Co-authored-by: tzssangglass <tzssangglass@gmail.com>

starsz approved these changes Mar 8, 2022

View reviewed changes

tzssangglass approved these changes Mar 8, 2022

View reviewed changes

spacewander merged commit 1853bc8 into apache:master Mar 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: add curl test for redirect plugin's `http_to_https`. #6378

docs: add curl test for redirect plugin's `http_to_https`. #6378

	if conf.http_to_https and _scheme == "http" then
	-- TODO： add test case
	-- PR: https://github.com/apache/apisix/pull/1958
	uri = "https://$host$request_uri"
	local method_name = ngx.req.get_method()
	if method_name == "GET" or method_name == "HEAD" then
	ret_code = 301
	else
	-- https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308
	ret_code = 308
	end
	end

	=== TEST 18: redirect
	--- request
	GET /hello
	--- more_headers
	Host: foo.com
	--- error_code: 301
	--- response_headers
	Location: https://foo.com/hello

docs: add curl test for redirect plugin's http_to_https. #6378

docs: add curl test for redirect plugin's http_to_https. #6378

Conversation

What this PR does / why we need it:

Pre-submission checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

docs: add curl test for redirect plugin's `http_to_https`. #6378

docs: add curl test for redirect plugin's `http_to_https`. #6378