masaori335 commented on code in PR #13739:
URL: https://github.com/apache/trafficserver/pull/13739#discussion_r4118590497


##########
tools/hrw4u/src/symbols.py:
##########
@@ -180,7 +181,7 @@ def resolve_condition(self, name: str, section: SectionType 
| None = None) -> tu
             raise error
 
     def resolve_function(self, func_name: str, args: list[str], strip_quotes: 
bool = False) -> str:
-        with self.debug_context("resolve_function", func_name, args):
+        with self.debug_context("resolve_function", func_name, args), 
self._warnings_on_success():

Review Comment:
   Enforcing function policy on value functions is #13674's change; its diff 
adds this `check_function` call at the same line. This PR only puts 
`resolve_function` inside `_warnings_on_success()`, so the warning #13674 
introduces is rolled back on failure once both land.



##########
tools/hrw4u/tests/test_coverage.py:
##########
@@ -101,6 +102,46 @@ def test_is_matched_prefix(self):
         assert not _is_matched("response_header.Host", 
frozenset(["request_header."]))
 
 
+class TestSandboxWarningOnFailure:
+    """A policy warning must not outlive a resolution that fails."""
+
+    @staticmethod
+    def _resolver(**warn: frozenset[str]) -> SymbolResolver:
+        return SymbolResolver(sandbox=SandboxConfig(message="", 
deny=PolicySets(), warn=PolicySets(**warn)))
+
+    def test_statement_function(self):
+        resolver = self._resolver(functions=frozenset(["set-debug"]))
+        with pytest.raises(SymbolResolutionError):
+            resolver.resolve_statement_func("set-debug", ['"extra"'], 
SectionType.REMAP)
+        assert resolver.drain_warnings() == []
+        resolver.resolve_statement_func("set-debug", [], SectionType.REMAP)
+        assert resolver.drain_warnings()
+
+    def test_assignment(self):
+        resolver = self._resolver(operators=frozenset(["inbound.resp."]))
+        with pytest.raises(SymbolResolutionError):
+            resolver.resolve_assignment("inbound.resp.X-A", '"1"', 
SectionType.REMAP)
+        assert resolver.drain_warnings() == []
+        resolver.resolve_assignment("inbound.resp.X-A", '"1"', 
SectionType.SEND_RESPONSE)
+        assert resolver.drain_warnings()
+
+    def test_add_assignment(self):
+        resolver = self._resolver(operators=frozenset(["inbound.resp."]))
+        with pytest.raises(SymbolResolutionError):
+            resolver.resolve_add_assignment("inbound.resp.X-A", '"1"', 
SectionType.REMAP)
+        assert resolver.drain_warnings() == []
+        resolver.resolve_add_assignment("inbound.resp.X-A", '"1"', 
SectionType.SEND_RESPONSE)
+        assert resolver.drain_warnings()
+
+    def test_condition(self):
+        resolver = self._resolver(conditions=frozenset(["inbound.resp."]))
+        with pytest.raises(SymbolResolutionError):
+            resolver.resolve_condition("inbound.resp.X-A", SectionType.REMAP)
+        assert resolver.drain_warnings() == []
+        resolver.resolve_condition("inbound.resp.X-A", 
SectionType.SEND_RESPONSE)
+        assert resolver.drain_warnings()

Review Comment:
   On this PR alone `resolve_function` collects no warning, so a rollback test 
here would pass against the unfixed code. The failing-then-successful 
value-function case will be added by whichever of this PR and #13674 merges 
second, alongside the `resolve_function` conflict resolution.



-- 
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]

Reply via email to