From a3291fe5bf914253357ac47ab330f64364c2c982 Mon Sep 17 00:00:00 2001
From: Masaori Koshiba <masaori@apache.org>
Date: Wed, 16 Sep 2026 13:50:25 +0900
Subject: [PATCH 1/4] hrw4u: add an AST round-trip test over the whole corpus

Render every corpus input's AST back to hrw4u and require the compiled
config to be unchanged: whatever the AST drops, the config loses too.
Unlike hand-written cases, nothing has to be enumerated in advance.

It found five losses, all fixed here:

  - an empty `else { }` looked like no else clause, so a sandbox policy
    denying 'else' was evaded by writing one
  - comments were discarded, though five .conf goldens carry them
  - a bool assignment lost the spelling the emitter echoes back
  - parentheses were unwrapped, dropping the cond %{GROUP} they emit
  - a set and an iprange both became a tuple of IPValue, though
    in [1.2.3.4] emits (1.2.3.4) and in {1.2.3.4} emits {1.2.3.4}

IfBlock.has_else is required rather than defaulted, so a site that
rebuilds the node and forgets it fails instead of reopening the bypass.

A second test asserts the corpus reaches every grammar rule; a bare
$param value had no fixture, now added.
---
 tools/hrw4u/src/ast_nodes.py                  |  43 ++++-
 tools/hrw4u/src/ast_visitor.py                |  47 ++++--
 tools/hrw4u/tests/ast_unparse.py              | 149 ++++++++++++++++++
 .../procedures/local-bare-param.input.txt     |   7 +
 .../procedures/local-bare-param.output.txt    |   2 +
 .../denied-language-else-empty.error.txt      |   1 +
 .../denied-language-else-empty.input.txt      |   6 +
 .../denied-language-else-empty.sandbox.yaml   |   6 +
 tools/hrw4u/tests/test_ast_roundtrip.py       | 103 ++++++++++++
 tools/hrw4u/tests/test_ast_visitor.py         | 139 +++++++++++++---
 10 files changed, 455 insertions(+), 48 deletions(-)
 create mode 100644 tools/hrw4u/tests/ast_unparse.py
 create mode 100644 tools/hrw4u/tests/data/procedures/local-bare-param.input.txt
 create mode 100644 tools/hrw4u/tests/data/procedures/local-bare-param.output.txt
 create mode 100644 tools/hrw4u/tests/data/sandbox/denied-language-else-empty.error.txt
 create mode 100644 tools/hrw4u/tests/data/sandbox/denied-language-else-empty.input.txt
 create mode 100644 tools/hrw4u/tests/data/sandbox/denied-language-else-empty.sandbox.yaml
 create mode 100644 tools/hrw4u/tests/test_ast_roundtrip.py

diff --git a/tools/hrw4u/src/ast_nodes.py b/tools/hrw4u/src/ast_nodes.py
index acf5bacccb3..f7203fc7279 100644
--- a/tools/hrw4u/src/ast_nodes.py
+++ b/tools/hrw4u/src/ast_nodes.py
@@ -25,16 +25,21 @@
     "IdentValue",
     "IPValue",
     "ParamRef",
+    "BoolValue",
     "RegexValue",
+    "SetValue",
+    "IpRangeValue",
     "ValueExpr",
     "Node",
     "Target",
     "Assignment",
     "FunctionCall",
     "Break",
+    "Comment",
     "Comparison",
     "LogicalOp",
     "NotOp",
+    "Group",
     "BoolLiteral",
     "IdentCondition",
     "ElifBranch",
@@ -72,12 +77,27 @@ class ParamRef:
     raw: str
 
 
+@dataclass(frozen=True, kw_only=True)
+class BoolValue:
+    raw: str  # source spelling, e.g. "TRUE", "true", "TRue"
+
+
 @dataclass(frozen=True, kw_only=True)
 class RegexValue:
     raw: str
 
 
-ValueExpr = Union[LiteralStringValue, IdentValue, IPValue, ParamRef, int, bool, tuple[IPValue, ...]]
+@dataclass(frozen=True, kw_only=True)
+class SetValue:
+    raw: str  # bracket-stripped source text, quoting preserved
+
+
+@dataclass(frozen=True, kw_only=True)
+class IpRangeValue:
+    raw: str  # verbatim source text
+
+
+ValueExpr = Union[LiteralStringValue, IdentValue, IPValue, ParamRef, BoolValue, int, bool, IpRangeValue]
 
 
 @dataclass(frozen=True, kw_only=True)
@@ -119,11 +139,16 @@ class Break(Node):
     pass
 
 
+@dataclass(frozen=True, kw_only=True)
+class Comment(Node):
+    text: str
+
+
 @dataclass(frozen=True, kw_only=True)
 class Comparison(Node):
     left: IdentValue | FunctionCall
     operator: str  # "==", "!=", ">", "<", "~", "!~", "in", "!in"
-    right: ValueExpr | RegexValue | tuple[ValueExpr, ...]
+    right: ValueExpr | RegexValue | SetValue | IpRangeValue
     modifiers: tuple[str, ...]
 
 
@@ -139,6 +164,11 @@ class NotOp(Node):
     operand: ConditionExpr
 
 
+@dataclass(frozen=True, kw_only=True)
+class Group(Node):
+    inner: ConditionExpr
+
+
 @dataclass(frozen=True, kw_only=True)
 class BoolLiteral(Node):
     value: bool
@@ -161,6 +191,7 @@ class IfBlock(Node):
     body: tuple[BodyNode, ...]
     elif_branches: tuple[ElifBranch, ...]
     else_body: tuple[BodyNode, ...]
+    has_else: bool  # an empty else body is not the same as no else clause
 
 
 @dataclass(frozen=True, kw_only=True)
@@ -185,7 +216,7 @@ class VarDecl(Node):
 @dataclass(frozen=True, kw_only=True)
 class VarSection(Node):
     scope: str
-    declarations: tuple[VarDecl, ...]
+    declarations: tuple[VarDecl | Comment, ...]
 
 
 @dataclass(frozen=True, kw_only=True)
@@ -206,6 +237,6 @@ class HRW4UAST:
 
 
 # Type aliases: must follow all class definitions (evaluated at runtime).
-ConditionExpr = Union[Comparison, LogicalOp, NotOp, BoolLiteral, IdentCondition, FunctionCall]
-BodyNode = Union[Assignment, FunctionCall, IfBlock, Break]
-TopLevelNode = Union[UseDirective, VarSection, ProcedureDecl, Section]
+ConditionExpr = Union[Comparison, LogicalOp, NotOp, Group, BoolLiteral, IdentCondition, FunctionCall]
+BodyNode = Union[Assignment, FunctionCall, IfBlock, Break, Comment]
+TopLevelNode = Union[UseDirective, VarSection, ProcedureDecl, Section, Comment]
diff --git a/tools/hrw4u/src/ast_visitor.py b/tools/hrw4u/src/ast_visitor.py
index 4a66ec0a710..153bcd1ba8f 100644
--- a/tools/hrw4u/src/ast_visitor.py
+++ b/tools/hrw4u/src/ast_visitor.py
@@ -29,6 +29,9 @@ class ASTVisitor(hrw4uVisitor):
     # method has an explicit return type and full control over how
     # child results are assembled into parent AST nodes.
 
+    def _visit_comment(self, ctx) -> Comment:
+        return Comment(text=ctx.COMMENT().getText(), line=ctx.start.line)
+
     def visitProgram(self, ctx) -> HRW4UAST:
         items = []
         for item in ctx.programItem():
