Skip to content

Commit

Permalink
fix(core): reduce uninitialized variable message when nginx return 400 (
Browse files Browse the repository at this point in the history
#13201)

When Kong receives abnormal traffic, it will trigger 400 responses without initializing any Nginx variable, So it will trigger report.lua error, which is unnecessary.

eg: send HTTP traffic to HTTPS port, Nginx will finalize the current request by 400 response in the TLS handshake, So it will never call any openresty HTTP processing phase, it also does not initialize any Nginx variable.

Fix #13197
https://konghq.atlassian.net/browse/FTI-6025
  • Loading branch information
oowl authored Jul 15, 2024
1 parent f2ddfd9 commit 58ffebd
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
message: |
Fixed an issue where unnecessary uninitialized variable error log is reported when 400 bad requests were received.
type: bugfix
scope: Core
6 changes: 6 additions & 0 deletions kong/reports.lua
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,12 @@ function get_current_suffix(ctx)
return nil
end

-- 400 case is for invalid requests, eg: if a client sends a HTTP
-- request to a HTTPS port, it does not initialized any Nginx variables
if proxy_mode == "" and kong.response.get_status() == 400 then
return nil
end

log(WARN, "could not determine log suffix (scheme=", tostring(scheme),
", proxy_mode=", tostring(proxy_mode), ")")
end
Expand Down
5 changes: 3 additions & 2 deletions kong/templates/nginx_kong.lua
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ ssl_ciphers ${{SSL_CIPHERS}};
$(el.name) $(el.value);
> end
uninitialized_variable_warn off;
init_by_lua_block {
> if test and coverage then
require 'luacov'
Expand Down Expand Up @@ -386,9 +388,8 @@ server {
location = /kong_error_handler {
internal;
default_type '';
uninitialized_variable_warn off;
default_type '';
rewrite_by_lua_block {;}
access_by_lua_block {;}
Expand Down
14 changes: 14 additions & 0 deletions spec/02-integration/05-proxy/22-reports_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,20 @@ for _, strategy in helpers.each_strategy() do
proxy_ssl_client:close()
end)

it("when send http request to https port, no other error in error.log", function()
local https_port = assert(helpers.get_proxy_port(true))
local proxy_client = assert(helpers.proxy_client(nil, https_port))
local res = proxy_client:get("/", {
headers = { host = "http-service.test" }
})
reports_send_ping({port=constants.REPORTS.STATS_TLS_PORT})

assert.response(res).has_status(400)
assert.logfile().has.no.line("using uninitialized")
assert.logfile().has.no.line("could not determine log suffix (scheme=http, proxy_mode=)")
proxy_client:close()
end)

it("reports h2c requests", function()
local h2c_client = assert(helpers.proxy_client_h2c())
local body, headers = h2c_client({
Expand Down

1 comment on commit 58ffebd

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Bazel Build

Docker image available kong/kong:58ffebd65a3a88c6c12856714a5004caf219d5a4
Artifacts available https://github.com/Kong/kong/actions/runs/9935512156

Please sign in to comment.