Copilot commented on code in PR #13675:
URL: https://github.com/apache/trafficserver/pull/13675#discussion_r4003699970
##########
doc/admin-guide/configuration/hrw4u.en.rst:
##########
@@ -778,9 +779,24 @@ Construct What it controls
``variables`` The entire ``VARS`` section and all variable usage
``else`` The ``else { ... }`` branch of conditionals
``elif`` The ``elif ... { ... }`` branch of conditionals
-``in`` The ``in [...]`` and ``!in [...]`` set membership operators
+``in`` Set membership: the ``[...]`` value form and
+ the ``{...}`` IP range form, negated or not
================ ===================================================
+Condition Modifiers
+-------------------
+
+The ``modifiers`` list accepts ``AND``, ``OR``, ``NOT``, ``NOCASE``, ``PRE``,
+``SUF``, ``EXT``, ``MID``, ``I``, ``L`` and ``QSA``. Entries match the modifier
+however it is written, not only the explicit ``with`` form: ``AND`` also covers
+``&&``, ``OR`` also covers ``||``, and ``NOT`` also covers ``!``, ``!=``,
+``!~`` and ``!in``.
+
+Negation that the compiler introduces on its own is not matched. A bare header
+test such as ``if inbound.req.X-Foo`` compiles to ``cond %{HEADER:X-Foo} =""
+[NOT]``, and denying ``NOT`` does not reject it — the policy governs what the
+source writes.
Review Comment:
The inline-literal markup appears unbalanced: `[NOT]`` has closing backticks
but not an opening literal marker, which can break Sphinx rendering. Also, the
example cond token (`%{HEADER:...}`) doesn’t match the emitted form shown in
the new golden output (`%{CLIENT-HEADER:...}`); consider aligning the
documentation example with actual compiler output to avoid confusion.
##########
tools/hrw4u/src/visitor.py:
##########
@@ -1029,6 +1029,9 @@ def visitComparison(self, ctx, *, last: bool = False) ->
None:
else:
negate = operator.symbol.type in (hrw4uParser.NEQ,
hrw4uParser.NOT_TILDE)
+ if negate and not self._sandbox_check(ctx, lambda:
self._sandbox.check_modifier("NOT")):
+ return
Review Comment:
These new early `return`s stop traversal/emission for the current node,
which can suppress additional sandbox diagnostics inside the same expression
subtree (e.g., `!(denied_fn() && ...)` would now report only the `NOT` denial
and skip checking `denied_fn()`). Consider separating (1) validation/diagnostic
collection from (2) code emission: record the policy violation but still visit
relevant child nodes to collect other errors, while gating only the output
generation.
##########
tools/hrw4u/src/visitor.py:
##########
@@ -1051,6 +1054,8 @@ def visitComparison(self, ctx, *, last: bool = False) ->
None:
cond_txt = f"{lhs} {regex_expr}"
case _ if ctx.iprange():
+ if not self._sandbox_check(ctx, lambda:
self._sandbox.check_language("in")):
+ return
Review Comment:
These new early `return`s stop traversal/emission for the current node,
which can suppress additional sandbox diagnostics inside the same expression
subtree (e.g., `!(denied_fn() && ...)` would now report only the `NOT` denial
and skip checking `denied_fn()`). Consider separating (1) validation/diagnostic
collection from (2) code emission: record the policy violation but still visit
relevant child nodes to collect other errors, while gating only the output
generation.
##########
tools/hrw4u/src/visitor.py:
##########
@@ -1148,6 +1153,8 @@ def emit_factor(self, ctx, *, last: bool = False) -> None:
match ctx:
case _ if ctx.getChildCount() == 2 and
ctx.getChild(0).getText() == "!":
self._dbg("`NOT' detected")
+ if not self._sandbox_check(ctx, lambda:
self._sandbox.check_modifier("NOT")):
+ return
Review Comment:
These new early `return`s stop traversal/emission for the current node,
which can suppress additional sandbox diagnostics inside the same expression
subtree (e.g., `!(denied_fn() && ...)` would now report only the `NOT` denial
and skip checking `denied_fn()`). Consider separating (1) validation/diagnostic
collection from (2) code emission: record the policy violation but still visit
relevant child nodes to collect other errors, while gating only the output
generation.
--
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]
