Skip to content

fix: move_uploaded_file() and is_uploaded_file() accept a worker's uploads - #65

Merged
kwhorne merged 1 commit into
mainfrom
worker-move-uploaded-file
Oct 25, 2026
Merged

kwhorne merged 1 commit into
mainfrom
worker-move-uploaded-file

Conversation

@kwhorne

@kwhorne kwhorne commented Oct 25, 2026

Copy link
Copy Markdown
Owner

The problem

is_uploaded_file() and move_uploaded_file() consult SG(rfc1867_uploaded_files), a table that only PHP's own multipart parser fills. A worker's files never go through that parser, so both functions refused every upload in worker mode:

  • a worker script written for PHP-FPM could not move an upload;
  • laravel-worker.php built its files in test mode to get round it.

The fix

  • The shim fills the table with the request's intact files (error == 0) right before the handler runs. It does this the same way main/rfc1867.c does: the path is both key and value, and the table releases it.
  • The table is destroyed when the handler returns, so a file from an earlier request, or any other file, is still refused. That table is what keeps move_uploaded_file() from moving arbitrary files, so it must hold exactly this request's files.
  • Deleting the files stays with Askr's existing guard.
  • laravel-worker.php builds ordinary UploadedFile objects: isValid() checks they are uploads, and ->move() is move_uploaded_file(), as under PHP-FPM.

Verification

  • e2e a_worker_can_move_its_own_uploads_and_nothing_else:

    • the request's files are uploads and can be moved;
    • a file that has been moved is no longer an upload;
    • the previous request's file is refused;
    • an arbitrary file (__FILE__) is neither "uploaded" nor moved.

    It fails without the fill (moved=0) and without the clear (previous uploaded=1).

  • php-src: empty_upload, max_file_size, max_file_uploads_empty_files and post_max_filesize now pass in worker mode and have left the catalogue. 8 divergences remain.

  • Laravel smoke: an upload is valid and ->move()s. It fails with the new worker on the old shim. Passes on Laravel 12 and a fresh 13.

Note for the release: the new laravel-worker.php needs Askr ≥ 1.9.2. Without the table, isValid() is false. The image and tarball ship worker and binary together, but someone who copied the worker into their app must upgrade Askr first. That goes in UPGRADING.

…loads

PHP consults SG(rfc1867_uploaded_files), which only its own multipart
parser fills. A worker's files never go through that parser, so both
functions refused every one of them: a worker script written for PHP-FPM
could not move an upload, and the Laravel worker built its files in test
mode to get round it.

The shim fills the table with the request's intact files before the
handler runs, as main/rfc1867.c does (path as key and value, released by
the table), and destroys it when the handler returns, so a file from an
earlier request, or any other file, is still refused. Deleting the files
stays with Askr's guard.

examples/laravel-worker.php builds ordinary UploadedFiles: isValid()
checks they are uploads, ->move() is move_uploaded_file().

- e2e a_worker_can_move_its_own_uploads_and_nothing_else: the request's
  files are uploads and move; a moved one, the previous request's file and
  an arbitrary file are not. Fails without the fill (moved=0) and without
  the clear (previous uploaded=1).
- php-src: empty_upload, max_file_size, max_file_uploads_empty_files and
  post_max_filesize now pass in worker mode and leave the catalogue (8
  divergences remain).
- laravel-smoke: an upload is valid and ->move()s; fails with the new
  worker on the old shim. Laravel 12 and a fresh 13.
@kwhorne
kwhorne merged commit 70ec421 into main Oct 25, 2026
6 checks passed
@kwhorne
kwhorne deleted the worker-move-uploaded-file branch October 25, 2026 11:29
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.

1 participant