Repository navigation
fix(pipeline): emit and reconstruct route properties as valid JSON - #2738
Conversation
An unchanged handler's Route is rebuilt from the route_path and route_method stored on its Function. They were read with a quote scan: an escaped quote or backslash cut the path short, and a nested, broken or non-string value still named a Route. They are now decoded with yyjson, and anything that is not a top-level string that fits names no Route. Every Route and HANDLES properties writer in pass_route_nodes.c now escapes its values into a buffer that fits them: the rebuilt handler QN was unescaped, a QN filling the 512-byte escape buffer left the closing "} off the 512-byte properties, a method is source text where a DRF @action names it, and the SvelteKit +server writer had both problems in 256 bytes. Tests: each of the five new infrascan tests fails on 4cf4414 without this change. Signed-off-by: Petteri Ponsimaa <[email protected]>
add0565 to
925703d
Compare
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
Signed-off-by: Martin Vogel <[email protected]>
|
Thanks for the route-properties JSON fix and the regression coverage! I merged current main into this branch in 671bc49 so the PR picks up our updated CI configuration. The merge was conflict-free, and your two-file patch is unchanged. This preserves your commit and sign-off; the new head will now run the current required checks. |
Review follow-up to the previous commit. Properties hold source text as its bytes, so a Latin-1 docstring stored beside route_path is not UTF-8. The strict yyjson parse rejected that whole object, so the route pass skipped rebuilding the handler's Route, where the old quote scan had rebuilt it. Parse with YYJSON_READ_ALLOW_INVALID_UNICODE, as internal/cbm/diagram.c does. Test: infrascan_decorator_route_metadata_survives_invalid_utf8 fails with the strict parse, and passes with this change and on 4cf4414. Signed-off-by: Petteri Ponsimaa <[email protected]>
|
Written with AI assistance (Claude Code); I reviewed it and stand behind it. Thanks for syncing the branch, @DeusData. I added one commit on top of your merge, d9766ae, rather than force-pushing over it. Why: a review pass found a regression in my first commit. Properties store source text as raw bytes, so a Latin-1 docstring beside On the previous run, |
Signed-off-by: Petteri Ponsimaa <[email protected]>
Signed-off-by: Petteri Ponsimaa <[email protected]>
|
Thank you, @petterip - merged! This is a careful, well-evidenced fix. The stored route metadata is now decoded as real JSON instead of being read up to the next quote. That means a route path with an escaped quote or backslash keeps its Route across an incremental re-index, and malformed values no longer invent Routes. Every Route and HANDLES properties writer in the pass now produces valid JSON as well, including long handler names and SvelteKit +server handlers. Catching the Latin-1 regression yourself in the follow-up commit, and keeping the old behaviour for properties that are not UTF-8, was exactly right. The six regression tests will keep each of these defects fixed. The earlier lint failures on this branch were our CI hitting its time limit while cppcheck ran, not anything in your change, and that is fixed on main now. Thanks also for keeping the Next.js App Router work separate in #2747; with this merged, that PR's diff should shrink to just the new part, and we look forward to reviewing it. |
What does this PR do?
Makes
src/pipeline/pass_route_nodes.cwrite and read Route and HANDLES properties as valid JSON. It is the groundwork for #759 (Next.js App Router routes), which follows as a separate PR stacked on this one: a route path from a directory with a quote in its name has to come out of the route pass intact.On every index the route pass also rebuilds a Route from the
route_path/route_methodstored on each Function, so an unchanged handler keeps its Route across an incremental re-index. Those were read with a quote scan, and the writers didn't escape everything they wrote:4cf44147without the fix)route_path/route_methodup to the next": an escaped quote or backslash cut the path short, and a value nested in another object, cut off, or holding an escaped NUL still named a Route (3 invented Routes in the test)infrascan_decorator_route_metadata_keeps_its_escapes,infrascan_decorator_route_metadata_rejects_malformed_valueshandlerunescapedcbm_json_escapechar[512]around a 512-byte escape buffer: a long QN loses the closing"}(emitter and rebuild)CBM_SZ_1K, as inpass_parallel.candpass_calls.cinfrascan_handles_properties_hold_a_long_handler_qnmethodunescaped; a DRF@action(methods=['a"b'])method is source text (emitter and rebuild)cbm_json_escapeinfrascan_route_properties_escape_the_method+serverHANDLES: QN unescaped,char[256]sveltekit_handles_propshelper, escaped,CBM_SZ_1Kinfrascan_sveltekit_handles_properties_escape_the_handler_qnThe parse uses
YYJSON_READ_ALLOW_INVALID_UNICODE, asinternal/cbm/diagram.cdoes: properties hold source text as its bytes (a Latin-1 docstring is stored as-is besideroute_path), and a strict parse rejects the whole object, so the pass would skip that handler where the quote scan rebuilt its Route.infrascan_decorator_route_metadata_survives_invalid_utf8guards that; it passes on4cf44147and fails with a strict parse.All five are the same defect (properties that aren't valid JSON) in one file, so I kept them together. If you'd rather have the emitter and SvelteKit fixes as a separate PR, I'll split them.
I checked the other
"handler":"%s"and"method":"%s"writers undersrc/: they already escape into 1K buffers, take values from fixed tables (cbm_service_pattern_*, the shellMETHODS[], the SvelteKit verbs), or write constants.The SvelteKit write moved into a helper so the diff-scoped clang-tidy check doesn't flag the existing complexity of
sveltekit_file_visitor; that function gets simpler, not more complex.Evidence
Tested on
4cf44147. The branch now carries current main through the merge in671bc495; the second commit,d9766ae8, is the UTF-8 fix described above. Machine: x86_64, 16 cores, WSL2 (Linux 6.18) on Ubuntu 22.04, gcc 11.4; clang-format 20.1.8, cppcheck 2.20.0 and clang-tidy 21.1.8, the versions CI uses.4cf44147without the fix: 5 failed, the 12 existing infrascan tests passed. The UTF-8 guard passes there.infrascansuite with the fix: 18 passed, 0 failed; each new test passes in under 0.2 s on its own.scripts/test.sh: rc 0, 9904 passed, 0 failed, 9 skipped (SKIP_PLATFORM), 191 suites. Run with step 0w (published language-count contract) removed, because that step fails on4cf44147itself; fix(ci): repair language count and Windows CodeQL retrace contracts #2732 fixes it.scripts/lint.sh --ci CLANG_FORMAT=clang-format-20: rc 0, "All linters passed".make -f Makefile.cbm lint-tidy-diff CC=clang-21 CLANG_TIDY=clang-tidy-21on the staged change: rc 0, no findings on changed lines.make -f Makefile.cbm lint-mem-ci CC=clang-21 CLANG_TIDY=clang-tidy-21: rc 0, "memory gate clean".AI assistance
Implementation and tests were written with AI assistance (Claude Code; planning with Codex). I reviewed the change, ran the commands listed above, and take responsibility for it under my DCO sign-off.
Checklist
git commit -s)make -f Makefile.cbm test)make -f Makefile.cbm lint-ci)