Skip to content

A single sections request stalls every other request to the server #481

Description

@thcp

What goes wrong

PATCH /api/jobs/{job_id}/sections accepts a section list of any length. A single
request with a large body blocks the event loop, so every other request to the
server waits for it, including the SSE progress streams a running job depends on.

Measured against a local server:

health latency, idle server : 31 ms
health latency, during parse: 3812 ms

That was a 33 MB body carrying 400,000 sections. The stall grows linearly with
body size, so it is bounded only by how much the sender is willing to upload.

Two things make it worse than a slow endpoint:

  • A valid job is not required. FastAPI validates the request body before the
    handler runs, so the whole payload is parsed into Pydantic models and only then
    does the handler answer 404 for a job that never existed. No job ID, no library,
    no prior state is needed.
  • With a real job it is permanent, not transient. The list is written to
    metadata.json and held in the in-memory registry, which is re-serialised into
    registry.json on every persist. One request permanently inflates every future
    save.

Why this is inconsistent rather than merely absent

The sibling endpoint in the same file already guards exactly this, and says so:

# Upper bound on an edited grid. A 20-minute track at 300 BPM is ~6000 beats;
# 20000 leaves generous headroom while refusing a payload crafted to exhaust
# memory or disk.
_MAX_EDITED_BEATS = 20000

class BeatsBody(BaseModel):
    beats: list[float] = Field(max_length=_MAX_EDITED_BEATS)

SectionsBody.sections has no equivalent. The individual fields are validated
carefully (id pattern, colour pattern, name truncated to 64 characters, times
range-checked) which makes the missing list bound easy to overlook: every element
looks guarded, and nothing bounds how many elements there are.

Constraints for anyone fixing this

  • StemDeck has no per-IP rate limiting and that is deliberate; MAX_PENDING_JOBS
    is the only request-rate throttle. So the bound has to be on the payload itself.
  • The cap must not be so tight that a real song is rejected. Sections are
    human-scale: a long track annotated densely is still tens of entries, not
    thousands.
  • Whatever number is chosen should carry the same kind of justification comment
    the beat-grid cap does, so the next reader knows what it is protecting against.

Activity

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