hrw4u: Fix u4wrh emitting HTTP-section operators into TXN_CLOSE - #13204
hrw4u: Fix u4wrh emitting HTTP-section operators into TXN_CLOSE#13204Clendenin wants to merge 1 commit into
Conversation
u4wrh bypassed section access validation when emitting statements, so operators that declare sections=HTTP_SECTIONS could land inside a generated TXN_CLOSE block. The output then failed re-parse in hrw4u. Validate the operator's declared sections at emit time and surface a clear error when the originating HRW places it in an incompatible hook. Adds a dedicated OPERATOR_MAP reverse-fail fixture for the operator-level path and documents the *.reverse.fail.* fixture naming convention.
There was a problem hiding this comment.
Pull request overview
Fixes u4wrh (HRW → hrw4u reverse compiler) emitting HTTP-section operators inside TXN_CLOSE blocks, which produced output that failed re-parsing. Section access is now validated at emit time using the same mechanism used by the forward compiler, and the emit site is wrapped in trap so the error is surfaced cleanly instead of crashing.
Changes:
- In
op_to_hrw4u, callvalidate_section_accessfor bothOPERATOR_MAPandSTATEMENT_FUNCTION_MAPmatches before emitting. - In
visitOpLine, wrapop_to_hrw4uinself.trap(ctx)and skip emission when no statement was produced. - Add reverse-failure fixture infrastructure (
collect_reverse_failing_inputs,run_reverse_failing_test) plus ops/hooks fixtures asserting the expectedTXN_CLOSEerror.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/hrw4u/src/hrw_symbols.py | Validate section access against operator/statement-function sections before emitting; minor else cleanup |
| tools/hrw4u/src/hrw_visitor.py | Wrap op_to_hrw4u call in trap, only emit when a statement is returned |
| tools/hrw4u/tests/utils.py | Add collect_reverse_failing_inputs and run_reverse_failing_test helpers |
| tools/hrw4u/tests/test_ops_reverse.py | Parametrized invalid-reverse test for ops fixtures |
| tools/hrw4u/tests/test_hooks_reverse.py | Parametrized invalid-reverse test for hooks fixtures |
| tools/hrw4u/tests/data/ops/txn_close_set_destination.reverse.fail.{hrw,error}.txt | Fixture: set-destination under TXN_CLOSE_HOOK should error |
| tools/hrw4u/tests/data/hooks/txn_close_http_op.reverse.fail.{hrw,error}.txt | Fixture: add-header under TXN_CLOSE_HOOK should error |
|
This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community. |
|
Not for this PR, but while reviewing, I filed two more issues. @Clendenin do you want to take a look at these two as well ? |
zwoop
left a comment
zwoop
left a comment
There was a problem hiding this comment.
If you don't mind, can you address the "rm-destination" case as well ?
| if cmd == "no-op" and op_state.last: | ||
| return "break" | ||
|
|
||
| if cmd == "rm-destination" and args and args[0].upper() == "QUERY": |
There was a problem hiding this comment.
I think there's a use case here as well which slips through:
cond %{TXN_CLOSE_HOOK}
rm-destination QUERY
This produces
TXN_CLOSE {
inbound.url.query = "";
}
Which is not legal:
leif@hel ~/apache/reviews/tools/hrw4u ±master⚡ » uv run ./scripts/u4wrh /tmp/bar.hrw| uv run ./scripts/hrw4u
cond %{TXN_CLOSE_HOOK} [AND]
<stdin>:2:4: error: inbound.url.query is not available in the TXN_CLOSE section
2 | inbound.url.query = "";
| ^
Summary
u4wrh bypassed section access validation when emitting statements, so operators
that declare
sections=HTTP_SECTIONScould land inside a generatedTXN_CLOSEblock. The output then failed re-parse in hrw4u.
This change validates the operator's declared sections at emit time and surfaces
a clear error when the originating HRW places it in an incompatible hook.