This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.2.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit faed911e76c7c18af40d11c03f733f323c522ae9 Author: Leif Hedstrom <[email protected]> AuthorDate: Tue Mar 3 11:43:05 2026 -0700 hrw4u: More fixes for complex groups and logical operations (#12907) * hrw4u: More fixes for complex groups and logical operations * Fixes from reviews (cherry picked from commit 49557875c90a288cf09bce1171e642a432996f61) --- tools/hrw4u/src/hrw_visitor.py | 15 ++++++++++++++- tools/hrw4u/src/visitor.py | 19 +++++++++++++++---- .../tests/data/conds/double-negation.output.txt | 4 ++-- tools/hrw4u/tests/data/conds/exceptions.txt | 2 +- tools/hrw4u/tests/data/conds/group-and-not.ast.txt | 1 + .../hrw4u/tests/data/conds/group-and-not.input.txt | 5 +++++ .../tests/data/conds/group-and-not.output.txt | 6 ++++++ tools/hrw4u/tests/data/conds/group-in-if.ast.txt | 1 + tools/hrw4u/tests/data/conds/group-in-if.input.txt | 11 +++++++++++ .../hrw4u/tests/data/conds/group-in-if.output.txt | 15 +++++++++++++++ tools/hrw4u/tests/data/conds/group-nested.ast.txt | 1 + .../hrw4u/tests/data/conds/group-nested.input.txt | 5 +++++ .../hrw4u/tests/data/conds/group-nested.output.txt | 7 +++++++ tools/hrw4u/tests/data/conds/group-or.ast.txt | 1 + tools/hrw4u/tests/data/conds/group-or.input.txt | 5 +++++ tools/hrw4u/tests/data/conds/group-or.output.txt | 6 ++++++ tools/hrw4u/tests/data/conds/group-two.ast.txt | 1 + tools/hrw4u/tests/data/conds/group-two.input.txt | 5 +++++ tools/hrw4u/tests/data/conds/group-two.output.txt | 10 ++++++++++ .../tests/data/examples/all-nonsense.output.txt | 22 ++++++---------------- 20 files changed, 118 insertions(+), 24 deletions(-) diff --git a/tools/hrw4u/src/hrw_visitor.py b/tools/hrw4u/src/hrw_visitor.py index cbaa8f7eff..453fe062b6 100644 --- a/tools/hrw4u/src/hrw_visitor.py +++ b/tools/hrw4u/src/hrw_visitor.py @@ -57,6 +57,7 @@ class HRWInverseVisitor(u4wrhVisitor, BaseHRWVisitor): self._if_depth = 0 # Track nesting depth of if blocks self._in_elif_mode = False self._just_closed_nested = False + self._deferred_comments: list[str] = [] @lru_cache(maxsize=128) def _cached_percent_parsing(self, pct_text: str) -> tuple[str, str | None]: @@ -83,6 +84,12 @@ class HRWInverseVisitor(u4wrhVisitor, BaseHRWVisitor): self._in_group = False self._group_terms.clear() + def _flush_deferred_comments(self) -> None: + """Emit comments that were deferred while inside a rule's if-block.""" + for comment in self._deferred_comments: + self.output.append(comment) + self._deferred_comments.clear() + def _start_new_section(self, section_type: SectionType) -> None: """Start a new section, handling continuation of existing sections.""" with self.debug_context(f"start_section {section_type.value}"): @@ -93,6 +100,7 @@ class HRWInverseVisitor(u4wrhVisitor, BaseHRWVisitor): self.emit("}") self._if_depth -= 1 self._reset_condition_state() + self._flush_deferred_comments() if self.output and self.output[-1] != "": self.output.append("") return @@ -105,6 +113,8 @@ class HRWInverseVisitor(u4wrhVisitor, BaseHRWVisitor): if had_section and self.output and self.output[-1] != "": self.output.append("") + self._flush_deferred_comments() + self._section_label = section_type self.emit(f"{section_type.value} {{") self._section_opened = True @@ -152,6 +162,7 @@ class HRWInverseVisitor(u4wrhVisitor, BaseHRWVisitor): for line in ctx.line(): self.visit(line) self._close_if_and_section() + self._flush_deferred_comments() var_declarations = self.symbol_resolver.get_var_declarations() if var_declarations: @@ -167,7 +178,9 @@ class HRWInverseVisitor(u4wrhVisitor, BaseHRWVisitor): with self.debug_context("visitCommentLine"): comment_text = ctx.COMMENT().getText() self._flush_pending_condition() - if self._section_opened: + if self._if_depth > 0: + self._deferred_comments.append(comment_text) + elif self._section_opened: self.emit(comment_text) else: self.output.append(comment_text) diff --git a/tools/hrw4u/src/visitor.py b/tools/hrw4u/src/visitor.py index ad240db05d..e62f222078 100644 --- a/tools/hrw4u/src/visitor.py +++ b/tools/hrw4u/src/visitor.py @@ -283,7 +283,7 @@ class HRW4UVisitor(hrw4uVisitor, BaseHRWVisitor): else: self.visit(body) elif is_conditional or not in_statement_block: - if idx > 0: + if first_hook_emitted: self._flush_condition() self.output.append("") @@ -603,14 +603,25 @@ class HRW4UVisitor(hrw4uVisitor, BaseHRWVisitor): match ctx: case _ if ctx.getChildCount() == 2 and ctx.getChild(0).getText() == "!": self._dbg("`NOT' detected") - self._cond_state.not_ = True - self.emit_factor(ctx.getChild(1), last=last) + child = ctx.getChild(1) + if child.LPAREN(): + self._dbg("GROUP-START (negated)") + self.emit_condition("cond %{GROUP}", final=True) + with self.cond_indented(): + self.emit_expression(child.expression(), nested=False, last=True, grouped=False) + self._cond_state.last = last + self._cond_state.not_ = True + self.emit_condition("cond %{GROUP:END}") + else: + self._cond_state.not_ = True + self.emit_factor(child, last=last) case _ if ctx.LPAREN(): self._dbg("GROUP-START") self.emit_condition("cond %{GROUP}", final=True) with self.cond_indented(): - self.emit_expression(ctx.expression(), nested=False, last=True, grouped=True) + # grouped=False: GROUP is already open; no second wrap for OR. + self.emit_expression(ctx.expression(), nested=False, last=True, grouped=False) self._cond_state.last = last self.emit_condition("cond %{GROUP:END}") diff --git a/tools/hrw4u/tests/data/conds/double-negation.output.txt b/tools/hrw4u/tests/data/conds/double-negation.output.txt index 9ee7a7dfe0..81c4e86754 100644 --- a/tools/hrw4u/tests/data/conds/double-negation.output.txt +++ b/tools/hrw4u/tests/data/conds/double-negation.output.txt @@ -1,5 +1,5 @@ cond %{REMAP_PSEUDO_HOOK} [AND] cond %{GROUP} - cond %{CLIENT-HEADER:X-Debug} ="keep" -cond %{GROUP:END} + cond %{CLIENT-HEADER:X-Debug} ="keep" [NOT] +cond %{GROUP:END} [NOT] set-header X-Found "yes" diff --git a/tools/hrw4u/tests/data/conds/exceptions.txt b/tools/hrw4u/tests/data/conds/exceptions.txt index 85aecfe361..86e44106ba 100644 --- a/tools/hrw4u/tests/data/conds/exceptions.txt +++ b/tools/hrw4u/tests/data/conds/exceptions.txt @@ -3,5 +3,5 @@ # # Implicit = comparisons implicit-cmp.input: u4wrh -# Double negation !(x != y) cancels to x == y, reverse can't reconstruct the original form +# Double negation !(x != y) emits GROUP/GROUP:END [NOT] pairs; reverse can't reconstruct the original form double-negation.input: hrw4u diff --git a/tools/hrw4u/tests/data/conds/group-and-not.ast.txt b/tools/hrw4u/tests/data/conds/group-and-not.ast.txt new file mode 100644 index 0000000000..eef1424430 --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-and-not.ast.txt @@ -0,0 +1 @@ +(program (programItem (section SEND_RESPONSE { (sectionBody (conditional (ifStatement if (condition (expression (term (factor ! (factor ( (expression (term (term (factor inbound.req.X-Foo)) && (factor inbound.req.X-Bar))) )))))) (block { (blockItem (statement inbound.resp.X-Result = (value "matched") ;)) })))) })) <EOF>) diff --git a/tools/hrw4u/tests/data/conds/group-and-not.input.txt b/tools/hrw4u/tests/data/conds/group-and-not.input.txt new file mode 100644 index 0000000000..c517ebae9b --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-and-not.input.txt @@ -0,0 +1,5 @@ +SEND_RESPONSE { + if !(inbound.req.X-Foo && inbound.req.X-Bar) { + inbound.resp.X-Result = "matched"; + } +} diff --git a/tools/hrw4u/tests/data/conds/group-and-not.output.txt b/tools/hrw4u/tests/data/conds/group-and-not.output.txt new file mode 100644 index 0000000000..b9f6292210 --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-and-not.output.txt @@ -0,0 +1,6 @@ +cond %{SEND_RESPONSE_HDR_HOOK} [AND] +cond %{GROUP} + cond %{CLIENT-HEADER:X-Foo} ="" [AND,NOT] + cond %{CLIENT-HEADER:X-Bar} ="" [NOT] +cond %{GROUP:END} [NOT] + set-header X-Result "matched" diff --git a/tools/hrw4u/tests/data/conds/group-in-if.ast.txt b/tools/hrw4u/tests/data/conds/group-in-if.ast.txt new file mode 100644 index 0000000000..0e833a389f --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-in-if.ast.txt @@ -0,0 +1 @@ +(program (programItem (section SEND_RESPONSE { (sectionBody (conditional (ifStatement if (condition (expression (term (factor (comparison (comparable inbound.url.path) ~ (regex /\/alpha\//)))))) (block { (blockItem (conditional (ifStatement if (condition (expression (term (term (factor (comparison (comparable inbound.req.X-Branch) == (value "alpha")))) && (factor ( (expression (expression (term (factor inbound.req.X-Sub-A))) || (term (factor inbound.req.X-Sub-B))) ))))) (block { (blockIt [...] diff --git a/tools/hrw4u/tests/data/conds/group-in-if.input.txt b/tools/hrw4u/tests/data/conds/group-in-if.input.txt new file mode 100644 index 0000000000..51be6387b5 --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-in-if.input.txt @@ -0,0 +1,11 @@ +SEND_RESPONSE { + if inbound.url.path ~ /\/alpha\// { + if inbound.req.X-Branch == "alpha" && (inbound.req.X-Sub-A || inbound.req.X-Sub-B) { + inbound.resp.X-Result = "alpha"; + } elif inbound.req.X-Branch == "beta" { + inbound.resp.X-Result = "beta"; + } else { + inbound.resp.X-Result = "other"; + } + } +} diff --git a/tools/hrw4u/tests/data/conds/group-in-if.output.txt b/tools/hrw4u/tests/data/conds/group-in-if.output.txt new file mode 100644 index 0000000000..c74ee840b5 --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-in-if.output.txt @@ -0,0 +1,15 @@ +cond %{SEND_RESPONSE_HDR_HOOK} [AND] +cond %{CLIENT-URL:PATH} /\/alpha\// + if + cond %{CLIENT-HEADER:X-Branch} ="alpha" [AND] + cond %{GROUP} + cond %{CLIENT-HEADER:X-Sub-A} ="" [OR,NOT] + cond %{CLIENT-HEADER:X-Sub-B} ="" [NOT] + cond %{GROUP:END} + set-header X-Result "alpha" + elif + cond %{CLIENT-HEADER:X-Branch} ="beta" + set-header X-Result "beta" + else + set-header X-Result "other" + endif diff --git a/tools/hrw4u/tests/data/conds/group-nested.ast.txt b/tools/hrw4u/tests/data/conds/group-nested.ast.txt new file mode 100644 index 0000000000..cd9fc6d5f1 --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-nested.ast.txt @@ -0,0 +1 @@ +(program (programItem (section SEND_RESPONSE { (sectionBody (conditional (ifStatement if (condition (expression (term (term (factor inbound.req.X-Outer)) && (factor ( (expression (expression (term (factor inbound.req.X-Inner-A))) || (term (factor inbound.req.X-Inner-B))) ))))) (block { (blockItem (statement inbound.resp.X-Result = (value "matched") ;)) })))) })) <EOF>) diff --git a/tools/hrw4u/tests/data/conds/group-nested.input.txt b/tools/hrw4u/tests/data/conds/group-nested.input.txt new file mode 100644 index 0000000000..ebf8d563f8 --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-nested.input.txt @@ -0,0 +1,5 @@ +SEND_RESPONSE { + if inbound.req.X-Outer && (inbound.req.X-Inner-A || inbound.req.X-Inner-B) { + inbound.resp.X-Result = "matched"; + } +} diff --git a/tools/hrw4u/tests/data/conds/group-nested.output.txt b/tools/hrw4u/tests/data/conds/group-nested.output.txt new file mode 100644 index 0000000000..18a7d8f917 --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-nested.output.txt @@ -0,0 +1,7 @@ +cond %{SEND_RESPONSE_HDR_HOOK} [AND] +cond %{CLIENT-HEADER:X-Outer} ="" [AND,NOT] +cond %{GROUP} + cond %{CLIENT-HEADER:X-Inner-A} ="" [OR,NOT] + cond %{CLIENT-HEADER:X-Inner-B} ="" [NOT] +cond %{GROUP:END} + set-header X-Result "matched" diff --git a/tools/hrw4u/tests/data/conds/group-or.ast.txt b/tools/hrw4u/tests/data/conds/group-or.ast.txt new file mode 100644 index 0000000000..f08d9740d2 --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-or.ast.txt @@ -0,0 +1 @@ +(program (programItem (section SEND_RESPONSE { (sectionBody (conditional (ifStatement if (condition (expression (term (factor ( (expression (expression (term (factor inbound.req.X-Foo))) || (term (factor inbound.req.X-Bar))) ))))) (block { (blockItem (statement inbound.resp.X-Result = (value "matched") ;)) })))) })) <EOF>) diff --git a/tools/hrw4u/tests/data/conds/group-or.input.txt b/tools/hrw4u/tests/data/conds/group-or.input.txt new file mode 100644 index 0000000000..74abbefead --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-or.input.txt @@ -0,0 +1,5 @@ +SEND_RESPONSE { + if (inbound.req.X-Foo || inbound.req.X-Bar) { + inbound.resp.X-Result = "matched"; + } +} diff --git a/tools/hrw4u/tests/data/conds/group-or.output.txt b/tools/hrw4u/tests/data/conds/group-or.output.txt new file mode 100644 index 0000000000..8d7a03187b --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-or.output.txt @@ -0,0 +1,6 @@ +cond %{SEND_RESPONSE_HDR_HOOK} [AND] +cond %{GROUP} + cond %{CLIENT-HEADER:X-Foo} ="" [OR,NOT] + cond %{CLIENT-HEADER:X-Bar} ="" [NOT] +cond %{GROUP:END} + set-header X-Result "matched" diff --git a/tools/hrw4u/tests/data/conds/group-two.ast.txt b/tools/hrw4u/tests/data/conds/group-two.ast.txt new file mode 100644 index 0000000000..d87d0e1db8 --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-two.ast.txt @@ -0,0 +1 @@ +(program (programItem (section SEND_RESPONSE { (sectionBody (conditional (ifStatement if (condition (expression (expression (term (factor ( (expression (term (term (factor inbound.req.X-Left-A)) && (factor inbound.req.X-Left-B))) )))) || (term (factor ( (expression (term (term (factor inbound.req.X-Right-A)) && (factor inbound.req.X-Right-B))) ))))) (block { (blockItem (statement inbound.resp.X-Result = (value "matched") ;)) })))) })) <EOF>) diff --git a/tools/hrw4u/tests/data/conds/group-two.input.txt b/tools/hrw4u/tests/data/conds/group-two.input.txt new file mode 100644 index 0000000000..30b477cada --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-two.input.txt @@ -0,0 +1,5 @@ +SEND_RESPONSE { + if (inbound.req.X-Left-A && inbound.req.X-Left-B) || (inbound.req.X-Right-A && inbound.req.X-Right-B) { + inbound.resp.X-Result = "matched"; + } +} diff --git a/tools/hrw4u/tests/data/conds/group-two.output.txt b/tools/hrw4u/tests/data/conds/group-two.output.txt new file mode 100644 index 0000000000..45c3e09781 --- /dev/null +++ b/tools/hrw4u/tests/data/conds/group-two.output.txt @@ -0,0 +1,10 @@ +cond %{SEND_RESPONSE_HDR_HOOK} [AND] +cond %{GROUP} + cond %{CLIENT-HEADER:X-Left-A} ="" [AND,NOT] + cond %{CLIENT-HEADER:X-Left-B} ="" [NOT] +cond %{GROUP:END} [OR] +cond %{GROUP} + cond %{CLIENT-HEADER:X-Right-A} ="" [AND,NOT] + cond %{CLIENT-HEADER:X-Right-B} ="" [NOT] +cond %{GROUP:END} + set-header X-Result "matched" diff --git a/tools/hrw4u/tests/data/examples/all-nonsense.output.txt b/tools/hrw4u/tests/data/examples/all-nonsense.output.txt index 0134d0aa96..a51a6fbd1e 100644 --- a/tools/hrw4u/tests/data/examples/all-nonsense.output.txt +++ b/tools/hrw4u/tests/data/examples/all-nonsense.output.txt @@ -1,6 +1,5 @@ # Boolean and integer state you can flip/use across sections - cond %{REMAP_PSEUDO_HOOK} [AND] # Plugin controls set-http-cntl TXN_DEBUG true @@ -30,19 +29,15 @@ cond %{READ_REQUEST_PRE_REMAP_HOOK} [AND] cond %{GROUP} cond %{METHOD} ="GET" [AND,NOCASE] cond %{GROUP} - cond %{GROUP} - cond %{CLIENT-URL:PATH} ("mp3","m3u","m3u8") [OR,NOCASE,EXT] - cond %{CLIENT-URL:HOST} /(?i)^api\./ - cond %{GROUP:END} [AND] + cond %{CLIENT-URL:PATH} ("mp3","m3u","m3u8") [OR,NOCASE,EXT] + cond %{CLIENT-URL:HOST} /(?i)^api\./ cond %{GROUP:END} cond %{GROUP:END} set-header X-Prefilter "static-or-api" elif cond %{GROUP} - cond %{GROUP} - cond %{ACCESS:/var/developertesting} [OR] - cond %{INTERNAL-TRANSACTION} - cond %{GROUP:END} [AND] + cond %{ACCESS:/var/developertesting} [OR] + cond %{INTERNAL-TRANSACTION} cond %{GROUP:END} set-header X-Prefilter "dev-or-internal" elif @@ -56,10 +51,8 @@ else cond %{READ_REQUEST_PRE_REMAP_HOOK} [AND] cond %{GROUP} - cond %{GROUP} - cond %{CIDR:16,48} ="10.0.0.0" [OR] - cond %{CIDR:8,8} ="fd00::" - cond %{GROUP:END} [AND] + cond %{CIDR:16,48} ="10.0.0.0" [OR] + cond %{CIDR:8,8} ="fd00::" cond %{GROUP:END} set-header X-Network "private" @@ -102,7 +95,6 @@ cond %{GROUP:END} no-op [L] # like [L] - cond %{READ_REQUEST_HDR_HOOK} [AND] # Header presence / equality and capture groups cond %{GROUP} @@ -144,7 +136,6 @@ cond %{GROUP:END} set-state-int16 0 6553 # assign int16 - cond %{SEND_REQUEST_HDR_HOOK} [AND] # Use NEXT-HOP information to adjust Host header cond %{GROUP} @@ -167,7 +158,6 @@ cond %{GROUP:END} cond %{REMAP_PSEUDO_HOOK} [AND] cond %{CLIENT-HEADER:X-Bar} ="fie" [MID] - cond %{READ_RESPONSE_HDR_HOOK} [AND] # Cache lookup status checks cond %{GROUP}
