Copilot commented on code in PR #13587:
URL: https://github.com/apache/trafficserver/pull/13587#discussion_r4148622121
##########
plugins/header_rewrite/condition.cc:
##########
@@ -75,6 +76,36 @@ parse_matcher_op(std::string &arg)
}
}
+void
+Condition::normalize(std::string &s, size_t start)
+{
+ auto in = s.find('%', start);
+
+ if (in == std::string::npos) {
+ return;
+ }
+
+ auto out = in;
+
+ while (in < s.size()) {
+ if (s[in] == '%' && s.size() - in > 2) {
+ const char *hex = s.data() + in + 1;
+ unsigned char c = 0;
+ auto [end, ec] = std::from_chars(hex, hex + 2, c, 16);
+
+ // Escaped controls stay encoded, since a decoded CR/LF in an expansion
would inject a header field.
+ if (ec == std::errc{} && end == hex + 2 && c >= 0x20 && c != 0x7f) {
+ s[out++] = static_cast<char>(c);
+ in += 3;
+ continue;
Review Comment:
`std::from_chars` support for `unsigned char` is less portable across
standard library implementations than parsing into a wider integer type (e.g.,
`unsigned int`) and then casting. To reduce build/portability risk, parse into
an `unsigned int` (or similar) and range-check before converting to `char`.
##########
tools/hrw4u/src/hrw_symbols.py:
##########
@@ -414,6 +415,20 @@ def negate_expression(self, term: str) -> str:
def percent_to_ident_or_func(self, percent: str, section: SectionType |
None) -> tuple[str, bool]:
"""Convert percent block to identifier or function call."""
+ stripped, mods = split_percent_mods(percent)
+ expr, is_func = self._percent_to_ident_or_func(stripped, section)
+
+ if not mods:
+ return expr, is_func
+
+ # A block with no DSL equivalent comes back as %{...}, where the
modifiers belong
+ # inside the braces rather than in a "with" clause.
+ if expr == stripped:
+ return apply_percent_mods(expr, mods), is_func
+
+ return f"{expr} with {','.join(mods)}", is_func
Review Comment:
The docstring for `percent_to_ident_or_func()` is now incomplete/misleading:
it no longer only “converts percent block to identifier or function call”, it
also parses trailing `[MODS]` and may return either `expr with ...` or a
rewritten `%{... [MODS]}` when unmapped. Update the docstring to describe the
modifier-aware behavior so callers understand the new contract.
##########
tools/hrw4u/src/common.py:
##########
@@ -43,20 +43,57 @@ class RegexPatterns:
SUBSTITUTE_PATTERN: Final = re.compile(
r"""(?P<escaped>\{\{.*?\}\})
|
-
(?<!%)\{\s*(?P<func>[a-zA-Z_][a-zA-Z0-9_-]*)\s*\((?P<args>[^)]*)\)\s*\}
+ (?<!%)\{\s*(?P<func>[a-zA-Z_][a-zA-Z0-9_-]*)\s*\((?P<args>[^)]*)\)
+ (?:\s+with\s+(?P<func_mods>[A-Za-z][A-Za-z0-9,\s]*?))?\s*\}
|
(?<!%)\{(?P<var>[^{}()]+)\}
""",
re.VERBOSE | re.DOTALL,
)
+ # An interpolated symbol may carry modifiers, spelled as on a condition:
{inbound.url.path with NORM}
+ INTERPOLATION_MODS: Final =
re.compile(r'^\s*(?P<name>\S+?)\s+with\s+(?P<mods>[A-Za-z][A-Za-z0-9,\s]*?)\s*$')
+
+ # Trailing [MODS] inside a %{} block, which u4wrh turns back into a "with"
clause
+ PERCENT_MODS: Final =
re.compile(r'^(?P<body>.*?)\s+\[(?P<mods>[A-Za-z][A-Za-z0-9,\s]*)\]$')
+
# Additional performance patterns
IDENTIFIER: Final = re.compile(r'^[a-zA-Z_][a-zA-Z0-9_]*$')
WHITESPACE: Final = re.compile(r'\s+')
COMMENT_BLOCK: Final = re.compile(r'/\*.*?\*/', re.DOTALL)
STRING_INTERPOLATION: Final =
re.compile(r'\{([a-zA-Z_][a-zA-Z0-9_.-]*(?:\([^)]*\))?)\}', re.MULTILINE)
+def parse_mods(raw: str | None) -> list[str]:
+ """Parse 'MOD, mod,MOD' into ['MOD', 'MOD', 'MOD']."""
+ return [mod.strip().upper() for mod in raw.split(",") if mod.strip()] if
raw else []
+
+
Review Comment:
`parse_mods()` preserves duplicates, so an input like `with NORM,NORM` will
flow through and get re-emitted as duplicate modifiers (and may trigger
duplicate warnings). Consider de-duplicating while preserving order (e.g.,
first occurrence wins) to keep output stable and error reporting cleaner.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
