Skip to content

feat: reject uploads with a synchronous validator#24925

Open
mcollovati wants to merge 3 commits into
mainfrom
issues/24923-upload_validator
Open

feat: reject uploads with a synchronous validator#24925
mcollovati wants to merge 3 commits into
mainfrom
issues/24923-upload_validator

Conversation

@mcollovati

Copy link
Copy Markdown
Collaborator

The pre-made upload handlers offered no supported way to refuse an upload while it was being received; only a fully custom UploadHandler could call UploadEvent.reject(...). Subclassing a pre-made handler to reject a captured event did not help: rejection was signalled asynchronously and never checked, so the success callback still ran and file handlers left the written file on disk.

Add UploadValidator, registered via withValidator(...), which is invoked synchronously while the upload is received and can call UploadEvent.reject(...) to abort it before it is stored. Downloads are unaffected.

Fixes #24923

@github-actions github-actions Bot added the +0.1.0 label Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Test Results

 1 456 files  ± 0   1 456 suites  ±0   1h 45m 31s ⏱️ + 5m 0s
10 326 tests +20  10 258 ✅ +20  68 💤 ±0  0 ❌ ±0 
10 798 runs  +20  10 729 ✅ +20  69 💤 ±0  0 ❌ ±0 

Results for commit c236bfb. ± Comparison against base commit 27154cd.

♻️ This comment has been updated with latest results.

* Adds a validator that can reject the upload while it is being received.
* <p>
* The validator is invoked synchronously on the request thread: once before
* any data is read (for metadata-only checks) and then once per chunk of

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once before any data is read (for metadata-only checks) and then once per chunk

That means - it's possible that the validator run literally 1000 times? 😬 I doubt that there are validator that require to run so often. Would it make sense to have a way to define when a validator runs? e.g. once, meta + first chunk or at every chunk, last chunk? I'm looking at things like:

  • once: typical file extension check
  • meta + first chunk: mime type check on the magic bytes
  • every chunk: possible disc threshold exceeded?
  • last chunk: anti virus check

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Allowing to configure when validation should happen makes sense to avoid calling the validator many times without a good reason

@mcollovati mcollovati force-pushed the issues/24923-upload_validator branch from 2a59bbe to 23c8741 Compare July 8, 2026 08:31
The pre-made upload handlers offered no supported way to refuse an upload while
it was being received; only a fully custom `UploadHandler` could call
`UploadEvent.reject(...)`, and the handlers invoked the success callback
unconditionally and left rejected files on disk.

Add `UploadValidator`, invoked synchronously by the pre-made handlers in three
phases: `validateMetadata` before any data is read, `validateHeader` over the
first N bytes (e.g. magic-byte checks, aborting after only those bytes), and
`validateComplete` over the fully received content (e.g. antivirus). Calling
`UploadEvent.reject(...)` from any phase aborts the upload before it is stored,
skips the success callback, deletes any written file, and reports the rejection
to the client. Register validators additively via `withValidator(...)` or the
fluent `validateMetadata`/`validateHeader(int, ...)`/`validateComplete` methods.

Fixes #24923
@mcollovati mcollovati force-pushed the issues/24923-upload_validator branch from 23c8741 to 79cd68d Compare July 8, 2026 09:11
Constructing a real UploadEvent lets the rejection state machine
(reject/isRejected/getRejectionMessage) run for real, removing the
simulateReject() helper that stubbed those on the mock. Upload content
is now served through a mocked Part.
Extract the in-memory read phase into a helper so metadata and header
rejection become flat early returns instead of nested guards, and drop
the redundant post-transfer isRejected() check inside the UI.access
command in both handlers: rejection state is fully settled once all
validators have run, so a rejected upload reports its terminal onError,
cleans up, and returns without scheduling a no-op delivery.
@sonarqubecloud

sonarqubecloud Bot commented Jul 9, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UploadHandler: built-in handlers ignore event.isRejected() when subclassed to reject uploads

2 participants