Skip to content

Tracking: minor follow-ups named in the #93 review (post-merge) #97

Description

@chiangchenghsin-hash

PR #93 merged on 2026-10-06 — thank you again for the review and the merge. The blocker and majors 1–6 were fixed and gated before merge (fix summary). Opening this small tracker so the minor findings named inline in the review comment don't get lost post-merge.

Line numbers below refer to the reviewed revision (ff1e47a); they have shifted since (majors 3–6 touched the same files).

  • ;-split false rejection (gql_transformer.cpp:1320 region) — input containing ; can be wrongly rejected at the statement-splitting boundary.
  • ACYCLIC (n) edge case (gql_transformer.cpp:2133 region) — edge case in the ACYCLIC (n) path-mode handling.
  • fixed-{n} up to 1000 bypassing width caps (gql_transformer.cpp:906,929 region) — quantified path pattern expansion.
  • Recursive JSON descent depth — nesting depth in the JSON value bridges is not bounded.
  • jsonEscape control characters — control characters in string values when producing JSON.
  • addFunc silent shadowing — registering a function over an existing name does not warn.
  • Note-tier weakness — the 120 "passed-with-note" TCK results are weak evidence for "green"; worth considering a stricter classification in the report.

Notes:

  • The full minor list ("available on request") was never posted before the merge; this tracks the seven items enumerated in the review comment itself. Happy to fold in more if the rest surfaces.
  • Triage rule on our side: anything that can surface a wrong result silently (rather than throwing or being documented as a known gap) jumps the queue — our first reading is that addFunc shadowing and jsonEscape control chars are the two candidates for that class, but each item gets verified before any fix.
  • Happy to send these as small follow-up PRs here — or to take direction on which to prioritize.

Activity

  1. adsharma commented on Oct 8, 2026

    @adsharma
    Contributor

    Triage — 7 minors from #93 (verified at main@2b9d5f4)

    # Item Valid? Silent-wrong? Severity Fix
    6 addFunc silent shadowing ✅ Yes — genuine mechanism low* Loud load-time check in gql_extension.cpp
    5 jsonEscape control chars ✅ No (invalid output, not wrong data) low Full JSON escaping or yyjson-build the payload
    4 Recursive JSON descent depth ✅ No (crash = loud, but worst loud) medium Depth cap + throw
    7 Note-tier weakness (120 notes) ✅ N/A (reporting honesty) medium Add --strict CI gate
    1 ;-split false rejection ✅ No (loud, misleading msg) minor Structured prefix/body, don't re-scan text
    2 ACYCLIC (n) ✅ No (loud or vacuously-true) trivial 1-line edgeCount > 0 guard
    3 Fixed-{n} ≤1000 bypass ✅ No (exact, but DoS-shaped) low Total-hop bound + doc fix

    *Low only because all names are _GQL_-prefixed today — the mechanism is real.

    Suggested order

    1. 4 (crash) + 6 (only silent-wrong) first.
    2. 5 with its serialization sibling.
    3. 7 (--strict, test-only, no engine risk).
    4. 1, 2, 3 as one small transformer cleanup.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions