Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix_error_handler_494 #12114

Merged
merged 3 commits into from
Nov 30, 2023
Merged

fix_error_handler_494 #12114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68e5189
fix(error_handler): fix the bug that error handler can't recognize
liverpool8056 Nov 29, 2023
cb2626c
add changelog
liverpool8056 Nov 29, 2023
7ee8fcd
add some comments
liverpool8056 Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/unreleased/kong/error_handler_494.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: Fix a bug that the error_handler can not provide the meaningful response body when the internal error code 494 is triggered.
type: bugfix
scope: Core
7 changes: 7 additions & 0 deletions kong/error_handlers.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ return function(ctx)
local status = kong.response.get_status()
local message = get_body(status)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better if we add some comments in the code to explain why we do this.

-- Nginx 494 status code is used internally when the client sends
-- too large or invalid HTTP headers. Kong is obliged to convert
-- it back to `400 Bad Request`.
if status == 494 then
status = 400
end

local headers
if find(accept_header, TYPE_GRPC, nil, true) == 1 then
message = { message = message }
Expand Down
3 changes: 2 additions & 1 deletion kong/templates/nginx_kong.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ server {
listen $(entry.listener);
> end

error_page 400 404 405 408 411 412 413 414 417 494 /kong_error_handler;
error_page 400 404 405 408 411 412 413 414 417 /kong_error_handler;
error_page 494 =494 /kong_error_handler;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems we return 400. So can remove =494?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=494 is essential, or the status code got in error_handler is always 400.

error_page 500 502 503 504 /kong_error_handler;

# Append the kong request id to the error log
Expand Down
2 changes: 1 addition & 1 deletion spec/02-integration/05-proxy/13-error_handlers_spec.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe("Proxy error handlers", function()
assert.res_status(400, res)
local body = res:read_body()
assert.matches("kong/", res.headers.server, nil, true)
assert.matches("Bad request\nrequest_id: %x+\n", body)
assert.matches("Request header or cookie too large", body)
end)

it("Request For Routers With Trace Method Not Allowed", function ()
Expand Down