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(core): reduce uninitialized variable message when nginx return 400 #13201

Merged
merged 8 commits into from
Jul 15, 2024

Conversation

oowl
Copy link
Member

@oowl oowl commented Jun 13, 2024

Summary

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.

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

Issue reference

Fix #13197
https://konghq.atlassian.net/browse/FTI-6025

@github-actions github-actions bot added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Jun 13, 2024
bungle
bungle previously requested changes Jun 16, 2024
kong/reports.lua Outdated Show resolved Hide resolved
@bungle
Copy link
Member

bungle commented Jun 17, 2024

@oowl, did we consider defining those variables? E.g. move them to upper scope or something? Or skipping accessing them when we know they are not there (currently it feels like only reports.lua has this issue, so turning totally off feel like hitting a fly with a hammer)? Uninitialized warnings seems to be nice thing to catch some small issues? So turning it totally off feels like we will also miss something.

@oowl oowl force-pushed the fix/ngx-var-not-init-in-400 branch 2 times, most recently from 7c5a012 to 5a8629e Compare July 11, 2024 07:21
@pull-request-size pull-request-size bot added size/M and removed size/S labels Jul 11, 2024
@oowl oowl force-pushed the fix/ngx-var-not-init-in-400 branch from 5a8629e to 50d6c66 Compare July 12, 2024 06:27
@oowl
Copy link
Member Author

oowl commented Jul 12, 2024

I have tried to place set command to the main block, but it did not resolve this problem. 50d6c66

I think the problem is nginx can not get the location in the SSL handshake phase, it has no chance to init any variable.
@bungle

@pull-request-size pull-request-size bot added size/S and removed size/M labels Jul 12, 2024
@oowl oowl force-pushed the fix/ngx-var-not-init-in-400 branch from 5fb40f4 to 9ffe605 Compare July 15, 2024 07:00
@windmgc windmgc merged commit 58ffebd into master Jul 15, 2024
25 checks passed
@windmgc windmgc deleted the fix/ngx-var-not-init-in-400 branch July 15, 2024 07:38
@github-actions github-actions bot added the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Jul 15, 2024
oowl added a commit that referenced this pull request Jul 15, 2024
#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
@kikito kikito removed the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Jul 16, 2024
@Kong Kong deleted a comment from team-gateway-bot Jul 16, 2024
oowl added a commit that referenced this pull request Aug 15, 2024
#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
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.

Odd logs maybe potential bugs in 3.7.0 trying to wrap my head didn't see on 2.8.x
6 participants