@@ -39,7 +42,7 @@ def visitProgram(self, ctx) -> HRW4UAST:
             elif item.section() is not None:
                 items.append(self._visit_section(item.section()))
             elif item.commentLine() is not None:
-                pass
+                items.append(self._visit_comment(item.commentLine()))
             else:
                 raise ValueError(f"Unhandled programItem alternative at line {item.start.line}")
         return HRW4UAST(body=tuple(items))
@@ -75,7 +78,7 @@ def _visit_var_section(self, ctx, scope) -> VarSection:
             if var_item.variableDecl() is not None:
                 decls.append(self._visit_var_decl(var_item.variableDecl()))
             elif var_item.commentLine() is not None:
-                pass
+                decls.append(self._visit_comment(var_item.commentLine()))
             else:
                 raise ValueError(f"Unhandled variablesItem alternative at line {var_item.start.line}")
         return VarSection(scope=scope, declarations=tuple(decls), line=ctx.start.line)
@@ -93,7 +96,7 @@ def _visit_body(self, items) -> list[BodyNode]:
             elif item.conditional() is not None:
                 result.append(self._visit_conditional(item.conditional()))
             elif item.commentLine() is not None:
-                pass
+                result.append(self._visit_comment(item.commentLine()))
             else:
                 raise ValueError(f"Unhandled body item alternative at line {item.start.line}")
         return result
@@ -106,11 +109,11 @@ def _visit_statement(self, ctx) -> BodyNode:
             return self._visit_function_call(ctx.functionCall())
         if ctx.EQUAL():
             target = Target.from_dotted(ctx.lhs.text)
-            value = self._extract_value(ctx.value())
+            value = self._extract_assignment_value(ctx.value())
             return Assignment(target=target, operator="=", value=value, line=line)
         if ctx.PLUSEQUAL():
             target = Target.from_dotted(ctx.lhs.text)
-            value = self._extract_value(ctx.value())
+            value = self._extract_assignment_value(ctx.value())
             return Assignment(target=target, operator="+=", value=value, line=line)
         if ctx.op:
             return FunctionCall(name=ctx.op.text, args=(), line=line)
@@ -123,6 +126,14 @@ def _visit_function_call(self, ctx) -> FunctionCall:
             args = tuple(self._extract_value(v) for v in ctx.argumentList().value())
         return FunctionCall(name=name, args=args, line=ctx.start.line)
 
+    def _extract_assignment_value(self, ctx) -> ValueExpr:
+        # Only an assignment RHS echoes its bool spelling; elsewhere a plain bool is enough.
+        if ctx.TRUE():
+            return BoolValue(raw=ctx.TRUE().getText())
+        if ctx.FALSE():
+            return BoolValue(raw=ctx.FALSE().getText())
+        return self._extract_value(ctx)
+
     def _extract_value(self, ctx) -> ValueExpr:
         if ctx.number is not None:
             return int(ctx.number.text)
@@ -137,7 +148,7 @@ def _extract_value(self, ctx) -> ValueExpr:
         if ctx.ip():
             return IPValue(raw=ctx.ip().getText())
         if ctx.iprange():
-            return tuple(IPValue(raw=ip.getText()) for ip in ctx.iprange().ip())
+            return IpRangeValue(raw=ctx.iprange().getText())
         if ctx.paramRef():
             return ParamRef(raw=ctx.paramRef().IDENT().getText())
         raise ValueError(f"Unhandled value alternative at line {ctx.start.line}")
@@ -155,13 +166,17 @@ def _visit_conditional(self, ctx) -> IfBlock:
             elif_body = tuple(self._visit_body(elif_block.blockItem())) if elif_block else ()
             elif_branches.append(ElifBranch(condition=elif_cond, body=elif_body, line=elif_ctx.start.line))
 
-        else_body = ()
-        if ctx.elseClause():
-            else_block = ctx.elseClause().block()
-            if else_block:
-                else_body = tuple(self._visit_body(else_block.blockItem()))
+        else_clause = ctx.elseClause()
+        else_block = else_clause.block() if else_clause else None
+        else_body = tuple(self._visit_body(else_block.blockItem())) if else_block else ()
 
-        return IfBlock(condition=condition, body=body, elif_branches=tuple(elif_branches), else_body=else_body, line=ctx.start.line)
+        return IfBlock(
+            condition=condition,
+            body=body,
+            elif_branches=tuple(elif_branches),
+            else_body=else_body,
+            has_else=else_clause is not None,
+            line=ctx.start.line)
 
     def _visit_condition(self, ctx) -> ConditionExpr:
         return self._visit_expression(ctx.expression())
@@ -184,7 +199,7 @@ def _visit_factor(self, ctx) -> ConditionExpr:
         if ctx.getChildCount() == 2 and ctx.getChild(0).getText() == "!":
             return NotOp(operand=self._visit_factor(ctx.factor()), line=ctx.start.line)
         if ctx.LPAREN():
-            return self._visit_expression(ctx.expression())
+            return Group(inner=self._visit_expression(ctx.expression()), line=ctx.start.line)
         if ctx.functionCall():
             return self._visit_function_call(ctx.functionCall())
         if ctx.comparison():
@@ -231,14 +246,14 @@ def _detect_comparison_operator(self, ctx) -> str:
             return "in"
         raise ValueError(f"Unhandled comparison operator at line {ctx.start.line}")
 
-    def _extract_comparison_rhs(self, ctx, operator) -> ValueExpr | RegexValue | tuple[ValueExpr, ...]:
+    def _extract_comparison_rhs(self, ctx, operator) -> ValueExpr | RegexValue | SetValue | IpRangeValue:
         if operator in ("~", "!~"):
             return RegexValue(raw=ctx.regex().getText()[1:-1])
         if operator in ("in", "!in"):
             if ctx.set_():
-                return tuple(self._extract_value(v) for v in ctx.set_().value())
+                return SetValue(raw=ctx.set_().getText()[1:-1])
             if ctx.iprange():
-                return tuple(IPValue(raw=ip.getText()) for ip in ctx.iprange().ip())
+                return IpRangeValue(raw=ctx.iprange().getText())
         if ctx.value():
             return self._extract_value(ctx.value())
         raise ValueError(f"Unhandled comparison RHS at line {ctx.start.line}")
