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

Jump to bottom

fix(core): validation against header regex match #13290

Closed
wants to merge 1 commit into from
Closed

fix(core): validation against header regex match #13290

wants to merge 1 commit into from

Conversation

StarlightIbuki
Copy link
Contributor

@StarlightIbuki StarlightIbuki commented Jun 24, 2024 •
edited
Loading

Summary

The regex pattern for header should starts with "~*" instead of "~"

Checklist

Issue reference

fixup #12365
fix KAG-4788

@github-actions github-actions bot added core/db schema-change-noteworthy cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Jun 24, 2024
@StarlightIbuki StarlightIbuki force-pushed the fix/regex-header branch 2 times, most recently from ef8b4c2 to f5accc1 Compare June 24, 2024 08:58
ADD-SP
ADD-SP requested changes Jun 24, 2024
View reviewed changes
Copy link
Contributor

@ADD-SP ADD-SP left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The header.value will be treated as a regex if and only if there is only one value, but the current impl doesn't fit it.

#6079

kong/db/schema/typedefs.lua Outdated Show resolved Hide resolved
@StarlightIbuki StarlightIbuki requested a review from ADD-SP June 25, 2024 07:56
@StarlightIbuki StarlightIbuki requested a review from ADD-SP June 25, 2024 08:19
ADD-SP
ADD-SP reviewed Jun 25, 2024
View reviewed changes
kong/db/schema/typedefs.lua Outdated Show resolved Hide resolved
@pull-request-size pull-request-size bot added size/M and removed size/S labels Jun 25, 2024
@StarlightIbuki StarlightIbuki requested a review from ADD-SP June 25, 2024 09:08
@StarlightIbuki StarlightIbuki force-pushed the fix/regex-header branch 2 times, most recently from e62b197 to b00ac60 Compare July 1, 2024 09:35
@ADD-SP ADD-SP requested a review from chronolaw July 1, 2024 09:40
StarlightIbuki
StarlightIbuki commented Jul 2, 2024
View reviewed changes
kong/router/utils.lua Outdated Show resolved Hide resolved
kong/db/schema/typedefs.lua Show resolved Hide resolved
kong/db/schema/typedefs.lua Show resolved Hide resolved
chronolaw
chronolaw reviewed Jul 2, 2024
View reviewed changes
kong/db/schema/typedefs.lua
-- the value will be interpreted as a regex by the router; but is it a
-- valid one? Let's dry-run it with the same options as our router.
local _, _, err = ngx.re.find("", regex, "aj")
local _, _, err = ngx.re.find("", pattern, "a")
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment for removing the option j.

kong/db/schema/typedefs.lua
if not is_regex_pattern(pattern) then
local function validate_header_regex_or_plain_pattern(patterns)
-- when there's only 1 pattern and it starts with "~*", it's a regex pattern
if #patterns ~= 1 or patterns[1]:sub(1, 2) ~= "~*" then
Copy link
Contributor

Choose a reason for hiding this comment

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

The NOT condition is hard to understand, could we rewrite it like

if #patterns == 1 and patterns[1]:sub(1, 2) == "~*" then
  return is_valid_regex_pattern(patterns[1]:sub(3))
end

return true

@StarlightIbuki @ADD-SP
fix(core): header regex for compat mode
619feef 
2 condtions:
1. only 1 header value presents
2. it starts with "~*"

Fixup #12365
Fix KAG-4788

Co-authored-by: Qi <add_sp@outlook.com>
@StarlightIbuki StarlightIbuki marked this pull request as draft July 2, 2024 07:52
@chronolaw
Copy link
Contributor

should we close it?

@StarlightIbuki
Copy link
Contributor Author

Let's close this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@randmonkey randmonkey randmonkey left review comments

@chronolaw chronolaw chronolaw left review comments

@ADD-SP ADD-SP Awaiting requested review from ADD-SP

Assignees

@StarlightIbuki StarlightIbuki

Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/db core/router schema-change-noteworthy size/M skip-changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@StarlightIbuki @chronolaw @randmonkey @ADD-SP