Problem/Motivation

The fragment, branch, context, delivery, filter, and refresh endpoints do real work but carry no flood control, and all bypass page/render cache. The filter path renders matches at hardcoded full density up to a result limit that can be raised to 500. Several unbounded inputs amplify cost: an unconstrained instance_id, cache-context flooding via the act_mode_* and comment query parameters, and a filter() query that builds up to 4xN IN placeholders plus a Unicode branch that makes the CONTAINS conditions redundant.

Steps to reproduce

  • Send repeated anonymous requests to a filter or fragment endpoint; note no throttling.
  • Raise filter_result_limit to 500 and filter for a common term; observe ~500 full comment entity renders per request.
  • Pass a long instance_id and observe response amplification; vary act_mode_* and comment and observe unbounded cache entries.

Proposed resolution

  • Add flood control keyed on client + route (reuse the existing flood service pattern).
  • Cap the filter render path and the number of comments load() materialises.
  • Require the access comments permission so checks fail fast.
  • Bound instance_id length; normalise act_mode_* and comment to stable values before adding cache contexts.
  • Drop the per-path IN lists and the redundant Unicode branch in filter().
  • Declare methods: GET on the branch and context routes, which are pure reads but currently declare no method restriction.
  • Raise the filter minimum query length to 2-3 characters with a visible hint; every single character currently fires a full search after the 250 ms debounce, so capping render cost does not reduce the trigger count.

Remaining tasks

  • Confirm throttling and bounded rendering on a 200+ comment node.

User interface changes

None, beyond faster and safer filtering.

API changes

Adds flood limits and input bounds; cache contexts use normalised values.

Data model changes

None anticipated.

AI-Generated: Yes (Used Opus 5 and DeepSeek Flash to review and discover these issues)

Comments

freelock created an issue. See original summary.

  • freelock committed 5496e552 on 1.0.x 
    test: #3623173 Cover flood control, route permissions, and filter length

  • freelock committed b075cf90 on 1.0.x 
    fix: #3623173 Harden and bound the on-demand fragment endpoints

  • freelock committed e1ccd242 on 1.0.x 
    test: #3623173 Update route contracts and approval fixtures for the...

  • freelock committed 2ea3809e on 1.0.x 
    fix: #3623173 Remove the unused comment query parameter and keep...
freelock’s picture

freelock
Location Seattle
commented
Status: Active » Needs work

Alpha3 follow-up: rejected input still creates distinct cache variants

Input validation does not yet normalize the cache-context values. The reader preference ignores invalid act_reading cookies, but cookies:act_reading still varies on their raw values. Arbitrary positive nonexistent comment query IDs also select no comment while creating distinct cache variants.

Steps to reproduce

  1. Request the same thread with act_reading=junk-a, then act_reading=junk-b.
  2. Compare effective display settings and cache-context values: the settings match, but the contexts differ.
  3. Repeat with different positive comment IDs that do not exist in the accessible discussion.

Remaining work and acceptance

  • Make equivalent effective preferences and absent targets reuse cache variants.
  • Keep legitimate density/depth and accessible target choices distinct.
  • Verify actual cache reuse, valid deep links, and multiple placements sharing one request.
  • Preserve the first-paint cookie behavior and Reset to site defaults from #3623182.
freelock’s picture

freelock
Location Seattle
commented

Alpha3 follow-up: minimum query length still permits full-index work

The permission checks and minimum search length are present, but the filter loads the full index before rejecting a too-short query. The configurable result limit also still permits 500 full-density renders, leaving the requested render-cost cap unfinished.

Steps to reproduce

  1. Request a one-character filter on a large discussion and instrument index loading; it runs before the empty response.
  2. Set the result limit to 500 and search for a common two-character term.
  3. Count the full comment renders permitted by that request.

Remaining work and acceptance

  • Reject too-short searches before expensive index construction, while retaining the required authorization checks.
  • Choose and document a bounded full-render limit, including visible feedback when results are limited and a way to refine the search.
  • Test that invalid queries do not hydrate the thread and that valid queries cannot exceed the chosen render bound.
  • Retain Unicode matching, field access, ancestor access, and result-order behavior.
freelock’s picture

freelock
Location Seattle
commented

Alpha3 follow-up: align the flood lock with the client bucket

The flood bucket is keyed by client IP and route, but its lock is route-wide. Because lock acquisition does not wait, contention can return 429 to an unrelated client whose own bucket is allowed.

Forced contention reproduced that mechanism. A subsequent review reported zero false rejections from 25 concurrent branch requests and 30 concurrent filter requests on fresh buckets. Practical availability impact has not been demonstrated; treat this as P3 hardening rather than an established service disruption.

Steps to reproduce

  1. Using an isolated test, hold the route lock through one database-lock instance.
  2. Invoke the guard through another instance for a different client with an unused flood bucket.
  3. Observe rejection despite that client's allowance.

Remaining work and acceptance

  • Scope synchronization to the same identity as the protected bucket.
  • Test that unrelated clients do not contend for the same lock.
  • Preserve race-safe accounting for the same bucket and legitimate rate-limit enforcement.
  • Exercise permitted parallel delivery batches and retain appropriate retry feedback.

  • freelock committed 28ca25b6 on 1.0.x 
    fix: #3623173 Harden and bound the on-demand fragment endpoints

  • freelock committed dee3b294 on 1.0.x 
    fix: #3623173 Harden and bound the on-demand fragment endpoints

  • freelock committed 3fe219e1 on 1.0.x 
    fix: #3623173 Harden and bound the on-demand fragment endpoints
freelock’s picture

freelock
Location Seattle
commented
Status: Needs work » Fixed

Fixed in Alpha4.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.