diff --git a/tools/hrw4u/tests/ast_unparse.py b/tools/hrw4u/tests/ast_unparse.py
new file mode 100644
index 00000000000..2fc4408067a
--- /dev/null
+++ b/tools/hrw4u/tests/ast_unparse.py
@@ -0,0 +1,149 @@
+#
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+"""Render an AST back to hrw4u source. Test-only; see test_ast_roundtrip.py for why.
+
+Whitespace is not reproduced: the emitter derives its own indentation and never reads the
+source's.
+"""
+
+from __future__ import annotations
+
+from hrw4u.ast_nodes import *
+
+INDENT = "    "
+
+
+def unparse(ast: HRW4UAST) -> str:
+    return "\n".join(_top_level(item) for item in ast.body) + "\n"
+
+
+def _top_level(node: TopLevelNode) -> str:
+    match node:
+        case Comment():
+            return node.text
+        case UseDirective():
+            return f"use {node.spec}"
+        case ProcedureDecl():
+            params = ", ".join(_proc_param(p) for p in node.params)
+            return _braced(f"procedure {node.name}({params})", [_body_item(b, 1) for b in node.body])
+        case VarSection():
+            keyword = "SESSION_VARS" if node.scope == "session" else "VARS"
+            return _braced(keyword, [_var_item(d) for d in node.declarations])
+        case Section():
+            return _braced(node.type, [_body_item(b, 1) for b in node.body])
+    raise ValueError(f"unparse: unhandled top-level node {type(node).__name__}")
+
+
+def _braced(header: str, lines: list[str]) -> str:
+    return "\n".join([f"{header} {{", *(f"{INDENT}{line}" for line in lines), "}"])
+
+
+def _proc_param(p: ProcParam) -> str:
+    return f"${p.name}" if p.default is None else f"${p.name}={_value(p.default)}"
+
+
+def _var_item(node: VarDecl | Comment) -> str:
+    if isinstance(node, Comment):
+        return node.text
+    slot = "" if node.slot is None else f" @{node.slot}"
+    return f"{node.name}: {node.type_name}{slot};"
+
+
+def _body_item(node: BodyNode, depth: int) -> str:
+    match node:
+        case Comment():
+            return node.text
+        case Break():
+            return "break;"
+        case FunctionCall():
+            return f"{_call(node)};"
+        case Assignment():
+            return f"{_target(node.target)} {node.operator} {_value(node.value)};"
+        case IfBlock():
+            return _if_block(node, depth)
+    raise ValueError(f"unparse: unhandled body node {type(node).__name__}")
+
+
+def _if_block(node: IfBlock, depth: int) -> str:
+    pad = INDENT * depth
+    lines = [f"if {_condition(node.condition)} {{"]
+    lines += [f"{INDENT}{line}" for line in _nested(node.body, depth)]
+    for arm in node.elif_branches:
+        lines.append(f"}} elif {_condition(arm.condition)} {{")
+        lines += [f"{INDENT}{line}" for line in _nested(arm.body, depth)]
+    if node.has_else:
+        lines.append("} else {")
+        lines += [f"{INDENT}{line}" for line in _nested(node.else_body, depth)]
+    lines.append("}")
+    return f"\n{pad}".join(lines)
+
+
+def _nested(body: tuple[BodyNode, ...], depth: int) -> list[str]:
+    return [line for item in body for line in _body_item(item, depth + 1).splitlines()]
+
+
+def _target(t: Target) -> str:
+    return t.field if t.namespace is None else f"{t.namespace}.{t.field}"
+
+
+def _call(node: FunctionCall) -> str:
+    return f"{node.name}({', '.join(_value(a) for a in node.args)})"
+
+
+def _condition(node: ConditionExpr) -> str:
+    match node:
+        case Group():
+            return f"({_condition(node.inner)})"
+        case LogicalOp():
+            return f"{_condition(node.left)} {node.operator} {_condition(node.right)}"
+        case NotOp():
+            return f"!{_condition(node.operand)}"
+        case BoolLiteral():
+            return "true" if node.value else "false"
+        case IdentCondition():
+            return node.name
+        case FunctionCall():
+            return _call(node)
+        case Comparison():
+            return _comparison(node)
+    raise ValueError(f"unparse: unhandled condition node {type(node).__name__}")
+
+
+def _comparison(node: Comparison) -> str:
+    left = node.left.raw if isinstance(node.left, IdentValue) else _call(node.left)
+    mods = f" with {', '.join(node.modifiers)}" if node.modifiers else ""
+    return f"{left} {node.operator} {_value(node.right)}{mods}"
+
+
+def _value(v) -> str:
+    # bool first: it is a subclass of int, so the int arm would swallow it.
+    if isinstance(v, bool):
+        return "true" if v else "false"
+    match v:
+        case int():
+            return str(v)
+        case LiteralStringValue():
+            return f'"{v.raw}"'
+        case BoolValue() | IdentValue() | IPValue() | IpRangeValue():
+            return v.raw
+        case ParamRef():
+            return f"${v.raw}"
+        case RegexValue():
+            return f"/{v.raw}/"
+        case SetValue():
+            return f"[{v.raw}]"
+    raise ValueError(f"unparse: unhandled value {type(v).__name__}")
diff --git a/tools/hrw4u/tests/data/procedures/local-bare-param.input.txt b/tools/hrw4u/tests/data/procedures/local-bare-param.input.txt
new file mode 100644
index 00000000000..1cbee7941d1
--- /dev/null
+++ b/tools/hrw4u/tests/data/procedures/local-bare-param.input.txt
@@ -0,0 +1,7 @@
+procedure local::tag($name) {
+    inbound.req.X-Tag = $name;
+}
+
+REMAP {
+    local::tag("hello");
+}
diff --git a/tools/hrw4u/tests/data/procedures/local-bare-param.output.txt b/tools/hrw4u/tests/data/procedures/local-bare-param.output.txt
new file mode 100644
index 00000000000..82acc54b246
--- /dev/null
+++ b/tools/hrw4u/tests/data/procedures/local-bare-param.output.txt
@@ -0,0 +1,2 @@
+cond %{REMAP_PSEUDO_HOOK} [AND]
+    set-header X-Tag hello
diff --git a/tools/hrw4u/tests/data/sandbox/denied-language-else-empty.error.txt b/tools/hrw4u/tests/data/sandbox/denied-language-else-empty.error.txt
new file mode 100644
index 00000000000..4aa2d20b1d5
--- /dev/null
+++ b/tools/hrw4u/tests/data/sandbox/denied-language-else-empty.error.txt
@@ -0,0 +1 @@
+'else' is denied by sandbox policy (language)
diff --git a/tools/hrw4u/tests/data/sandbox/denied-language-else-empty.input.txt b/tools/hrw4u/tests/data/sandbox/denied-language-else-empty.input.txt
new file mode 100644
index 00000000000..dcaf3a6d84d
--- /dev/null
+++ b/tools/hrw4u/tests/data/sandbox/denied-language-else-empty.input.txt
@@ -0,0 +1,6 @@
+REMAP {
+    if inbound.req.X-Foo == "a" {
+        inbound.req.X-Result = "yes";
+    } else {
+    }
+}
diff --git a/tools/hrw4u/tests/data/sandbox/denied-language-else-empty.sandbox.yaml b/tools/hrw4u/tests/data/sandbox/denied-language-else-empty.sandbox.yaml
new file mode 100644
index 00000000000..58c2bde66b0
--- /dev/null
+++ b/tools/hrw4u/tests/data/sandbox/denied-language-else-empty.sandbox.yaml
@@ -0,0 +1,6 @@
+sandbox:
+  message: "Feature denied by sandbox policy. Contact platform team."
+
+  deny:
+    language:
+      - else
diff --git a/tools/hrw4u/tests/test_ast_roundtrip.py b/tools/hrw4u/tests/test_ast_roundtrip.py
new file mode 100644
index 00000000000..c3dc3fdda5d
--- /dev/null
+++ b/tools/hrw4u/tests/test_ast_roundtrip.py
@@ -0,0 +1,103 @@
+#
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+"""The AST must carry everything the emitter reads out of the source.
+
+Hand-written cases only catch losses someone thought of; an empty `else { }`, a dropped
+comment and a `TRUE`/`true` spelling all shipped because nobody did. So instead of naming
+distinctions, run the whole corpus through the AST and back and require the compiled
+config to be unchanged: whatever the AST drops, the config loses too.
+
+        corpus .hrw4u  --parse-->  AST  --unparse-->  regenerated .hrw4u
+              |                                              |
+            emit                                           emit
+              |                                              |
+              v                                              v
+           config  <----------- must match ------------>  config
+
+The comparison is the compiled config, never the regenerated source text. An AST holds no
+whitespace, indentation or blank lines, and the emitter reads none of them -- it even
+indents a preserved comment by nesting depth rather than by the column it came from. So
+demanding byte-identical source would fail on almost every input for reasons that change
+no output, and satisfying it would mean turning the AST back into a CST.
+
+A companion test asserts the corpus reaches every grammar rule, so a rule with no fixture
+is reported rather than silently unguarded.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+from antlr4 import ParserRuleContext
+
+import ast_unparse
+import utils
+from hrw4u.ast_visitor import ASTVisitor
+from hrw4u.hrw4uParser import hrw4uParser
+from hrw4u.visitor import HRW4UVisitor
+
+CORPUS = Path("tests/data")
+
+# Rejected before any sandbox check; named so a newly-broken input fails instead of dropping out.
+DOES_NOT_COMPILE = frozenset({"sandbox/per-test-sandbox"})
+
+
+def _case_id(input_file: Path) -> str:
+    return f"{input_file.parent.name}/{input_file.name.removesuffix('.input.txt')}"
+
+
+def _cases() -> list[pytest.param]:
+    files = (f for f in sorted(CORPUS.glob("*/*.input.txt")) if ".fail." not in f.name)
+    return [pytest.param(f, id=_case_id(f)) for f in files if _case_id(f) not in DOES_NOT_COMPILE]
+
+
+def _compile(text: str, input_file: Path) -> list[str]:
+    _, tree = utils.parse_input_text(text)
+    visitor = HRW4UVisitor(filename=str(input_file), proc_search_paths=[input_file.parent / "procs"])
+    return visitor.visit(tree)
+
+
+@pytest.mark.parametrize("input_file", _cases())
+def test_emitted_config_survives_a_round_trip_through_the_ast(input_file: Path) -> None:
+    source = input_file.read_text()
+    _, tree = utils.parse_input_text(source)
+    regenerated = ast_unparse.unparse(ASTVisitor().visit(tree))
+
+    expected = _compile(source, input_file)
+    # An empty config would pass no matter what the AST drops.
+    assert expected, f"{input_file} compiles to nothing; it cannot witness a round trip"
+    assert _compile(regenerated, input_file) == expected, (
+        f"{input_file}: the AST lost something the emitter reads.\n"
+        f"--- regenerated hrw4u ---\n{regenerated}")
+
+
+def test_the_corpus_reaches_every_grammar_rule() -> None:
+    reached: set[str] = set()
+
+    def walk(ctx) -> None:
+        if isinstance(ctx, ParserRuleContext):
+            reached.add(hrw4uParser.ruleNames[ctx.getRuleIndex()])
+            for child in ctx.getChildren():
+                walk(child)
+
+    for param in _cases():
+        _, tree = utils.parse_input_text(param.values[0].read_text())
+        walk(tree)
+
+    missing = set(hrw4uParser.ruleNames) - reached
+    assert not missing, f"no corpus input exercises: {', '.join(sorted(missing))}"
diff --git a/tools/hrw4u/tests/test_ast_visitor.py b/tools/hrw4u/tests/test_ast_visitor.py
index ec919d1f060..677a4c4a786 100644
--- a/tools/hrw4u/tests/test_ast_visitor.py
+++ b/tools/hrw4u/tests/test_ast_visitor.py
@@ -15,6 +15,8 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 
+import pytest
+
 from hrw4u.ast_nodes import *
 from utils import parse_input_text
 from hrw4u.ast_visitor import ASTVisitor
@@ -39,7 +41,20 @@ def test_bool_value(self):
         ast = _build('SEND_RESPONSE {\n    http.cntl.TXN_DEBUG = true;\n}')
         a = ast.body[0].body[0]
         assert isinstance(a, Assignment)
-        assert a.value is True
+        assert a.value == BoolValue(raw="true")
+
+    def test_bool_assignment_keeps_source_spelling(self):
+        """The emitter echoes the spelling back, so the RHS cannot normalize to a plain bool."""
+        upper = _build('SEND_RESPONSE {\n    http.cntl.TXN_DEBUG = TRUE;\n}').body[0].body[0]
+        lower = _build('SEND_RESPONSE {\n    http.cntl.TXN_DEBUG = true;\n}').body[0].body[0]
+        assert upper.value == BoolValue(raw="TRUE")
+        assert lower.value == BoolValue(raw="true")
+
+    def test_bool_outside_an_assignment_stays_a_plain_bool(self):
+        src = 'procedure local::p($on=true, $off=FALSE) {\n    set-debug();\n}\nREMAP {\n    set-debug();\n}'
+        pd = _build(src).body[0]
+        assert pd.params[0].default is True
+        assert pd.params[1].default is False
 
     def test_int_value(self):
         ast = _build('REMAP {\n    http.cntl.INTERCEPT_RETRY = 1;\n}')
@@ -57,6 +72,16 @@ def test_ip_value(self):
         assert isinstance(a, Assignment)
         assert a.value == IPValue(raw="10.0.0.1")
 
+    def test_ident_value(self):
+        src = 'VARS {\n    a: bool;\n    b: bool;\n}\nREMAP {\n    b = a;\n}'
+        a = _build(src).body[1].body[0]
+        assert isinstance(a, Assignment)
+        assert a.value == IdentValue(raw="a")
+
+    def test_iprange_value(self):
+        a = _build('REMAP {\n    inbound.req.X = {1.2.3.4, 5.6.7.8};\n}').body[0].body[0]
+        assert a.value == IpRangeValue(raw="{1.2.3.4,5.6.7.8}")
+
     def test_param_ref_value(self):
         src = 'procedure local::stamp($tag) {\n    inbound.req.X-Stamp = $tag;\n}\nREMAP {\n    set-debug();\n}'
         ast = _build(src)
@@ -95,15 +120,15 @@ def test_break(self):
 
 class TestSections:
 
-    def test_comments_in_section_body_skipped(self):
+    def test_comments_in_section_body_preserved(self):
         src = 'REMAP {\n    # a comment\n    set-debug();\n    # another comment\n}'
         ast = _build(src)
-        assert len(ast.body[0].body) == 1
+        assert len(ast.body[0].body) == 3
 
-    def test_comments_in_block_skipped(self):
+    def test_comments_in_block_preserved(self):
         src = 'REMAP {\n    if true {\n        # comment\n        set-debug();\n    }\n}'
         ast = _build(src)
-        assert len(ast.body[0].body[0].body) == 1
+        assert len(ast.body[0].body[0].body) == 2
 
     def test_section_type(self):
         ast = _build('REMAP {\n    set-debug();\n}')
@@ -138,12 +163,12 @@ def test_item_ordering(self):
 
 class TestVarSections:
 
-    def test_comments_in_var_section_skipped(self):
+    def test_comments_in_var_section_preserved(self):
         src = 'VARS {\n    # comment\n    x: bool;\n    # another\n    y: int;\n}\nREMAP {\n    set-debug();\n}'
         ast = _build(src)
         vs = ast.body[0]
         assert isinstance(vs, VarSection)
-        assert len(vs.declarations) == 2
+        assert len(vs.declarations) == 4
 
     def test_txn_scope(self):
         src = 'VARS {\n    flag: bool;\n}\nREMAP {\n    set-debug();\n}'
@@ -237,7 +262,7 @@ def test_in_set(self):
         cond = self._first_condition('REMAP {\n    if inbound.url.path in ["a", "b"] {\n        set-debug();\n    }\n}')
         assert isinstance(cond, Comparison)
         assert cond.operator == "in"
-        assert cond.right == (LiteralStringValue(raw="a"), LiteralStringValue(raw="b"))
+        assert cond.right == SetValue(raw='"a","b"')
 
     def test_not_in_set(self):
         cond = self._first_condition('REMAP {\n    if inbound.url.path !in ["a"] {\n        set-debug();\n    }\n}')
@@ -248,7 +273,7 @@ def test_in_iprange(self):
         cond = self._first_condition('REMAP {\n    if inbound.ip in {10.0.0.0/8} {\n        set-debug();\n    }\n}')
         assert isinstance(cond, Comparison)
         assert cond.operator == "in"
-        assert cond.right == (IPValue(raw="10.0.0.0/8"),)
+        assert cond.right == IpRangeValue(raw="{10.0.0.0/8}")
 
     def test_modifiers(self):
         cond = self._first_condition('REMAP {\n    if inbound.req.X-Foo == "bar" with NOCASE {\n        set-debug();\n    }\n}')
@@ -328,9 +353,25 @@ def test_neq_comparison(self):
 
     def test_parenthesized_condition(self):
         cond = self._first_condition('REMAP {\n    if (inbound.req.X-Foo == "bar") {\n        set-debug();\n    }\n}')
-        assert isinstance(cond, Comparison)
-        assert cond.operator == "=="
-        assert cond.right == LiteralStringValue(raw="bar")
+        assert isinstance(cond, Group)
+        assert isinstance(cond.inner, Comparison)
+        assert cond.inner.operator == "=="
+        assert cond.inner.right == LiteralStringValue(raw="bar")
+
+    def test_parens_are_kept(self):
+        """A parenthesized factor becomes cond %{GROUP}."""
+        grouped = self._first_condition('REMAP {\n    if (true) {\n        set-debug();\n    }\n}')
+        bare = self._first_condition('REMAP {\n    if true {\n        set-debug();\n    }\n}')
+        assert isinstance(grouped, Group)
+        assert isinstance(grouped.inner, BoolLiteral)
+        assert isinstance(bare, BoolLiteral)
+
+    def test_set_and_iprange_are_distinguishable(self):
+        """`in [1.2.3.4]` emits (1.2.3.4) but `in {1.2.3.4}` emits {1.2.3.4}."""
+        as_set = self._first_condition('REMAP {\n    if inbound.ip in [1.2.3.4] {\n        set-debug();\n    }\n}')
+        as_range = self._first_condition('REMAP {\n    if inbound.ip in {1.2.3.4} {\n        set-debug();\n    }\n}')
+        assert as_set.right == SetValue(raw="1.2.3.4")
+        assert as_range.right == IpRangeValue(raw="{1.2.3.4}")
 
     def test_and_binds_tighter_than_or(self):
         # a || b && c  should parse as  a || (b && c)
@@ -370,9 +411,9 @@ def test_not_comparison_with_or(self):
         assert isinstance(cond, LogicalOp)
         assert cond.operator == "||"
         assert isinstance(cond.left, NotOp)
-        assert isinstance(cond.left.operand, Comparison)
-        assert cond.left.operand.left == IdentValue(raw="inbound.req.X-A")
-        assert cond.left.operand.right == LiteralStringValue(raw="x")
+        assert isinstance(cond.left.operand, Group)
+        assert cond.left.operand.inner.left == IdentValue(raw="inbound.req.X-A")
+        assert cond.left.operand.inner.right == LiteralStringValue(raw="x")
         assert isinstance(cond.right, Comparison)
         assert cond.right.left == IdentValue(raw="inbound.req.X-B")
 
@@ -397,10 +438,10 @@ def test_parens_override_precedence(self):
             '        set-debug();\n    }\n}')
         assert isinstance(cond, LogicalOp)
         assert cond.operator == "&&"
-        assert isinstance(cond.left, LogicalOp)
-        assert cond.left.operator == "||"
-        assert cond.left.left.left == IdentValue(raw="inbound.req.X-A")
-        assert cond.left.right.left == IdentValue(raw="inbound.req.X-B")
+        assert isinstance(cond.left, Group)
+        assert cond.left.inner.operator == "||"
+        assert cond.left.inner.left.left == IdentValue(raw="inbound.req.X-A")
+        assert cond.left.inner.right.left == IdentValue(raw="inbound.req.X-B")
         assert isinstance(cond.right, Comparison)
         assert cond.right.left == IdentValue(raw="inbound.req.X-C")
 
@@ -413,8 +454,8 @@ def test_nested_parens_with_not(self):
         assert isinstance(cond, LogicalOp)
         assert cond.operator == "&&"
         assert isinstance(cond.left, NotOp)
-        assert isinstance(cond.left.operand, LogicalOp)
-        assert cond.left.operand.operator == "||"
+        assert isinstance(cond.left.operand, Group)
+        assert cond.left.operand.inner.operator == "||"
         assert isinstance(cond.right, Comparison)
         assert cond.right.left == IdentValue(raw="inbound.req.X-C")
 
@@ -433,8 +474,32 @@ def test_if_else(self):
         src = 'REMAP {\n    if true {\n        inbound.req.X = "a";\n    } else {\n        inbound.req.X = "b";\n    }\n}'
         ast = _build(src)
         ib = ast.body[0].body[0]
+        assert ib.has_else is True
         assert len(ib.else_body) == 1
 
+    def test_empty_else_is_distinguishable_from_no_else(self):
+        """Gating on else_body alone lets a sandbox policy denying 'else' be evaded."""
+        no_else = _build('REMAP {\n    if true {\n        inbound.req.X = "y";\n    }\n}').body[0].body[0]
+        empty_else = _build('REMAP {\n    if true {\n        inbound.req.X = "y";\n    } else {\n    }\n}').body[0].body[0]
+        assert no_else.has_else is False
+        assert empty_else.has_else is True
+        assert empty_else.else_body == ()
+
+    def test_empty_else_after_elif_sets_has_else(self):
+        src = (
+            'REMAP {\n    if inbound.req.X == "a" {\n        set-debug();\n'
+            '    } elif inbound.req.X == "b" {\n        set-debug();\n'
+            '    } else {\n    }\n}')
+        ib = _build(src).body[0].body[0]
+        assert len(ib.elif_branches) == 1
+        assert ib.has_else is True
+        assert ib.else_body == ()
+
+    def test_has_else_is_required(self):
+        """A default would silently mean "no else" at any node-rebuilding site."""
+        with pytest.raises(TypeError):
+            IfBlock(condition=BoolLiteral(value=True, line=1), body=(), elif_branches=(), else_body=(), line=1)
+
     def test_if_elif_else(self):
         src = (
             'SEND_RESPONSE {\n    if inbound.url.path == "foo" {\n'
@@ -686,8 +751,8 @@ def test_http_cntl_booleans(self):
 }'''
         ast = _build(src)
         body = ast.body[0].body
-        assert body[0].value is True
-        assert body[1].value is False
+        assert body[0].value == BoolValue(raw="true")
+        assert body[1].value == BoolValue(raw="FALSE")
 
     def test_ip_range_condition(self):
         """Validates IP range handling from tests/data/conds/ip.input.txt."""
@@ -700,7 +765,7 @@ def test_ip_range_condition(self):
         cond = ast.body[0].body[0].condition
         assert isinstance(cond, Comparison)
         assert cond.operator == "in"
-        assert len(cond.right) == 2
+        assert cond.right == IpRangeValue(raw="{192.168.0.0/16,10.0.0.0/8}")
 
     def test_set_membership_with_modifier(self):
         """From tests/data/conds/in-sets.input.txt."""
@@ -713,7 +778,7 @@ def test_set_membership_with_modifier(self):
         cond = ast.body[0].body[0].condition
         assert isinstance(cond, Comparison)
         assert cond.operator == "in"
-        assert cond.right == (LiteralStringValue(raw="php"), LiteralStringValue(raw="php3"), LiteralStringValue(raw="php4"))
+        assert cond.right == SetValue(raw='"php","php3","php4"')
         assert cond.modifiers == ("EXT",)
 
     def test_debug_pattern_for_lint_rules(self):
@@ -733,8 +798,30 @@ def test_debug_pattern_for_lint_rules(self):
         # TXN_DEBUG assignment with True
         assert isinstance(body[1], Assignment)
         assert body[1].target == Target.from_dotted("http.cntl.TXN_DEBUG")
-        assert body[1].value is True
+        assert body[1].value == BoolValue(raw="true")
 
         # Regular assignment (not flagged)
         assert isinstance(body[2], Assignment)
         assert body[2].target.namespace == "inbound.req"
+
+
+class TestComments:
+
+    def test_top_level_comment_preserved(self):
+        ast = _build('# hello\nREMAP {\n    set-debug();\n}')
+        assert ast.body[0] == Comment(text="# hello", line=1)
+
+    def test_comment_in_section_body_keeps_position(self):
+        body = _build('REMAP {\n    # first\n    set-debug();\n}').body[0].body
+        assert isinstance(body[0], Comment)
+        assert body[0].text == "# first"
+        assert isinstance(body[1], FunctionCall)
+
+    def test_comment_in_block(self):
+        ast = _build('REMAP {\n    if inbound.status > 399 {\n        # why\n        set-debug();\n    }\n}')
+        assert isinstance(ast.body[0].body[0].body[0], Comment)
+
+    def test_comment_in_vars_section(self):
+        decls = _build('VARS {\n    # a counter\n    hits: int8;\n}').body[0].declarations
+        assert isinstance(decls[0], Comment)
+        assert isinstance(decls[1], VarDecl)

From 8eb2e735576984d8847f80245d127a9e86df3e93 Mon Sep 17 00:00:00 2001
From: Masaori Koshiba <masaori@apache.org>
Date: Wed, 16 Sep 2026 15:20:39 +0900
Subject: [PATCH 2/4] hrw4u: keep bool spelling in every value context

The AST kept the source spelling only on an assignment RHS, so a TRUE
in a comparison or a function argument regenerated as true and changed
the emitted config. bool-spelling.input.txt witnesses both; the reverse
normalizes an argument's spelling, hence the exceptions.txt entry.
---
 tools/hrw4u/src/ast_nodes.py                    |  2 +-
 tools/hrw4u/src/ast_visitor.py                  | 17 +++++------------
 tools/hrw4u/tests/ast_unparse.py                |  5 +----
 .../tests/data/ops/bool-spelling.input.txt      |  7 +++++++
 .../tests/data/ops/bool-spelling.output.txt     |  5 +++++
 tools/hrw4u/tests/data/ops/exceptions.txt       |  2 ++
 tools/hrw4u/tests/test_ast_visitor.py           | 15 +++++++++++----
 7 files changed, 32 insertions(+), 21 deletions(-)
 create mode 100644 tools/hrw4u/tests/data/ops/bool-spelling.input.txt
 create mode 100644 tools/hrw4u/tests/data/ops/bool-spelling.output.txt

diff --git a/tools/hrw4u/src/ast_nodes.py b/tools/hrw4u/src/ast_nodes.py
index f7203fc7279..7f8cb8d5bb6 100644
--- a/tools/hrw4u/src/ast_nodes.py
+++ b/tools/hrw4u/src/ast_nodes.py
@@ -97,7 +97,7 @@ class IpRangeValue:
     raw: str  # verbatim source text
 
 
-ValueExpr = Union[LiteralStringValue, IdentValue, IPValue, ParamRef, BoolValue, int, bool, IpRangeValue]
+ValueExpr = Union[LiteralStringValue, IdentValue, IPValue, ParamRef, BoolValue, int, IpRangeValue]
 
 
 @dataclass(frozen=True, kw_only=True)
diff --git a/tools/hrw4u/src/ast_visitor.py b/tools/hrw4u/src/ast_visitor.py
index 153bcd1ba8f..f5fe470e623 100644
--- a/tools/hrw4u/src/ast_visitor.py
+++ b/tools/hrw4u/src/ast_visitor.py
@@ -109,11 +109,11 @@ def _visit_statement(self, ctx) -> BodyNode:
             return self._visit_function_call(ctx.functionCall())
         if ctx.EQUAL():
             target = Target.from_dotted(ctx.lhs.text)
-            value = self._extract_assignment_value(ctx.value())
+            value = self._extract_value(ctx.value())
             return Assignment(target=target, operator="=", value=value, line=line)
         if ctx.PLUSEQUAL():
             target = Target.from_dotted(ctx.lhs.text)
-            value = self._extract_assignment_value(ctx.value())
+            value = self._extract_value(ctx.value())
             return Assignment(target=target, operator="+=", value=value, line=line)
         if ctx.op:
             return FunctionCall(name=ctx.op.text, args=(), line=line)
@@ -126,23 +126,16 @@ def _visit_function_call(self, ctx) -> FunctionCall:
             args = tuple(self._extract_value(v) for v in ctx.argumentList().value())
         return FunctionCall(name=name, args=args, line=ctx.start.line)
 
-    def _extract_assignment_value(self, ctx) -> ValueExpr:
-        # Only an assignment RHS echoes its bool spelling; elsewhere a plain bool is enough.
-        if ctx.TRUE():
-            return BoolValue(raw=ctx.TRUE().getText())
-        if ctx.FALSE():
-            return BoolValue(raw=ctx.FALSE().getText())
-        return self._extract_value(ctx)
-
     def _extract_value(self, ctx) -> ValueExpr:
         if ctx.number is not None:
+            # Drops a leading zero the emitter would echo; no corpus input writes one.
             return int(ctx.number.text)
         if ctx.str_ is not None:
             return LiteralStringValue(raw=ctx.str_.text[1:-1])
         if ctx.TRUE():
-            return True
+            return BoolValue(raw=ctx.TRUE().getText())
         if ctx.FALSE():
-            return False
+            return BoolValue(raw=ctx.FALSE().getText())
         if ctx.ident is not None:
             return IdentValue(raw=ctx.ident.text)
         if ctx.ip():
diff --git a/tools/hrw4u/tests/ast_unparse.py b/tools/hrw4u/tests/ast_unparse.py
index 2fc4408067a..ddc8937d405 100644
--- a/tools/hrw4u/tests/ast_unparse.py
+++ b/tools/hrw4u/tests/ast_unparse.py
@@ -129,10 +129,7 @@ def _comparison(node: Comparison) -> str:
     return f"{left} {node.operator} {_value(node.right)}{mods}"
 
 
-def _value(v) -> str:
-    # bool first: it is a subclass of int, so the int arm would swallow it.
-    if isinstance(v, bool):
-        return "true" if v else "false"
+def _value(v: ValueExpr | RegexValue | SetValue) -> str:
     match v:
         case int():
             return str(v)
diff --git a/tools/hrw4u/tests/data/ops/bool-spelling.input.txt b/tools/hrw4u/tests/data/ops/bool-spelling.input.txt
new file mode 100644
index 00000000000..750713155e8
--- /dev/null
+++ b/tools/hrw4u/tests/data/ops/bool-spelling.input.txt
@@ -0,0 +1,7 @@
+# The emitter echoes a bool's spelling, so the AST cannot normalize one anywhere:
+# an assignment RHS is not the only value context that reaches header_rewrite.
+REMAP {
+    if inbound.req.X-Debug == TRUE {
+        set-config("proxy.config.http.cache.http", FALSE);
+    }
+}
diff --git a/tools/hrw4u/tests/data/ops/bool-spelling.output.txt b/tools/hrw4u/tests/data/ops/bool-spelling.output.txt
new file mode 100644
index 00000000000..25575881097
--- /dev/null
+++ b/tools/hrw4u/tests/data/ops/bool-spelling.output.txt
@@ -0,0 +1,5 @@
+# The emitter echoes a bool's spelling, so the AST cannot normalize one anywhere:
+# an assignment RHS is not the only value context that reaches header_rewrite.
+cond %{REMAP_PSEUDO_HOOK} [AND]
+cond %{CLIENT-HEADER:X-Debug} =TRUE
+    set-config "proxy.config.http.cache.http" FALSE
diff --git a/tools/hrw4u/tests/data/ops/exceptions.txt b/tools/hrw4u/tests/data/ops/exceptions.txt
index a991b1217a7..49bb3b414fb 100644
--- a/tools/hrw4u/tests/data/ops/exceptions.txt
+++ b/tools/hrw4u/tests/data/ops/exceptions.txt
@@ -9,3 +9,5 @@ header_value_context.input: u4wrh
 http_cntl_valid_bools.input: hrw4u
 # Literal JSON blocks compile to native header_rewrite escaping.
 json-body.input: hrw4u
+# The reverse normalizes a bool argument's spelling (FALSE -> false)
+bool-spelling.input: hrw4u
diff --git a/tools/hrw4u/tests/test_ast_visitor.py b/tools/hrw4u/tests/test_ast_visitor.py
index 677a4c4a786..b419c5e9007 100644
--- a/tools/hrw4u/tests/test_ast_visitor.py
+++ b/tools/hrw4u/tests/test_ast_visitor.py
@@ -50,11 +50,12 @@ def test_bool_assignment_keeps_source_spelling(self):
         assert upper.value == BoolValue(raw="TRUE")
         assert lower.value == BoolValue(raw="true")
 
-    def test_bool_outside_an_assignment_stays_a_plain_bool(self):
+    def test_bool_keeps_source_spelling_outside_an_assignment(self):
+        """A procedure default is bound raw into its use site, so it echoes its spelling too."""
         src = 'procedure local::p($on=true, $off=FALSE) {\n    set-debug();\n}\nREMAP {\n    set-debug();\n}'
         pd = _build(src).body[0]
-        assert pd.params[0].default is True
-        assert pd.params[1].default is False
+        assert pd.params[0].default == BoolValue(raw="true")
+        assert pd.params[1].default == BoolValue(raw="FALSE")
 
     def test_int_value(self):
         ast = _build('REMAP {\n    http.cntl.INTERCEPT_RETRY = 1;\n}')
@@ -290,7 +291,7 @@ def test_function_call_comparable(self):
         assert isinstance(cond, Comparison)
         assert isinstance(cond.left, FunctionCall)
         assert cond.left.name == "url"
-        assert cond.left.args == (True,)
+        assert cond.left.args == (BoolValue(raw="true"),)
 
     def test_bool_literal_true(self):
         cond = self._first_condition('REMAP {\n    if true {\n        set-debug();\n    }\n}')
@@ -345,6 +346,12 @@ def test_less_than_comparison(self):
         assert cond.operator == "<"
         assert cond.right == 500
 
+    def test_comparison_rhs_keeps_bool_spelling(self):
+        """`== TRUE` emits `=TRUE`, so normalizing the RHS would change the config."""
+        cond = self._first_condition('REMAP {\n    if inbound.req.X-Debug == TRUE {\n        set-debug();\n    }\n}')
+        assert isinstance(cond, Comparison)
+        assert cond.right == BoolValue(raw="TRUE")
+
     def test_neq_comparison(self):
         cond = self._first_condition('REMAP {\n    if inbound.req.X-Foo != "bar" {\n        set-debug();\n    }\n}')
         assert isinstance(cond, Comparison)

From bea9efe2f7e8a2ffeb172e93257416122420483f Mon Sep 17 00:00:00 2001
From: Masaori Koshiba <masaori@apache.org>
Date: Thu, 17 Sep 2026 15:36:07 +0900
Subject: [PATCH 3/4] hrw4u: keep number spelling in every value context

The AST parsed a NUMBER into a Python int, so a leading zero the emitter
echoes back was lost: 007 became 7 in a header value, three bytes
becoming one. number-spelling.input.txt witnesses all three value
contexts; the digits survive in each.

A plain int was a deliberate call, on the grounds that no corpus input
wrote a leading zero. It did not survive a semantic pass built on the
AST, which matches structurally over ValueExpr and has nowhere to put a
naked int.
---
 tools/hrw4u/src/ast_nodes.py                          | 8 +++++++-
 tools/hrw4u/src/ast_visitor.py                        | 3 +--
 tools/hrw4u/tests/ast_unparse.py                      | 4 +---
 tools/hrw4u/tests/data/ops/number-spelling.input.txt  | 7 +++++++
 tools/hrw4u/tests/data/ops/number-spelling.output.txt | 5 +++++
 tools/hrw4u/tests/test_ast_visitor.py                 | 8 ++++----
 6 files changed, 25 insertions(+), 10 deletions(-)
 create mode 100644 tools/hrw4u/tests/data/ops/number-spelling.input.txt
 create mode 100644 tools/hrw4u/tests/data/ops/number-spelling.output.txt

diff --git a/tools/hrw4u/src/ast_nodes.py b/tools/hrw4u/src/ast_nodes.py
index 7f8cb8d5bb6..2b8741e8ea8 100644
--- a/tools/hrw4u/src/ast_nodes.py
+++ b/tools/hrw4u/src/ast_nodes.py
@@ -26,6 +26,7 @@
     "IPValue",
     "ParamRef",
     "BoolValue",
+    "NumberValue",
     "RegexValue",
     "SetValue",
     "IpRangeValue",
@@ -82,6 +83,11 @@ class BoolValue:
     raw: str  # source spelling, e.g. "TRUE", "true", "TRue"
 
 
+@dataclass(frozen=True, kw_only=True)
+class NumberValue:
+    raw: str  # source spelling: header_rewrite echoes it, so 007 is three bytes and not 7
+
+
 @dataclass(frozen=True, kw_only=True)
 class RegexValue:
     raw: str
@@ -97,7 +103,7 @@ class IpRangeValue:
     raw: str  # verbatim source text
 
 
-ValueExpr = Union[LiteralStringValue, IdentValue, IPValue, ParamRef, BoolValue, int, IpRangeValue]
+ValueExpr = Union[LiteralStringValue, IdentValue, IPValue, ParamRef, BoolValue, NumberValue, IpRangeValue]
 
 
 @dataclass(frozen=True, kw_only=True)
diff --git a/tools/hrw4u/src/ast_visitor.py b/tools/hrw4u/src/ast_visitor.py
index f5fe470e623..df376cedb78 100644
--- a/tools/hrw4u/src/ast_visitor.py
+++ b/tools/hrw4u/src/ast_visitor.py
@@ -128,8 +128,7 @@ def _visit_function_call(self, ctx) -> FunctionCall:
 
     def _extract_value(self, ctx) -> ValueExpr:
         if ctx.number is not None:
-            # Drops a leading zero the emitter would echo; no corpus input writes one.
-            return int(ctx.number.text)
+            return NumberValue(raw=ctx.number.text)
         if ctx.str_ is not None:
             return LiteralStringValue(raw=ctx.str_.text[1:-1])
         if ctx.TRUE():
diff --git a/tools/hrw4u/tests/ast_unparse.py b/tools/hrw4u/tests/ast_unparse.py
index ddc8937d405..11cd17fe6ae 100644
--- a/tools/hrw4u/tests/ast_unparse.py
+++ b/tools/hrw4u/tests/ast_unparse.py
@@ -131,11 +131,9 @@ def _comparison(node: Comparison) -> str:
 
 def _value(v: ValueExpr | RegexValue | SetValue) -> str:
     match v:
-        case int():
-            return str(v)
         case LiteralStringValue():
             return f'"{v.raw}"'
-        case BoolValue() | IdentValue() | IPValue() | IpRangeValue():
+        case NumberValue() | BoolValue() | IdentValue() | IPValue() | IpRangeValue():
             return v.raw
         case ParamRef():
             return f"${v.raw}"
diff --git a/tools/hrw4u/tests/data/ops/number-spelling.input.txt b/tools/hrw4u/tests/data/ops/number-spelling.input.txt
new file mode 100644
index 00000000000..05680bf58a4
--- /dev/null
+++ b/tools/hrw4u/tests/data/ops/number-spelling.input.txt
@@ -0,0 +1,7 @@
+# The emitter echoes a number's digits, so the AST cannot normalize one anywhere:
+# 007 and 7 are different bytes once a number reaches a header value.
+REMAP {
+    if random(0100) > 007 {
+        inbound.req.X-Count = 007;
+    }
+}
diff --git a/tools/hrw4u/tests/data/ops/number-spelling.output.txt b/tools/hrw4u/tests/data/ops/number-spelling.output.txt
new file mode 100644
index 00000000000..6988ba47e22
--- /dev/null
+++ b/tools/hrw4u/tests/data/ops/number-spelling.output.txt
@@ -0,0 +1,5 @@
+# The emitter echoes a number's digits, so the AST cannot normalize one anywhere:
+# 007 and 7 are different bytes once a number reaches a header value.
+cond %{REMAP_PSEUDO_HOOK} [AND]
+cond %{RANDOM:0100} >007
+    set-header X-Count 007
diff --git a/tools/hrw4u/tests/test_ast_visitor.py b/tools/hrw4u/tests/test_ast_visitor.py
index b419c5e9007..2df26b5b5da 100644
--- a/tools/hrw4u/tests/test_ast_visitor.py
+++ b/tools/hrw4u/tests/test_ast_visitor.py
@@ -60,7 +60,7 @@ def test_bool_keeps_source_spelling_outside_an_assignment(self):
     def test_int_value(self):
         ast = _build('REMAP {\n    http.cntl.INTERCEPT_RETRY = 1;\n}')
         a = ast.body[0].body[0]
-        assert a.value == 1
+        assert a.value == NumberValue(raw="1")
 
     def test_plus_equals(self):
         ast = _build('REMAP {\n    inbound.req.X-Foo += "extra";\n}')
@@ -226,7 +226,7 @@ def test_default_param(self):
         pd = ast.body[0]
         assert isinstance(pd, ProcedureDecl)
         assert pd.params[0].name == "ttl"
-        assert pd.params[0].default == 300
+        assert pd.params[0].default == NumberValue(raw="300")
 
     def test_body(self):
         src = ('procedure local::multi() {\n    inbound.req.X = "a";\n'
@@ -338,13 +338,13 @@ def test_greater_than_comparison(self):
         cond = self._first_condition('REMAP {\n    if inbound.req.Content-Length > 1000 {\n        set-debug();\n    }\n}')
         assert isinstance(cond, Comparison)
         assert cond.operator == ">"
-        assert cond.right == 1000
+        assert cond.right == NumberValue(raw="1000")
 
     def test_less_than_comparison(self):
         cond = self._first_condition('REMAP {\n    if inbound.req.Content-Length < 500 {\n        set-debug();\n    }\n}')
         assert isinstance(cond, Comparison)
         assert cond.operator == "<"
-        assert cond.right == 500
+        assert cond.right == NumberValue(raw="500")
 
     def test_comparison_rhs_keeps_bool_spelling(self):
         """`== TRUE` emits `=TRUE`, so normalizing the RHS would change the config."""

From b0c418001ef13a48085239941566e0b2101c6e09 Mon Sep 17 00:00:00 2001
From: Masaori Koshiba <masaori@apache.org>
Date: Thu, 17 Sep 2026 16:50:23 +0900
Subject: [PATCH 4/4] hrw4u: make the per-test sandbox fixture compile

Its body wrote inbound.req.X-Foo inside TXN_START, where that field does
not exist, so the input was rejected before the sandbox ran and the
round-trip test had to name it as the one corpus input that does not
compile. The fixture only ever asserted that a per-test sandbox.yaml is
preferred over the shared one, and a section denial fires whatever the
body is, so the body is now a rule TXN_START actually admits.

That retires DOES_NOT_COMPILE: an input meant to be rejected is named
.fail., and a corpus input that stops compiling should fail the test
rather than opt out of it.
---
 tools/hrw4u/tests/data/sandbox/per-test-sandbox.input.txt | 4 +++-
 tools/hrw4u/tests/test_ast_roundtrip.py                   | 5 +----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/hrw4u/tests/data/sandbox/per-test-sandbox.input.txt b/tools/hrw4u/tests/data/sandbox/per-test-sandbox.input.txt
index bdfdf647a71..5ccf2f3bdce 100644
--- a/tools/hrw4u/tests/data/sandbox/per-test-sandbox.input.txt
+++ b/tools/hrw4u/tests/data/sandbox/per-test-sandbox.input.txt
@@ -1,3 +1,5 @@
 TXN_START {
-    inbound.req.X-Foo = "test";
+    if inbound.ip in {10.0.0.0/8} {
+        counter("txn.internal");
+    }
 }
diff --git a/tools/hrw4u/tests/test_ast_roundtrip.py b/tools/hrw4u/tests/test_ast_roundtrip.py
index c3dc3fdda5d..a0e8c411b34 100644
--- a/tools/hrw4u/tests/test_ast_roundtrip.py
+++ b/tools/hrw4u/tests/test_ast_roundtrip.py
@@ -53,9 +53,6 @@
 
 CORPUS = Path("tests/data")
 
-# Rejected before any sandbox check; named so a newly-broken input fails instead of dropping out.
-DOES_NOT_COMPILE = frozenset({"sandbox/per-test-sandbox"})
-
 
 def _case_id(input_file: Path) -> str:
     return f"{input_file.parent.name}/{input_file.name.removesuffix('.input.txt')}"
@@ -63,7 +60,7 @@ def _case_id(input_file: Path) -> str:
 
 def _cases() -> list[pytest.param]:
     files = (f for f in sorted(CORPUS.glob("*/*.input.txt")) if ".fail." not in f.name)
-    return [pytest.param(f, id=_case_id(f)) for f in files if _case_id(f) not in DOES_NOT_COMPILE]
+    return [pytest.param(f, id=_case_id(f)) for f in files]
 
 
 def _compile(text: str, input_file: Path) -> list[str]: