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.

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

Conversation

liverpool8056
Copy link
Contributor

Summary

fix the bug that error handler can't recognize status code 494.

There is a dedicated response body for 494 defined in error_handler. However, based on the current configuration for error_page in nginx-kong.conf, 494 will not be treated correctly wihout reserving it by the =response option in error_page directive. In this PR, a error_page configuration is added for 494 separately, so that it can be recognized in error handler, and it will be replaced with 400 finally.

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Full changelog

  • [Implement ...]

Issue reference

FTI-5374

@liverpool8056 liverpool8056 marked this pull request as draft November 29, 2023 03:56
@liverpool8056 liverpool8056 force-pushed the fix/core/error_handler_for_status_code_494 branch from a7afbd4 to b47005c Compare November 29, 2023 04:04
@liverpool8056 liverpool8056 force-pushed the fix/core/error_handler_for_status_code_494 branch from b47005c to 7db60b5 Compare November 29, 2023 04:05
@liverpool8056 liverpool8056 marked this pull request as ready for review November 29, 2023 04:05
@liverpool8056 liverpool8056 force-pushed the fix/core/error_handler_for_status_code_494 branch 2 times, most recently from 682327c to a89dc7b Compare November 29, 2023 07:48
@@ -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.

@@ -59,6 +59,10 @@ 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.

status code 494.

There is a dedicated response body for 494 defined in error_handler.
However, based on the current configuration for `error_page` in
nginx-kong.conf, 494 will not be treated correctly wihout reserving it
by the `=response` option in `error_page` directive.
In this PR, a `error_page` configuration is added for 494 separately, so
that it can be recognized in error handler, and it will be replaced
with 400 finally.

FTI-5374
@liverpool8056 liverpool8056 force-pushed the fix/core/error_handler_for_status_code_494 branch from a89dc7b to 7ee8fcd Compare November 30, 2023 03:35
@windmgc windmgc merged commit cfb56a7 into master Nov 30, 2023
24 checks passed
@windmgc windmgc deleted the fix/core/error_handler_for_status_code_494 branch November 30, 2023 08:16
@liverpool8056 liverpool8056 added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Dec 1, 2023
@kikito kikito added cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee and removed cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Jan 26, 2024
@team-gateway-bot
Copy link
Collaborator

Cherry-pick failed for master, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git remote add upstream https://github.com/kong/kong-ee
git fetch upstream master
git worktree add -d .worktree/cherry-pick-12114-to-master-to-upstream upstream/master
cd .worktree/cherry-pick-12114-to-master-to-upstream
git checkout -b cherry-pick-12114-to-master-to-upstream
ancref=$(git merge-base 25149497a5f8f71ef8693e46b8e183c0d08e46eb 7ee8fcd4c0afad181ae83278be7991fd5384ac99)
git cherry-pick -x $ancref..7ee8fcd4c0afad181ae83278be7991fd5384ac99

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/templates size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants