Skip to content

fix: parse recipient before creating file in upload_init#131

Merged
rubenhensen merged 2 commits intomainfrom
fix/upload-init-orphan-files-125
May 7, 2026
Merged

fix: parse recipient before creating file in upload_init#131
rubenhensen merged 2 commits intomainfrom
fix/upload-init-orphan-files-125

Conversation

@dobby-coder
Copy link
Copy Markdown
Contributor

@dobby-coder dobby-coder Bot commented May 1, 2026

Closes #125.

Problem

upload_init in src/main.rs created a zero-byte file on disk before validating the request's recipient. When the recipient was unparseable, the handler returned 400 but the empty UUID-named file in data_dir was orphaned — the in-memory purge_task only walks Store::expirations, and no FileState was ever inserted for that UUID. Repeated bad-recipient requests slowly fill data_dir.

Fix

Reorder so the recipient is parsed first. Only on success do we generate the UUID, create the on-disk file, and call store.create. Mirrors the cleanup-on-error approach already used elsewhere in the upload flow.

Tests

Two new regression tests (tests::upload_init_bad_recipient_does_not_create_file and tests::upload_init_good_recipient_creates_file) against a minimal Rocket harness in src/main.rs:

  • Bad recipient → asserts 400 Bad Request and that data_dir is empty afterwards.
  • Good recipient → asserts 200 OK and exactly one file appears in data_dir.

Both tests use a per-test temp data_dir so they run in parallel safely.

Verification

  • cargo test — 32 passed, 0 failed (was 30, +2 new)
  • cargo fmt --all -- --check — clean
  • cargo build --release — clean

Reviewer quickstart

git fetch origin && git checkout fix/upload-init-orphan-files-125 && cargo test

Closes #125

Reorders `upload_init` so the request's recipient is parsed first; only
on success do we generate a UUID and create the on-disk file. Previously
a malformed recipient returned 400 but left an empty UUID-named file
behind in `data_dir` that the in-memory `purge_task` could never reap
(no `FileState` was inserted on the rejection path).

Adds two regression tests against a minimal Rocket harness:
- bad recipient: 400 returned and `data_dir` stays empty.
- good recipient: 200 returned and exactly one file appears.
@rubenhensen
Copy link
Copy Markdown
Contributor

/dobby fix the conflicts

@dobby-coder
Copy link
Copy Markdown
Contributor Author

dobby-coder Bot commented May 7, 2026

Merged main into this branch to resolve conflicts.

Conflicts (both in src/main.rs):

  1. upload_init — main had since added notify_recipients, api_key_tenant, and api_key_validation_failed to FileState/InitBody (PRs feat(upload): add notifyRecipients toggle on /fileupload/init #135, fix: validate API key against pg-pkg, not a local allowlist #139), while this branch reordered the recipient parse to fix the orphan-file bug. Resolution: kept the parse-first reorder (this PR's fix) and propagated the new fields into the FileState constructor. Dropped main's wrapping match request.recipient.parse() since the same parse now happens earlier with ?.

  2. Unit tests block — both branches added tests in the same mod tests slot. Kept all of them: the two upload_init_* regression tests from this PR plus the six extract_pg_bearer_* tests from main.

Verification on the merged branch:

  • cargo test — 39 passed, 0 failed (was 32 here, 37 on main; +2 from this PR's regression tests, no overlap)
  • cargo fmt --all -- --check — clean
  • cargo clippy --all-targets -- -D warnings — clean
  • cargo build --release — clean

Approval is preserved (push was a merge commit, not a rebase). Should be mergeable now.

@rubenhensen rubenhensen merged commit 33905c3 into main May 7, 2026
6 checks passed
@rubenhensen rubenhensen deleted the fix/upload-init-orphan-files-125 branch May 7, 2026 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

upload_init creates an empty file before validating recipient, leaking orphans on bad input

1 participant