Skip to content

Commit

Permalink
fix: when a request caused a 500 error, the status was converted to 4…
Browse files Browse the repository at this point in the history
…05 (#4696)

It happened when this request is not a GET or HEAD request.

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
  • Loading branch information
spacewander authored Jul 29, 2021
1 parent 273b8b3 commit 6655a35
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 63 deletions.
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,3 @@ boofuzz-results/
# release tar package
*.tgz
release/*
html/*
12 changes: 9 additions & 3 deletions apisix/cli/ngx_tpl.lua
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,9 @@ http {
location @50x.html {
set $from_error_page 'true';
try_files /50x.html $uri;
content_by_lua_block {
require("apisix.error_handling").handle_500()
}
}
}
{% end %}
Expand Down Expand Up @@ -434,7 +436,9 @@ http {
location @50x.html {
set $from_error_page 'true';
try_files /50x.html $uri;
content_by_lua_block {
require("apisix.error_handling").handle_500()
}
}
}
{% end %}
Expand Down Expand Up @@ -683,7 +687,9 @@ http {
location @50x.html {
set $from_error_page 'true';
try_files /50x.html $uri;
content_by_lua_block {
require("apisix.error_handling").handle_500()
}
header_filter_by_lua_block {
apisix.http_header_filter_phase()
}
Expand Down
9 changes: 0 additions & 9 deletions apisix/cli/ops.lua
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ local etcd = require("apisix.cli.etcd")
local util = require("apisix.cli.util")
local file = require("apisix.cli.file")
local ngx_tpl = require("apisix.cli.ngx_tpl")
local html_page = require("apisix.cli.html_page")
local profile = require("apisix.core.profile")
local template = require("resty.template")
local argparse = require("argparse")
Expand Down Expand Up @@ -683,14 +682,6 @@ Please modify "admin_key" in conf/config.yaml .
if not ok then
util.die("failed to update nginx.conf: ", err, "\n")
end

local cmd_html = "mkdir -p " .. env.apisix_home .. "/html"
util.execute_cmd(cmd_html)

local ok, err = util.write_file(env.apisix_home .. "/html/50x.html", html_page)
if not ok then
util.die("failed to write 50x.html: ", err, "\n")
end
end


Expand Down
19 changes: 15 additions & 4 deletions apisix/cli/html_page.lua → apisix/error_handling.lua
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@
-- See the License for the specific language governing permissions and
-- limitations under the License.
--
local core = require("apisix.core")
local ngx = ngx

return [=[
<!DOCTYPE html>

local _M = {}
local page_500 = [[<!DOCTYPE html>
<html>
<head>
<meta content="text/html;charset=utf-8" http-equiv="Content-Type">
Expand All @@ -35,5 +38,13 @@ return [=[
<p>You can report issue to <a href="https://github.com/apache/apisix/issues">APISIX</a></p>
<p><em>Faithfully yours, <a href="https://apisix.apache.org/">APISIX</a>.</em></p>
</body>
</html>
]=]
</html>]]


function _M.handle_500()
core.response.set_header("Content-Type", "text/html")
ngx.say(page_500)
end


return _M
11 changes: 6 additions & 5 deletions t/APISIX.pm
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ my $ssl_ecc_crt = read_file("t/certs/apisix_ecc.crt");
my $ssl_ecc_key = read_file("t/certs/apisix_ecc.key");
my $test2_crt = read_file("t/certs/test2.crt");
my $test2_key = read_file("t/certs/test2.key");
my $test_50x_html = read_file("t/error_page/50x.html");
$user_yaml_config = <<_EOC_;
apisix:
node_listen: 1984
Expand Down Expand Up @@ -541,7 +540,9 @@ _EOC_
location \@50x.html {
set \$from_error_page 'true';
try_files /50x.html \$uri;
content_by_lua_block {
require("apisix.error_handling").handle_500()
}
header_filter_by_lua_block {
apisix.http_header_filter_phase()
}
Expand Down Expand Up @@ -627,7 +628,9 @@ _EOC_
location \@50x.html {
set \$from_error_page 'true';
try_files /50x.html \$uri;
content_by_lua_block {
require("apisix.error_handling").handle_500()
}
header_filter_by_lua_block {
apisix.http_header_filter_phase()
}
Expand Down Expand Up @@ -774,8 +777,6 @@ $ssl_ecc_key
$test2_crt
>>> ../conf/cert/test2.key
$test2_key
>>> 50x.html
$test_50x_html
$user_apisix_yaml
_EOC_

Expand Down
38 changes: 0 additions & 38 deletions t/error_page/50x.html

This file was deleted.

18 changes: 15 additions & 3 deletions t/error_page/error_page.t
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,19 @@ upstream status: 500



=== TEST 7: delete route(id: 1)
=== TEST 7: test apisix with internal error code 500, method isn't GET or HEAD
--- request
POST /hello
123
--- more_headers
X-Test-Status: 500
--- error_code: 500
--- response_body_like
.*apisix.apache.org.*



=== TEST 8: delete route(id: 1)
--- config
location /t {
content_by_lua_block {
Expand All @@ -142,7 +154,7 @@ GET /t



=== TEST 8: set route which upstream is blocking
=== TEST 9: set route which upstream is blocking
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -175,7 +187,7 @@ passed



=== TEST 9: client abort
=== TEST 10: client abort
--- request
GET /mysleep?seconds=3
--- abort
Expand Down

0 comments on commit 6655a35

Please sign in to comment.