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

caddyhttp: Escaping placeholders in CEL, add vars and vars_regexp #6594

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

francislavoie
Copy link
Member

@francislavoie francislavoie commented Sep 27, 2024

Related to #6584, prior work in #4715 (comment)

Our current CEL placeholder regexp is too naive. It replaces everything that looks like a placeholder, and offers no good option to opt out if a literal placeholder-looking input is given to some CEL functions as input.

To solve this, we can do the placeholder replacement in two steps.

  • First, we match whether the placeholder is not preceded by a \ (or start of a line) and replace it and preserving the preceding character which is matched (we now have two capture groups so ${1} has to go to the start)
  • Second, we match for a placeholder which does have a preceding \, and drop the \.

That gives a way to escape through the caddyPlaceholder() replacement and get an input directly to the given matcher.

One interesting quirk: CEL itself has its own \\ escape sequence, so depending on whether we want the placeholder to be replaced by the matcher (e.g. header matcher does do placeholder replacements on its inputs at runtime) or we want the value to be raw determines how many backslashes we need. The test shows this pretty well I think:

  • If the header matcher should take the value as-is and not perform placeholder replacement (because the match value is also placeholder-like), then we need three backslashes, like \\\{foobar}. This is because the last one of the three is for escaping caddyPlaceholder(), then the prior two are collapsed into one by CEL's parsing itself, then the last one is to escape the placeholder replacer, and the result is a clean {foobar} matching value.
  • If the header matcher itself should perform placeholder replacement (not done in the CEL matcher, but deeper in the matcher itself) then a single backslash is used, like \{http.request.uri.path} (or \{path} in the Caddyfile). This only escapes past the caddyPlaceholder() regexp, but does not escape past the placeholder replacer which runs inside header.

We'll need to document this, but it's tricky 😅

After doing the above, I decided to also implement vars and vars_regexp support in CEL, which we skipped implementing because of the above placeholder limitations.

@francislavoie francislavoie added the feature ⚙️ New feature or request label Sep 27, 2024
@francislavoie francislavoie changed the title caddyhttp: Escaping placeholders in CEL caddyhttp: Escaping placeholders in CEL, add vars and vars_regexp Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⚙️ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant