diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 1be74bc..e726eb7 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -4,13 +4,3 @@ ## Testing instructions - -## Screenshots - -## Checklist: -- [ ] I've tested the code -- [ ] I've written unit tests for new features (where appropriate) -- [ ] My code is easy to read, follow, and understand -- [ ] My code has proper inline documentation / docblocks. - -## Additional Comments diff --git a/.github/README.md b/.github/README.md deleted file mode 100644 index f7f4cc6..0000000 --- a/.github/README.md +++ /dev/null @@ -1,106 +0,0 @@ -# Upload - -[![codecov](https://codecov.io/gh/GravityPDF/Upload/branch/main/graph/badge.svg)](https://codecov.io/gh/GravityPDF/Upload) -[![License: MIT](https://img.shields.io/badge/License-MIT-blue.svg)](https://opensource.org/licenses/MIT) - -This component simplifies file validation and uploading. - -**Why was this library forked?** - -* Original library was abandoned (untouched since 2018) -* Adjusted namespace from \Upload to \GravityPdf\Upload -* Bumped minimum PHP version to 7.3+ -* Sanitized filename and extension, and add UTF-8 filename support -* Strict type checking -* Added `FileSystem::getDirectory()` and `FileInfo::setNameWithExtension()` methods -* Included unreleased code from upstream repo -* PSR-12 Code Formatting -* Automated tools: PHPUnit, PHPStan, PHPCS, and PHP Syntax Checker - -TODO: [PSR-7 and PSR-17 support (help wanted)](https://github.com/GravityPDF/Upload/issues/8) - -## Installation - -``` -composer require gravitypdf/upload -``` - -## Usage - -Assume a file is uploaded with this HTML form: - -```html -
- - -
-``` - -When the HTML form is submitted, the server-side PHP code can validate and upload the file like this: - -```php -$storage = new \GravityPdf\Upload\Storage\FileSystem('/path/to/directory'); -// To override existing files when uploading, pass `true` as the second parameter -// $storage = new \GravityPdf\Upload\Storage\FileSystem('/path/to/directory', true); -$file = new \GravityPdf\Upload\File('foo', $storage); - -// Validate file upload -// MimeType List => http://www.iana.org/assignments/media-types/media-types.xhtml -$file->addValidations([ - // Ensure file is of type "image/png" - new \GravityPdf\Upload\Validation\Mimetype('image/png'), - new \GravityPdf\Upload\Validation\Extension('png'), - - //You can also add multi mimetype validation or extensions - //new \GravityPdf\Upload\Validation\Mimetype(['image/png', 'image/gif']) - //new \GravityPdf\Upload\Validation\Extension(['png', 'gif']), - - // Ensure file is no larger than 5M (use "B", "K", M", or "G") - new \GravityPdf\Upload\Validation\Size('5M'), -]); - -// Access data about the file -// If upload accepts multiple files an array will be returned for each of these -$data = [ - 'name' => $file->getNameWithExtension(), - 'extension' => $file->getExtension(), - 'mime' => $file->getMimetype(), - 'size' => $file->getSize(), - 'md5' => $file->getMd5(), - 'dimensions' => $file->getDimensions(), -]; - -// If you have an upload field that accepts multiple files you can access each file's info individually -$firstFileName = $file[0]->getNameWithExtension(); -if(isset($file[1])) { - $secondFileName = $file[1]->getNameWithExtension(); -} - -// or loop over all files for this key -foreach($file as $i => $upload) { - $name = $upload->getNameWithExtension(); - $upload->setName('file-'.$i); -} - -// Try to upload file(s) -try { - // Success! - $file->upload(); -} catch (\Exception $e) { - // Validation errors - $errors = $file->getErrors(); - if(count($errors) === 0) { - // Failed for another reason, like the file already exists - $error = $e->getMessage(); - } -} -``` - -## Authors - -* [Josh Lockhart](https://github.com/codeguy) -* [Gravity PDF](https://github.com/GravityPDF) - -## License - -MIT Public License diff --git a/.github/workflows/php-syntax.yml b/.github/workflows/php-syntax.yml index 119dd4d..cc52561 100644 --- a/.github/workflows/php-syntax.yml +++ b/.github/workflows/php-syntax.yml @@ -6,6 +6,10 @@ on: - main pull_request: +# The workflows only read the repository; nothing needs a write-capable GITHUB_TOKEN. +permissions: + contents: read + # Cancels all previous workflow runs for pull requests that have not completed. concurrency: # The concurrency group contains the workflow name and the branch name for pull requests @@ -24,10 +28,10 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.0.0 - name: Install PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2.35.5 with: php-version: ${{ matrix.php }} @@ -42,14 +46,14 @@ jobs: # http://man7.org/linux/man-pages/man1/date.1.html - name: "Get last Monday's date" id: get-date - run: echo "::set-output name=date::$(/bin/date -u --date='last Mon' "+%F")" + run: echo "date=$(/bin/date -u --date='last Mon' "+%F")" >> "$GITHUB_OUTPUT" - name: Get Composer cache directory id: composer-cache - run: echo "::set-output name=dir::$(composer config cache-files-dir)" + run: echo "dir=$(composer config cache-files-dir)" >> "$GITHUB_OUTPUT" - name: Cache Composer dependencies - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5.0.4 env: cache-name: cache-composer-dependencies with: @@ -58,7 +62,7 @@ jobs: - name: Install Composer dependencies - run: composer install + run: composer install --no-scripts - name: Check PHP Syntax run: composer run check-syntax diff --git a/.github/workflows/phpcs.yml b/.github/workflows/phpcs.yml index 12b999b..2e2b7dd 100644 --- a/.github/workflows/phpcs.yml +++ b/.github/workflows/phpcs.yml @@ -6,6 +6,10 @@ on: - main pull_request: +# The workflows only read the repository; nothing needs a write-capable GITHUB_TOKEN. +permissions: + contents: read + # Cancels all previous workflow runs for pull requests that have not completed. concurrency: # The concurrency group contains the workflow name and the branch name for pull requests @@ -19,10 +23,10 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.0.0 - name: Install PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2.35.5 with: php-version: 7.3 @@ -37,14 +41,14 @@ jobs: # http://man7.org/linux/man-pages/man1/date.1.html - name: "Get last Monday's date" id: get-date - run: echo "::set-output name=date::$(/bin/date -u --date='last Mon' "+%F")" + run: echo "date=$(/bin/date -u --date='last Mon' "+%F")" >> "$GITHUB_OUTPUT" - name: Get Composer cache directory id: composer-cache - run: echo "::set-output name=dir::$(composer config cache-files-dir)" + run: echo "dir=$(composer config cache-files-dir)" >> "$GITHUB_OUTPUT" - name: Cache Composer dependencies - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5.0.4 env: cache-name: cache-composer-dependencies with: @@ -52,7 +56,7 @@ jobs: key: ${{ runner.os }}-php-7.3-date-${{ steps.get-date.outputs.date }}-composer-${{ hashFiles('**/composer.json') }} - name: Install Composer dependencies - run: composer install + run: composer install --no-scripts - name: Run PHPUnit tests run: composer run lint diff --git a/.github/workflows/phpstan.yml b/.github/workflows/phpstan.yml index 7fcbdfb..4754324 100644 --- a/.github/workflows/phpstan.yml +++ b/.github/workflows/phpstan.yml @@ -6,6 +6,10 @@ on: - main pull_request: +# The workflows only read the repository; nothing needs a write-capable GITHUB_TOKEN. +permissions: + contents: read + # Cancels all previous workflow runs for pull requests that have not completed. concurrency: # The concurrency group contains the workflow name and the branch name for pull requests @@ -19,12 +23,12 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.0.0 - name: Install PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2.35.5 with: - php-version: 7.3 + php-version: 8.5 - name: Log debug information run: | @@ -37,22 +41,22 @@ jobs: # http://man7.org/linux/man-pages/man1/date.1.html - name: "Get last Monday's date" id: get-date - run: echo "::set-output name=date::$(/bin/date -u --date='last Mon' "+%F")" + run: echo "date=$(/bin/date -u --date='last Mon' "+%F")" >> "$GITHUB_OUTPUT" - name: Get Composer cache directory id: composer-cache - run: echo "::set-output name=dir::$(composer config cache-files-dir)" + run: echo "dir=$(composer config cache-files-dir)" >> "$GITHUB_OUTPUT" - name: Cache Composer dependencies - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5.0.4 env: cache-name: cache-composer-dependencies with: path: ${{ steps.composer-cache.outputs.dir }} - key: ${{ runner.os }}-php-7.3-date-${{ steps.get-date.outputs.date }}-composer-${{ hashFiles('**/composer.json') }} + key: ${{ runner.os }}-php-8.5-date-${{ steps.get-date.outputs.date }}-composer-${{ hashFiles('**/composer.json') }} - name: Install Composer dependencies - run: composer install + run: composer install --no-scripts - - name: Run PHPUnit tests + - name: Run PHPStan run: composer run phpstan diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index ebc60a2..dabd97e 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -6,6 +6,10 @@ on: - main pull_request: +# The workflows only read the repository; nothing needs a write-capable GITHUB_TOKEN. +permissions: + contents: read + # Cancels all previous workflow runs for pull requests that have not completed. concurrency: # The concurrency group contains the workflow name and the branch name for pull requests @@ -28,10 +32,10 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.0.0 - name: Install PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2.35.5 with: php-version: ${{ matrix.php }} @@ -46,14 +50,14 @@ jobs: # http://man7.org/linux/man-pages/man1/date.1.html - name: "Get last Monday's date" id: get-date - run: echo "::set-output name=date::$(/bin/date -u --date='last Mon' "+%F")" + run: echo "date=$(/bin/date -u --date='last Mon' "+%F")" >> "$GITHUB_OUTPUT" - name: Get Composer cache directory id: composer-cache - run: echo "::set-output name=dir::$(composer config cache-files-dir)" + run: echo "dir=$(composer config cache-files-dir)" >> "$GITHUB_OUTPUT" - name: Cache Composer dependencies - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5.0.4 env: cache-name: cache-composer-dependencies with: @@ -61,13 +65,20 @@ jobs: key: ${{ runner.os }}-php-${{ matrix.php }}-date-${{ steps.get-date.outputs.date }}-composer-${{ hashFiles('**/composer.json') }} - name: Install Composer dependencies - run: composer install + run: composer install --no-scripts - name: Run PHPUnit tests run: vendor/bin/phpunit - name: Run (xDebug) tests if: ${{ matrix.report }} - run: | - vendor/bin/phpunit --verbose --coverage-clover=tmp/coverage/report-xml/php-coverage1.xml - bash <(curl -s https://codecov.io/bash); + run: vendor/bin/phpunit --verbose --coverage-clover=tmp/coverage/report-xml/php-coverage1.xml + + # Replaces `bash <(curl -s https://codecov.io/bash)` — an unpinned remote script piped + # into bash, which was compromised in April 2021 and has since been sunset by Codecov. + - name: Upload coverage to Codecov + if: ${{ matrix.report }} + uses: codecov/codecov-action@0fb7174895f61a3b6b78fc075e0cd60383518dac # v5.5.1 + with: + files: tmp/coverage/report-xml/php-coverage1.xml + token: ${{ secrets.CODECOV_TOKEN }} diff --git a/CHANGELOG.md b/CHANGELOG.md index cc341ba..9a2ad2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,101 @@ +# Upload 4.0.0 + +A security-hardening release, with new protections enabled by default. Step-by-step upgrade guide in [UPGRADE.md](https://github.com/GravityPDF/Upload/blob/main/UPGRADE.md). + +## Defaults That Changed + +* **A default extension deny-list is on.** `FileSystem` refuses to write files with the extensions in `FileSystem::getDefaultBlockedExtensions()` +* **The deny-list covers markup as well as executables.** The 15 in `FileSystem::MARKUP_EXTENSIONS` (`html`, `htm`, `xhtml`, `xht`, `xhtm`, `svg`, `svgz`, `xml`, `xsl`, `xslt`, `js`, `mjs`, `swf`, `mht`, `mhtml`) join the 59 in `EXECUTABLE_EXTENSIONS`, for a default list of 74. A server does not execute the markup ones, but serving them from your own origin is a stored XSS risk. **If you accept SVG or HTML uploads, pass `FileSystem::EXECUTABLE_EXTENSIONS` and sanitize the contents yourself** +* **The deny-list is checked against every dot-separated component of the name**, not only the extension `pathinfo()` returns, because a web server does not necessarily read a name the same way. `FileInfo` rewrites dots inside the name to hyphens as it always has, so `archive.config.zip` is stored as `archive-config.zip`; the wider check applies to a `FileInfoInterface` of your own. Trailing dots and spaces are removed before the check and before the write. The first component is not treated as an extension, so a file called `php` is still stored +* **Stored files get mode `0640`** (`FileSystem::DEFAULT_MODE`), where 3.x left the mode to the process umask +* **`File::upload()` refuses when no validations have been added** +* **`File::upload()` refuses when the collection is empty.** Validations pass vacuously with nothing to run against, so it previously returned `true` having stored nothing. A failed transfer is unchanged and still reports `'File validation failed'` with the detail in `getErrors()` +* **`FileSystem::upload()` rejects a destination beginning with a dot, and any name containing control characters.** It also rewrites the characters Windows refuses in a filename (`<`, `>`, `:`, `"`, `|`, `?`, `*`) to `-`, and rejects a name Windows resolves to a device rather than a file, such as `CON.txt`. `FileInfo` already rewrote or blanked all of these, so this only matters for a `FileInfoInterface` of your own + +## Security Fixes + +* **`FileInfo::setExtension()` no longer rewrites an invalid extension into a valid one.** Stripping the disallowed characters could produce an extension the client never sent. An extension that is not lowercase-alphanumeric once trimmed is now discarded, and the file is stored with **no extension** +* **`FileInfo::setExtension()` discards an extension longer than `Filename::MAX_EXTENSION_LENGTH` (32 bytes)**, which counts against the same 255 byte budget as the name +* **`FileInfo::setName()` rewrites the C1 control characters (`U+0080`–`U+009F`) and `\x7F` to `-`.** Its filter covered only the C0 range, and C1 survived because those bytes are valid UTF-8. One of them ends a line for anything reading `\R`, so a stored name could span what looked like several lines of a log. `Storage\FileSystem` refuses both for the same reason +* **`FileInfo::setName()` removes the characters that reorder, break or hide the text around them** — the whole of Unicode's `Bidi_Control` property including `U+061C`, plus the zero-width marks (`U+200B`–`U+200F`), the line and paragraph separators (`U+2028`, `U+2029`), `U+206A`–`U+206F` and the BOM (`U+FEFF`). Stored names are read by people, in admin listings, emails and log lines, and these let a name display as something other than what it is. The set is `Filename::BIDI_CONTROLS`, and **`Storage\FileSystem` refuses a name still carrying one** — it deletes nothing, since inventing a filename is the value object's job +* **The reserved Windows device names now include `COM0`, `LPT0` and the superscript variants** (`COM¹`, `COM²`, `COM³`, `LPT¹`, `LPT²`, `LPT³`), which Microsoft lists alongside `COM1`–`COM9`. A name matching one is blanked to `unnamed-file`, as the rest of the list always was. The list is now `Filename::RESERVED_WINDOWS_NAMES` +* **`FileSystem::upload()` reduces the destination to a basename** and rejects `''`, `.`, `..` and names containing a null byte +* **`FileSystem::upload()` refuses to write to a destination that is a symbolic link** +* **Uploads are written through a staged file rather than straight to the destination.** The file goes to a temporary name inside the destination directory and is then moved onto the destination in a single operation, which does not follow a symbolic link standing there, so no partial content is ever readable under the final name. With `overwrite = false` the name is claimed first by an empty placeholder at the configured mode, so a process killed mid-transfer leaves a 0-byte file rather than nothing +* **The overwrite guard is atomic with respect to two requests creating the same name.** With `overwrite = false`, the existence check and the create are a single exclusive operation rather than two steps, the file it creates is verified to be the destination itself, and a placeholder left by a failed move is cleaned up +* **A `chmod()` that fails is no longer ignored.** The mode is applied to the staged file and the upload is abandoned if it cannot be set, rather than storing the file at whatever the umask allowed while reporting the documented `0640` +* **`Validation\FileType::allow()` rejects empty and whitespace-only values on either side.** An empty media type is what `getMimetype()` reports for a file it cannot read, and it would match one. A lone `.` is rejected as an extension; a leading dot is accepted and removed +* **A name from a custom `FileInfoInterface` no longer reaches `getErrors()` unsanitized.** `File::isValid()` formatted it into its error strings as-is, so an implementation supplied through `FileInfo::setFactory()` could put a line break, a terminal escape or a bidi override into a string the README encourages callers to render. The shipped `FileInfo` was unaffected +* **Raw `$_FILES[…]['name']` no longer reaches `getErrors()`.** The `File` constructor's error path sanitizes it like every other error string. Escaping on output still applies +* **`getHash()`, `getMimetype()`, `getSize()` and `getDimensions()` handle a missing file**, returning `''`/`false`/`0` where `getSize()` raised a `RuntimeException`, `getHash()` a `TypeError`, and the other two emitted PHP warnings +* **`File::isValid()` absorbs non-`Upload\Exception` throwables** from validators instead of letting one abort the batch, recording `Validation could not be completed` with nothing appended — neither the message, which can contain server paths, nor the class name, which is the application's internal structure. `\LogicException` is re-thrown, because PHP defines the type as a bug in the program rather than a file that failed +* **The `File` constructor guards a malformed `$_FILES` entry in both of its branches.** A non-string `tmp_name` or `name` in the single-file shape, and a `name` or `error` that is not an array of the same length as `tmp_name` in the multi-file shape, are reported as unreadable rather than raising an uncaught `TypeError` or a warning. A `name` that was a string was the quiet one: indexing it yielded a single character, which passed the per-file check, so the file was stored under a one-letter name. No SAPI builds either shape, but a PSR-7 bridge, test harness or middleware can. A malformed entry costs only itself; well-formed files in the same request are still collected + +## Breaking Changes + +* **A developer error is no longer thrown as the type callers catch for a failed upload.** `File::upload()` throws `\LogicException` when no validations have been configured, and `FileInfo::getHash()` throws `\InvalidArgumentException` for an algorithm this PHP build does not support. 3.x had no no-validations check at all, and `getHash()` let `hash_file()` raise a `ValueError`. Both are typed now so a misconfigured object cannot be mistaken for a rejected file: `Upload\Exception` is what `File::isValid()` catches and formats into `getErrors()`. **Code catching `\GravityPdf\Upload\Exception` specifically around `upload()` needs `\LogicException` too**; a `catch (\Exception $e)` as shown in the README is unaffected +* **`File::__call()` throws `\BadMethodCallException`** for a method that is not on `FileInfoInterface`, so a typo in a method name is no longer reported to the end user as a failed upload. **`FileInfo::createFromFactory()` throws `\LogicException`** when an installed factory returns the wrong type; it threw a plain `\RuntimeException`, which no `Upload\Exception` handler caught either way, so that one is for consistency rather than a migration hazard +* **`FileInfoInterface`'s three setters no longer declare a return type.** They declared `: FileInfo`, the concrete class, so an implementation that did not extend `FileInfo` satisfied the compiler and then raised a `TypeError` on the first setter call: it could not return `$this`, and could not narrow its own return type either, since covariant returns arrived in PHP 7.4 and this library supports 7.3. **A custom `FileInfoInterface` is only now actually implementable.** `FileInfo` is unchanged, and an existing subclass declaring `: FileInfo` still satisfies the interface +* **`FileSystem::blockExtensions()` takes a required, non-empty list.** It previously defaulted to `null` for the full deny-list while `[]` meant none, so one expression turned a security control off depending on what a config key held. It now throws `InvalidArgumentException` on `[]`, and `allowAnyExtension()` is the only way to empty the list. Pass `FileSystem::getDefaultBlockedExtensions()` for the old no-argument meaning +* **`File::humanReadableToBytes()` now understands a trailing `B`, which changes an existing limit.** `'5MB'` previously parsed as 5 bytes, because `substr($input, -1)` saw only the `B`; it now parses as 5 MiB, matching what `Validation\Size`'s docblock always advertised. **If you pass a `MB`/`KB`/`GB` suffixed size anywhere, your effective limit becomes much larger.** Sizes without the trailing `B` are unaffected +* **`File::humanReadableToBytes()` throws on an unrecognized unit** instead of reading it as bytes. `'1T'` evaluated to `1`, so `new Size('1T')` was a one byte bound that rejected every upload while reading as a generous one. `B`, `K`, `M` and `G` are unchanged. **Check any size bound whose unit is not one of those four** +* **`FileInfo::getMd5()` has been removed**, from `FileInfo` and from `FileInfoInterface`. Call `getHash('md5')` for the same digest +* **`FileInfo::getHash()` defaults to `sha256` instead of `md5`**, and is now declared on `FileInfoInterface`. A no-argument call returns a different string than it did in 3.x +* **The extension deny-list and the reserved-name check moved out of `FileSystem::resolveFilename()` into `upload()`.** They ran inside the naming seam, so a subclass overriding `resolveFilename()` — the documented way to change how names are chosen — dropped both refusals without mentioning extensions. Both are now `private` and applied to whatever name the seam returns +* **The filename rules moved to the new `\GravityPdf\Upload\Filename`.** `MAX_EXTENSION_LENGTH`, `RESERVED_WINDOWS_NAMES`, `BIDI_CONTROLS` and `CONTROL_CHARACTERS` were public constants on `FileInfo`. `FileInfo` behaves exactly as before and delegates; only the constants moved, and they were introduced in this same release, so nothing released ever referenced them +* **`Filename::BIDI_CONTROLS` is a bare pattern fragment**, matching `CONTROL_CHARACTERS`. It was a complete delimited pattern while its neighbour was not, and the README listed the two side by side as if interchangeable +* **`FileSystem::createStagingPath()`, `discardFailedUpload()`, `releaseReservation()`, `refuseBlockedExtensions()`, `refuseReservedWindowsName()`, `File::formatUploadError()`, `getSanitizedFilename()`, `FileInfo::finalizeName()`, `acceptExtension()` and `FileType::normalize()` are `private`.** Each had one in-class caller and none was an extension seam; `formatUploadError()` also ran from the constructor, so an override saw a half-built object +* **Several methods added earlier in this release were renamed before it shipped.** `File::getUploadedFiles()` is `getUploadedLocators()`, since it returns storage-defined locators rather than files and collided with PSR-7's method of the same name; `FileSystem::defaultBlockedExtensions()` is `getDefaultBlockedExtensions()`; `FileSystem::statEntry()` is `lstatEntry()`, since the body is `lstat()` and not following the link is the point; `File::uploadErrorMessage()` is `formatUploadError()`; `Filename::sanitize()`/`sanitizeWithExtension()` are `sanitizeName()`/`sanitizeNameWithExtension()`, because on a class called `Filename` the short name was the obvious call and the wrong one; and `normalizeExtension()` is `acceptExtension()` on both `Filename` and `FileInfo`, because it validates and discards rather than normalizing +* `FileInfo::setExtension()` discards non-alphanumeric extensions rather than stripping characters from them (see above). Files that previously landed with a synthesized extension now land with none +* Windows reserved names are matched against the whole extension instead of as a substring. `doc.conf` kept its extension as `f` and `ico.icon` as `i`; both are now preserved intact (`conf`, `icon`). `x.aux` is still blanked, because `aux` is itself a reserved device name +* `File::__construct()` no longer creates a `FileInfo` for a single file that failed to upload, matching what the multi-file branch has always done. `count($file)` is `0` rather than `1` for e.g. `UPLOAD_ERR_NO_FILE`, and reading metadata on it no longer raises an uncaught `ValueError` +* `File::isValid()` resets the error list to the errors recorded during construction instead of appending to whatever the previous call left behind, so the usual `isValid()` then `upload()` sequence no longer reports every error twice. Validations are still re-run on every call, so a rename between the two cannot skip revalidation +* `File::humanReadableToBytes()` throws `InvalidArgumentException` on input it cannot parse. `'abc'` and `''` previously evaluated to `0`, silently configuring a `Size` bound that rejects every upload. A negative size such as `'-5M'` throws for the same reason +* `File::humanReadableToBytes()` supports fractions (`'0.5M'` → `524288`, previously `0`), tolerates surrounding and internal whitespace (`' 2 m '` → `2097152`, previously `2`) and clamps to `PHP_INT_MAX` instead of raising a `TypeError` on very large inputs +* `File::offsetSet()` throws `InvalidArgumentException` unless the value is a `FileInfoInterface`. The type was previously only a docblock, so `$file[0] = 'string';` succeeded and then faulted inside `isValid()` +* `FileInfo::getHash()` returns `''` on an unreadable file, where it previously raised a `TypeError` +* `FileInfo::getSize()` returns `false` for an unreadable file, matching the `int|false` that `FileInfoInterface` documents. On PHP 8 `SplFileInfo::getSize()` throws instead +* Sanitized filenames are valid UTF-8 where `ext-mbstring` is loaded (newly listed under `suggest`; `symfony/polyfill-mbstring` also works). Without it, truncation can still split a multibyte character, as in 3.x +* `Exception::__construct()` declares `string $message`. It was untyped while the rest of the codebase is strict + +## New Features + +* `Validation\FileType($extensions, $mimetypes)`: pairs the extension with the sniffed media type and requires them to agree. `Extension` and `Mimetype` check two independent allow-lists, so given `Extension(['png', 'gif'])` and `Mimetype(['image/png', 'image/gif'])` a file sniffed as `image/gif` and stored as `avatar.png` satisfies both. Both sides are folded to lowercase, so a custom `FileInfoInterface` returning `IMAGE/PNG` still matches `image/png`. Either side accepts a list, so `new FileType(['jpg', 'jpeg'], 'image/jpeg')` covers one format; chain `allow($extensions, $mimetypes)` for further formats rather than widening a single call, which would pair every extension with every media type +* `FileSystem::blockExtensions(array $extensions)`: the deny-list applied at the write, independent of whichever validations the caller configured. The argument is required and must not be empty — pass `getDefaultBlockedExtensions()` for the built-in set, `allowAnyExtension()` to turn the list off. Each entry is lowercased, trimmed, stripped of a leading dot and split on any remaining dots, because the list is matched one component at a time: `'.php'` blocks `php`, and `'tar.gz'` blocks `tar` and `gz` independently rather than nothing. Empty and duplicate entries are dropped. An allow-list via `Validation\FileType` remains the primary control +* `FileSystem::allowAnyExtension()`: the only way to empty the deny-list, so turning that control off is always this call rather than a config value that turned out to be missing +* `FileSystem::getDefaultBlockedExtensions(): string[]`: `EXECUTABLE_EXTENSIONS` merged with `MARKUP_EXTENSIONS`. A method rather than a constant because constant expressions cannot call `array_merge()` on PHP 7.3 +* `FileSystem::setMode(?int $mode)`: permissions for stored files, defaulting to `FileSystem::DEFAULT_MODE` +* `FileSystem::getBlockedExtensions()`, `getMode()`, `getOverwrite()`, `File::allowsUnvalidatedUploads()` and `FileType::getAllowedTypes()`: read back what was configured, so a test or a security scan can assert the policy rather than infer it +* `File::allowUnvalidatedUploads(): File`: the opt-out for the new requirement that `upload()` has something to validate against +* `File::getUploadedLocators(): string[]`: the locators written by the most recent `upload()`, emptied at the start of each call so a failure cannot hand back an earlier call's paths. A multi-file upload is not atomic, so when a later file fails this is how a caller finds out what was already committed. `getErrors()` is empty in that case, because a storage failure is not a validation failure +* `\GravityPdf\Upload\Filename`: the rules for what counts as a usable filename, in one place, so the two layers that apply them cannot drift apart. Constants `MAX_LENGTH`, `MAX_EXTENSION_LENGTH`, `FALLBACK`, `CONTROL_CHARACTERS`, `BIDI_CONTROLS` and `RESERVED_WINDOWS_NAMES`; `sanitizeNameWithExtension()` for a whole filename, plus `acceptExtension()`, `deviceComponent()`, `extensionComponents()`, `normalizeComponents()`, `isReservedDeviceComponent()`, `hasControlCharacters()` and `hasBidiControls()` for anyone reproducing the storage refusals in an implementation of their own. `CONTROL_CHARACTERS` and `BIDI_CONTROLS` are bare pattern fragments, not delimited patterns — use the two predicates rather than passing them to `preg_match()` +* `FileInfo::resetFactory(): void`: clears a factory installed with `setFactory()`. There was previously no supported way to undo it in a long-lived process or between tests +* Three new `protected` members on `Storage\FileSystem`: `resolveFilename()` decides the stored name and is the only one meant to be overridden; `reserveDestination()` and `lstatEntry()` exist so a test can reach branches that otherwise need a file-system race. `FileInfo::isReadableFile()` is the guard the metadata accessors share +* New storage exception messages: `'Invalid destination file name'`, `'Destination is a symbolic link'`, `'Permissions could not be applied to the stored file'` and `'Could not generate a temporary file name'` +* `Exception::__construct()` accepts `$code` and `$previous`, so the exception chain is no longer discarded + +## Deprecations + +Both still work and neither raises a runtime notice. + +* `Validation\Extension` and `Validation\Mimetype` are deprecated in favour of `Validation\FileType`. Used side by side they check independent allow-lists, which is the gap `FileType` closes + +## Bug Fixes + +* Case folding no longer depends on the host's locale. `strtolower()` follows `LC_CTYPE` before PHP 8.2, so on four of the eight supported versions an application calling `setlocale()` changed what counted as the same extension, media type or device name — under a Turkish locale `strtolower('TIFF')` is `tıff`, which 4.0's stricter `setExtension()` discards, storing `photo.TIFF` with no extension at all +* Filename UTF-8 handling detects `ext-mbstring` with `function_exists()` rather than `extension_loaded()`, so `symfony/polyfill-mbstring` satisfies it as the README has always said it does. The polyfill is userland and registers no extension, so an `extension_loaded()` gate would silently drop every polyfilled install onto the byte-wise fallback +* `mb_detect_encoding()` is called with an explicit detect order and falls back to UTF-8, so an application's own `mb_detect_order()` cannot change what this library makes of the same bytes. The fallback covers the orders for which detection returns `false`, where `mb_strcut()` would otherwise raise a `ValueError` +* `FileInfo::setExtension()` re-divides the 255 byte filename budget. `setName()` split it against whatever extension was set at the time, so `setName()` followed by `setExtension()` produced a name past the limit and a write that failed with a message about the destination directory +* A file named `0` keeps its name. `empty('0')` is true, so it was replaced with `unnamed-file` +* A field named `f[0][0]` no longer emits "Array to string conversion" or reports the file as `"Array: …"`. It nests `$_FILES` one level deeper than either shape `File` understands, so the entry is an array where a path belongs; those entries are now skipped with an error recorded +* `afterValidate` fires for a file that fails the uploaded-file check. The four callbacks are documented as firing once per file, and `beforeValidate` was running without its pair, so a caller using the two to open and close a per-file resource leaked on exactly the files that failed +* `FileSystem::upload()` names the file in its collision message: `'File already exists'` is now `'A file named "report.txt" already exists'`. Sanitizing is many-to-one, so two names a caller sees as distinct can resolve to one destination, and the old message gave them nothing to work with. The basename only, never the path. **Update any code matching on the old string** +* `FileSystem::upload()` distinguishes a destination it could not create from one that already exists, throwing `'Destination file could not be created'` for the first. Reporting a directory removed or de-permissioned after the constructor checked it as a collision would send the caller into a rename-and-retry loop that cannot succeed +* `Validation\Size` reports a file whose size cannot be determined as a validation error instead of letting `SplFileInfo::getSize()`'s `RuntimeException` escape +* Corrected `@throws RuntimeException` annotations on the shipped validators to `@throws \GravityPdf\Upload\Exception`, which is what they actually throw +* `StorageInterface::upload()` documents its return value. The interface declared none, and its `@throws` read "If validation fails", which storage does not do. `File::getUploadedLocators()` is the first API to hand those strings to callers, so what they are is now specified: a locator the implementation defines, an absolute path only for `Storage\FileSystem`. The README's rollback example says the same, because `unlink()` is wrong for a storage that returns a key rather than a path +* Documented that a storage exception message is for your logs rather than for the person who submitted the file. It distinguishes a name that already exists from a destination that could not be created, which is an existence check on your upload directory. `getErrors()` remains the list written to be rendered + # Upload 3.0.0 ## Breaking Changes @@ -13,7 +111,7 @@ ## Breaking Changes * PHP 7.3+ (previously PHP5.3+) * Namespace Change: `\Upload` -> `\GravityPdf\Upload` -* Sanitize Filename and Extension: replace invalid/unsafe/reserved characters/words, trim, prevent < 255 byte filenames. [See the unit tests for the expected transformations](https://github.com/GravityPDF/Upload/blob/main/tests/Upload/FileInfoTest.php#L107-L152). +* Sanitize Filename and Extension: replace invalid/unsafe/reserved characters/words, trim, prevent < 255 byte filenames. [See the unit tests for the expected transformations](https://github.com/GravityPDF/Upload/blob/main/tests/Upload/FileInfoTest.php). * Remove `\GravityPdf\Upload\File::__call()` magic method which would call the underlying `FileInfoInterface` object(s) and return the result as a string or array. Use `foreach($file as FileInfoInterface $fileInfo) { ... }` instead. * Changed return value of `\GravityPdf\Upload\Storage\FileSystem::upload()` to the destination file path `string` (previously `void`) * Strict type support added @@ -51,4 +149,4 @@ * ## Bug Fixes -* Resolved PHP 8.1 warnings \ No newline at end of file +* Resolved PHP 8.1 warnings diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..c038d87 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,132 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Project + +`gravitypdf/upload` — a standalone PHP library for validating and storing `$_FILES` uploads. It is a maintained fork of the abandoned `codeguy/upload` (declared via `"replace"` in composer.json), renamed to the `GravityPdf\Upload` namespace. + +Supports PHP 7.3 through 8.5. Any change must remain syntax- and behaviour-compatible across that whole range — CI runs the test suite on all eight versions. The only required runtime dependency is `ext-fileinfo`. `ext-mbstring` is under `suggest`: without it, filename truncation can split a multibyte character and the valid-UTF-8 guarantee does not hold. + +## Commands + +```bash +composer phpunit # full test suite +vendor/bin/phpunit --filter testConstructionWithSingleFile # single test +vendor/bin/phpunit tests/Upload/FileInfoTest.php # single file + +composer lint # PHPCS, PSR-12, over ./src and ./tests +composer lint:fix # PHPCBF autofix +composer phpstan # PHPStan level 9 over src and tests +composer check-syntax # parallel-lint across all PHP files +``` + +`phpunit`, `lint`, `phpstan` and `check-syntax` each have their own GitHub Actions workflow, run on push to `main` and on every PR. + +`composer phpstan` bootstraps PHPStan from `tools/phpstan/` rather than the root `require-dev`. PHPStan 2.x needs PHP 7.4 to run, and the root manifest has to stay resolvable on 7.3 or the 7.3 test and PHPCS jobs cannot install at all. `phpstan.neon` sets `phpVersion` to the 7.3-8.5 range, so the analysis still covers the whole supported range from whatever version runs it. + +## Architecture + +**`File` is a collection, not a file.** `new File($key, $storage)` reads `$_FILES[$key]` and normalizes both the single-file shape (`tmp_name` is a string) and the multi-file shape (`tmp_name` is an array) into an array of `FileInfoInterface` objects. `File` implements `ArrayAccess`, `IteratorAggregate` and `Countable` over that array. + +**`__call()` proxies to the collection with an asymmetric return type.** Any method not on `File` is forwarded to the underlying `FileInfo` objects: a scalar is returned when there is exactly one file, an **array** when there is more than one, and `null` when the collection is empty (`File::__call`). This asymmetry is deliberate API compatibility with v1 — don't "fix" it. It is also why `File` carries a `@mixin FileInfoInterface` annotation for PHPStan. + +**Three extension points, all interface-driven:** + +- `StorageInterface::upload(FileInfoInterface): string` — where the file lands. Only `Storage\FileSystem` ships. +- `ValidationInterface::validate(FileInfoInterface): void` — signals failure by throwing `GravityPdf\Upload\Exception`, never by returning. Four ship: `Extension`, `Mimetype`, `Size` and `FileType`. +- `FileInfoInterface` — the per-file value object. `FileInfo` extends `SplFileInfo` and implements it. + +`Validation\FileType` replaces `Extension` + `Mimetype` used side by side: those two check independent allow-lists, so a file passes both while the two answers describe different formats. `FileType` keys media types by extension, so each `allow()` call describes one format. Both older classes are `@deprecated` as of 4.0.0 but still work, with no runtime notice. + +**`upload()` requires something to validate against.** It throws `\LogicException` when `$validations` is empty, unless `File::allowUnvalidatedUploads()` was called. The type is the point: `Upload\Exception` is what a caller catches around `upload()` to handle a rejected file, and a misconfigured object must not land in that branch. The check sits in `upload()` rather than `isValid()` for the same reason — it is a configuration error for the developer, not a per-file failure to show an end user through `getErrors()`. `upload()` also throws when the collection is empty, since every validation passes vacuously against nothing. + +**The constructor never trusts the shape of `$_FILES`.** A PSR-7 bridge or test harness can supply an entry that is not an array, or one missing `tmp_name`/`name`/`error`, or a multi-file entry whose keys are not parallel arrays. Every such shape is recorded as `'An uploaded file was sent in a format that cannot be read'` rather than warning or raising a `TypeError` — remote input must not warn. + +**Validation errors accumulate; they don't abort.** `File::isValid()` runs every validation against every file and collects the failures, so `getErrors()` reports all of them at once. `upload()` throws only after the fact, with the generic message `'File validation failed'` — the detail is in `getErrors()`. A validator throwing something other than `Upload\Exception` is absorbed too, with **both** its message and its class name dropped — a PHP runtime message can contain absolute paths, and a class name is the application's internal structure. + +**`LogicException` is the exception to that.** It is re-thrown rather than absorbed, because PHP defines the type as a bug in the program. `FileInfo::getHash()` throws `InvalidArgumentException` for an unsupported algorithm precisely so a misspelling reaches the developer instead of the end user as a rejected upload. Absorb it and that guarantee is empty. + +`isValid()` resets `$this->errors` to the errors recorded during construction, so it is idempotent. It still **re-runs every validation** on each call, and `upload()` calls it again; memoizing the result would let a `setExtension()` between `isValid()` and `upload()` skip revalidation. + +Four optional callbacks (`beforeValidate`, `afterValidate`, `beforeUpload`, `afterUpload`) fire per file, each receiving that file's `FileInfoInterface`. The two validation hooks are a matched pair — `afterValidate` fires even for a file that fails the `is_uploaded_file()` check, so a caller can open and close a per-file resource across them. The upload hooks are not: a storage failure throws before `afterUpload`. + +`beforeUpload` runs **after** validation, so a name set there is never validated. Only the storage deny-list and `FileSystem`'s own filename rules apply to it. + +**`Exception` carries the offending `FileInfoInterface`** (`getFileInfo()`), so callers can tell which file in a multi-file upload failed. + +### Testability seams + +Four methods exist purely so tests can reach what a test otherwise cannot — preserve them: + +- `FileInfo::isUploadedFile()` wraps `is_uploaded_file()`. +- `FileSystem::moveUploadedFile()` (protected) wraps `move_uploaded_file()`. +- `FileSystem::lstatEntry()` (protected) wraps `@lstat()`, so a test can exercise the branch + where the stat does not answer. +- `FileSystem::reserveDestination()` (protected) is reachable from `tests/Upload/Storage/ExposedFileSystem.php`, + because `upload()`'s own `is_link()` check rejects a planted symlink before the reservation runs. + +None of the four is an extension seam; `resolveFilename()` is the only protected method on +`FileSystem` that is. + +`FileInfo::__construct` is `final`, so PHPUnit cannot subclass it freely for construction. Tests instead install a factory via the static `FileInfo::setFactory(callable)`, which `File` calls through `FileInfo::createFromFactory()`. This static is process-wide state; `FileInfo::resetFactory()` clears it and `FileTest`/`FileInfoTest` call it in `tear_down()`. `phpunit.xml` sets `backupGlobals="true"` so `$_FILES` fixtures set in `set_up()` don't leak. + +`File::formatUploadError()` sanitizes through `Filename` rather than through a `FileInfo` from the factory: sanitizing an error-path filename is the library's own guarantee and must not depend on a caller-installed implementation. + +### Filename sanitizing + +**`Filename` owns the rules; `FileInfo` and `Storage\FileSystem` apply them.** The two layers +have different outcomes on purpose — `FileInfo` rewrites a client-supplied name, storage refuses +one that still breaks a rule — but they must not disagree about what the rules *are*. That is +what went wrong before 4.0.0: both control-character filters covered C0 alone and each had to be +found separately. `MAX_LENGTH`, `MAX_EXTENSION_LENGTH`, `CONTROL_CHARACTERS`, `BIDI_CONTROLS` and +`RESERVED_WINDOWS_NAMES` are declared once, on `Filename`, and both layers call its splitters +(`deviceComponent()`, `extensionComponents()`, `normalizeComponents()`) rather than splitting for +themselves. Add a rule there, not in a caller. + +`FileInfo::setName()` sanitizes rather than validates: unsafe characters in the **name** are rewritten, never rejected. The steps, in order: + +1. Delete the characters that reorder, break or hide the text around them — `Filename::BIDI_CONTROLS`, which is Unicode's `Bidi_Control` property plus the zero-width marks, `U+2028`/`U+2029`, `U+206A`–`U+206F` and the BOM. Deleted, not rewritten, because they carry no visual content. +2. Rewrite the unsafe characters to `-`: the Windows-disallowed set, `%`, `/`, `\`, C0 and C1 controls, `\x7F`, and the sub-delimiters. Interior dots go too, so `release.config.zip` is stored as `release-config.zip`. +3. Truncate to fit 255 bytes **shared with the extension** — `255 - (strlen($extension) + 1)`, floored at 222 because `Filename::MAX_EXTENSION_LENGTH` is 32. +4. Force valid UTF-8, for names that arrived invalid. Nothing above produces invalid UTF-8 from valid input; don't re-document it as repairing this class's own damage. +5. Blank a reserved Windows device name (`con`, `nul`, `lpt1`, …) — `Filename::RESERVED_WINDOWS_NAMES`. +6. Fall back to `unnamed-file`. + +**Step 5 must stay after step 4.** Dropping an invalid byte can produce a name that was not reserved when the bytes arrived: `con\xC3.txt` is not `con` until that trailing byte goes. + +`setExtension()` re-runs steps 3–6 (`Filename::finalize()`), or `setName(300 chars)` followed by `setExtension('jpeg')` stores a 260-byte name. + +**`setExtension()` is the opposite: it validates, it does not rewrite.** It trims and lowercases first, then discards the extension entirely if anything other than letters and digits remains, if it exceeds `Filename::MAX_EXTENSION_LENGTH`, or if it is a reserved device name. `photo.PNG` keeps `png`; `doc.aux` keeps nothing. Do not "fix" this back into a strip: deletion normalizes, and `avatar.p-h-p` stripped down to `avatar.php` is a stored, executable file. + +**Case folding goes through `AsciiCase::toLower()`, never `strtolower()`.** `strtolower()` follows `LC_CTYPE` before PHP 8.2, so an application calling `setlocale()` changed what counted as the same extension, media type or device name — under `tr_TR`, `strtolower('TIFF')` is `tıff`, which `setExtension()` then discards. Everything this library folds is ASCII by definition. + +Sanitizing is **not** escaping — output still needs HTML escaping. The exact transformations are pinned by `tests/Upload/FileInfoTest.php`; change the regexes only alongside those assertions. + +### Storage invariants + +`Storage\FileSystem::upload()` does not trust `FileInfoInterface`, which is a public extension point that `FileInfo::setFactory()` lets any code in the process supply. Everything below is a backstop for an implementation that is not the shipped `FileInfo` — keep the two in step, but don't assume one makes the other redundant. + +**The write is staged, never direct.** The bytes go to `upload-<32 hex>.part` in the destination directory and are then `rename()`d onto the destination. `move_uploaded_file()` falls back to a stream copy across file systems, and that copy follows a symlink at the destination; sending it to an unguessable name is what removes the race. `rename()` replaces the directory entry rather than following it. **Don't "simplify" this back into a direct write.** + +With `overwrite = false`, `reserveDestination()` claims the name first with an exclusive `fopen(…, 'xb')` — not `is_file()` then a move, which two concurrent requests both pass. PHP resolves the path through its own stream layer before the create, so `x` **follows a dangling symlink and creates the target**; the `fstat`/`lstat` inode comparison that follows is what catches it, and `releaseReservation()` removes the file created at the far end. That comparison is POSIX-only: before PHP 7.4 Windows reports `ino` as 0. The placeholder carries the configured mode, so the name is briefly held by a 0-byte file — a process killed mid-transfer leaves it behind. + +`resolveFilename()` reduces the name to a `basename()`, strips trailing dots and spaces (Windows resolves `evil.php.` to `evil.php`), and rewrites `<>:"|?*` to `-` — rewritten, not refused, because POSIX allows all of them, and `:` would otherwise name an NTFS alternate data stream. It then **refuses** a leading `.` (so an upload cannot land as `.htaccess` or `.env`, and stays visible to globbing and cleanup scripts), C0/C1 controls and `\x7F` (which let a name forge a log line or move a terminal cursor), and anything in `Filename::BIDI_CONTROLS`. Refused rather than rewritten: inventing a filename is the value object's job, not storage's. + +**The device-name and deny-list refusals are deliberately *not* in there.** `resolveFilename()` is protected — the seam for changing how names are chosen — and while the two refusals sat inside it, a subclass that overrode it and said nothing about extensions dropped both. They run in `upload()` now, against whatever the seam returns, and both are `private`. A blocked extension is refused in **any** dot-separated component, because Apache's `AddHandler` matches any component, so `evil.php.jpg` is served as PHP. Do not move them back. + +**Three protections are on by default, each with its own opt-out**, applied in the constructor: `$overwrite = false`, `blockExtensions()` (turn off with `allowAnyExtension()`, the only route to the empty list — `blockExtensions()` takes a required, non-empty argument and throws otherwise, so a missing config key cannot quietly disable it) and `setMode(self::DEFAULT_MODE)` (turn off with `setMode(null)`). The deny-list default is `getDefaultBlockedExtensions()`, a **method** rather than a constant because PHP 7.3 constant expressions cannot call `array_merge()` on the two constants it joins. `EXECUTABLE_EXTENSIONS` is what a server runs; `MARKUP_EXTENSIONS` is what a browser renders, which is stored XSS rather than RCE, so a caller who needs SVG passes `EXECUTABLE_EXTENSIONS` alone. Entries passed to `blockExtensions()` are lowercased, trimmed, stripped of a leading dot and **split on any remaining dots**, because the list is matched one component at a time — `'tar.gz'` has to block `tar` and `gz` separately or it blocks nothing. The README table is pinned to `getDefaultBlockedExtensions()` by `FileSystemTest::testReadmeDocumentsTheDefaultDenyList()`. + +## Conventions + +- `declare(strict_types=1);` at the top of every file in `src/`. Tests do not declare it. +- PSR-12, enforced. Suppress narrowly with `/* phpcs:ignore */` (used on the polyfill `set_up()` methods) rather than relaxing the standard. +- PHPStan level 9 covers `src` **and** `tests`. `/* @phpstan-ignore-line */` is used sparingly — where `ArrayAccess` returns a nullable, and where a test deliberately passes the wrong type. +- Property types live in docblocks, not native property type declarations — PHP 7.3 support forbids the latter. Same for parameter/return types beyond what 7.3 allows (no union types, no `mixed`). +- Tests extend `Yoast\PHPUnitPolyfills\TestCases\TestCase` and use the polyfill's snake_case lifecycle hooks (`set_up()`, calling `parent::set_up()`), not `setUp()`. + +## Repository notes + +- The contributing guide and issue templates live in `.github/`; the README is at the repo root. +- `CHANGELOG.md` must be updated in the same PR as any user-facing change — it is the documented record of breaking changes between major versions. +- PRs target `main`; GitHub defaults new PRs to the upstream `codeguy/upload` repo, so the base repo needs correcting manually. diff --git a/LICENSE b/LICENSE index ec361cb..a54461b 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,5 @@ Copyright (c) 2012 Josh Lockhart +Copyright (c) 2022-2026 Gravity PDF Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/README.md b/README.md new file mode 100644 index 0000000..1b1b339 --- /dev/null +++ b/README.md @@ -0,0 +1,515 @@ +# Upload + +[![codecov](https://codecov.io/gh/GravityPDF/Upload/branch/main/graph/badge.svg)](https://codecov.io/gh/GravityPDF/Upload) +[![License: MIT](https://img.shields.io/badge/License-MIT-blue.svg)](https://opensource.org/licenses/MIT) + +A PHP library to validate and save uploaded files. + +**Why was this library forked?** + +* Original library was abandoned (untouched since 2018) +* Safe defaults: existing files are never overwritten, executable and markup extensions are + never written, stored files get mode `0640`, and `upload()` requires validation +* `Validation\FileType` requires the extension and mimetype to match +* Sanitizes filenames and extensions, with UTF-8 filename support +* Stops path traversal, symlinked destinations, dotfiles and control characters in + filenames, and writes through a staged file moved into place rather than writing to the + destination directly +* Strict type checking +* Added `FileSystem::getDirectory()` and `FileInfo::setNameWithExtension()` methods +* Included unreleased code from upstream repo +* Bumped minimum PHP version to 7.3+ +* PSR-12 Code Formatting +* Automated tools: PHPUnit, PHPStan, PHPCS, and PHP Syntax Checker + +## Installation + +``` +composer require gravitypdf/upload +``` + +### Requirements + +PHP 7.3 to 8.5 and `ext-fileinfo`. Optional but recommended: `ext-mbstring` (or `symfony/polyfill-mbstring`) so filenames can be guaranteed UTF-8. + +Migrating from `codeguy/upload`? Version 3.x of this package is a drop-in replacement: +update your imports from `\Upload\…` to `\GravityPdf\Upload\…`. + +Upgrading from 3.x? Version 4.0 turns new protections on by default. The +[upgrade guide](https://github.com/GravityPDF/Upload/blob/main/UPGRADE.md) covers what +changed and what to check. + +## Usage + +### Single-file upload + +Assume a file is uploaded with this HTML form: + +```html +
+ + +
+``` + +Server-side, validate the upload, rename it, and store it: + +```php +use GravityPdf\Upload\File; +use GravityPdf\Upload\Storage\FileSystem; +use GravityPdf\Upload\Validation\FileType; +use GravityPdf\Upload\Validation\Size; + +// Store uploads where the web server will not execute or serve them directly +$storage = new FileSystem('/path/to/uploads'); + +// Reads $_FILES['avatar'] +$file = new File('avatar', $storage); + +// upload() refuses to run unless at least one validation is added +$file->addValidations([ + new FileType('png', 'image/png'), // extension and file contents must both say PNG + new Size('2M'), // max 2 MiB ("B", "K", "M" or "G") +]); + +// isValid() also checks is_uploaded_file(), so call it before reading any metadata +if ($file->isValid() === false) { + foreach ($file->getErrors() as $message) { + echo htmlspecialchars($message, ENT_QUOTES, 'UTF-8'), '
'; // always escape on output + } + + return; +} + +// Store under a random name; keep the client's (sanitized) name for display only +$displayName = $file->getNameWithExtension(); +$file->setName(bin2hex(random_bytes(16))); + +try { + $file->upload(); + + $storedPath = $file->getUploadedLocators()[0]; +} catch (\Exception $e) { + // Validation has already passed, so this is a storage failure: the destination + // exists, the extension is blocked, or the disk is full +} +``` + +`FileType` accepts any registered +[IANA media type](https://www.iana.org/assignments/media-types/media-types.xhtml), such as +`image/png` or `application/pdf`. + +### Reading file metadata + +```php +$data = [ + 'name' => $file->getNameWithExtension(), // sanitized client name; display only + 'extension' => $file->getExtension(), + 'mime' => $file->getMimetype(), // sniffed from the contents, not the client's claim + 'size' => $file->getSize(), // bytes, or false if the file is unreadable + 'hash' => $file->getHash(), // sha256 unless you pass another algorithm + 'dimensions' => $file->getDimensions(), // ['width' => int, 'height' => int] +]; +``` + +These calls are forwarded to every file in the collection. With one file you get the value +back, with several you get an array of values, and with none you get `null`. When a field +accepts multiple files, read metadata per file instead. + +### Multi-file upload + +```html +
+ + +
+``` + +The `$_FILES` key drops the brackets: `new File('photos', $storage)`. `File` acts as a +collection of the individual files, so `count()`, `foreach` and array offsets all work. A +file that failed to transfer (too large, nothing selected) is left out, and its error +message is already in `getErrors()`. + +```php +use GravityPdf\Upload\File; +use GravityPdf\Upload\Storage\FileSystem; +use GravityPdf\Upload\Validation\FileType; +use GravityPdf\Upload\Validation\Size; + +$storage = new FileSystem('/path/to/uploads'); +$file = new File('photos', $storage); + +$file->addValidations([ + // One format per call, otherwise every extension is paired with every media type + (new FileType(['jpg', 'jpeg'], 'image/jpeg')) + ->allow('png', 'image/png') + ->allow('webp', 'image/webp'), + new Size('10M'), +]); + +// An empty collection has nothing to fail validation, so check the count as well +if (count($file) === 0 || $file->isValid() === false) { + foreach ($file->getErrors() as $message) { + echo htmlspecialchars($message, ENT_QUOTES, 'UTF-8'), '
'; + } + + return; +} + +// Rename each file server-side, keeping the client names for display +$manifest = []; +foreach ($file as $photo) { + $displayName = $photo->getNameWithExtension(); + $photo->setName(bin2hex(random_bytes(16))); + + $manifest[] = [ + 'display' => $displayName, + 'stored' => $photo->getNameWithExtension(), + 'hash' => $photo->getHash(), + ]; +} + +try { + $file->upload(); +} catch (\Exception $e) { + // Multi-file uploads are not atomic: earlier files may already be on disk. + // getUploadedLocators() lists what was written so the batch can be rolled back. + // unlink() is right for Storage\FileSystem, which returns local paths. A storage + // of your own returns whatever locator it defines, so undo it the way it stores. + foreach ($file->getUploadedLocators() as $uploadedPath) { + unlink($uploadedPath); + } + + return; +} +``` + +Individual files are also reachable by offset. Check with `isset($file[0])` first, because +files that failed to transfer are missing from the collection. + +### Lifecycle callbacks + +Four optional hooks fire once per file, each receiving that file's `FileInfoInterface`: +`beforeValidate`, `afterValidate`, `beforeUpload` and `afterUpload`. Use them for per-file +work like renaming or audit logging without writing your own loops. + +The two validation hooks are a matched pair: `afterValidate` fires for every file +`beforeValidate` fired for, including one that failed, so they can safely open and close a +per-file resource. The upload hooks are not a pair — a storage failure throws out of +`upload()` before `afterUpload` runs. + +```php +use GravityPdf\Upload\FileInfoInterface; + +$file->beforeUpload(static function (FileInfoInterface $fileInfo): void { + $fileInfo->setName(bin2hex(random_bytes(16))); +}); + +$file->afterUpload(static function (FileInfoInterface $fileInfo): void { + error_log(sprintf('Stored upload as %s', $fileInfo->getNameWithExtension())); +}); +``` + +**`beforeUpload` runs after validation, not before it**, so a name set there is never +validated — only the storage deny-list and `FileSystem`'s filename rules apply. `setName()` +is safe there, since it cannot change the extension. `setExtension()` and +`setNameWithExtension()` are not: given anything derived from user input, they can store a +file under an extension your validations would have rejected, and the deny-list covers only +[the formats below](#extensions-blocked-by-default). + +Rename in `beforeValidate` if the final name has to be the validated one. + +### Custom validation rules + +Implement `ValidationInterface` and throw `GravityPdf\Upload\Exception` to reject a file. +The exception message is what `getErrors()` shows the end user: + +```php +use GravityPdf\Upload\Exception; +use GravityPdf\Upload\FileInfoInterface; +use GravityPdf\Upload\ValidationInterface; + +class MaxDimensions implements ValidationInterface +{ + private $maxWidth; + private $maxHeight; + + public function __construct(int $maxWidth, int $maxHeight) + { + $this->maxWidth = $maxWidth; + $this->maxHeight = $maxHeight; + } + + public function validate(FileInfoInterface $fileInfo): void + { + $size = $fileInfo->getDimensions(); + + if ($size['width'] > $this->maxWidth || $size['height'] > $this->maxHeight) { + throw new Exception( + sprintf('Image must be no larger than %dx%d pixels', $this->maxWidth, $this->maxHeight), + $fileInfo + ); + } + } +} + +$file->addValidation(new MaxDimensions(2048, 2048)); +``` + +Failures accumulate rather than abort: every validation runs against every file, and +`getErrors()` reports them all at once. + +Throwing anything other than `GravityPdf\Upload\Exception` is caught too, but nothing it +carries reaches `getErrors()` — not its message, which can leak server paths, and not its +class name. Catch it in the validator and rethrow an `Upload\Exception` if either belongs in +what the user sees. + +`\LogicException` is the exception to that: it propagates out of `isValid()`, since PHP +defines the type as a bug in your program rather than a file that failed. A validator +calling `getHash()` with a misspelled algorithm reaches you, not the end user. + +### Custom storage backends + +Implement `StorageInterface` to store files somewhere other than the local filesystem: +read from `$fileInfo->getPathname()`, return the destination, and throw +`GravityPdf\Upload\Exception` on failure. + +The string you return is a locator you define — a key, a URL, an identifier — +and `File::getUploadedLocators()` hands it back unchanged, so it is what the application +rolls back with. Never return `''`: a caller cannot tell it from a usable value. + +```php +use GravityPdf\Upload\FileInfoInterface; +use GravityPdf\Upload\StorageInterface; + +class ObjectStorage implements StorageInterface +{ + public function upload(FileInfoInterface $fileInfo): string + { + $key = 'uploads/' . $fileInfo->getNameWithExtension(); + + // ... stream $fileInfo->getPathname() to your object store ... + + return $key; + } +} +``` + +The protections under "Security notes" — the deny-list, the `basename()` reduction, the +symlink refusal, the staged write — live in `Storage\FileSystem`. A custom backend needs +its own equivalents. + +## Security notes + +**Prefer `FileType` over `Mimetype` and `Extension` separately.** Those two check independent +lists, so content sniffed as `image/gif` stored as `avatar.png` satisfies both. `FileType` +requires the extension and the contents to describe the same format. + +**Generate storage names server-side.** Sanitizing normalizes client names, so `report.txt` +and `report!.txt` both land on `report.txt`. A server-side name avoids the collision and the +predictable destination: + +```php +$file->setName(bin2hex(random_bytes(16))); // keep the client name as display metadata only +``` + +**Sanitizing is not escaping.** Unsafe characters are rewritten, not escaped. Escape on +output and use parameterized queries. This applies to `getErrors()`. + +**Show `getErrors()`, log the exception.** Every string in `getErrors()` is sanitized and +describes the submitted file. A storage exception message is not: it distinguishes a name +that already exists from a destination that could not be created, which tells whoever +submitted the file what is in your upload directory. Log it and show something generic. + +**Call `isValid()` before reading metadata.** It performs the `is_uploaded_file()` check; +the metadata accessors do not. This matters where `$_FILES` is rebuilt by something other +than the PHP SAPI (PSR-7 bridges, test harnesses, middleware). + +**Serve uploads from a directory the web server won't execute.** The storage defaults below +are backstops, not a substitute for that. + +### Extensions blocked by default + +`FileSystem` refuses to write these, whatever the validations allowed. The check runs against +the sanitized extension about to be written and throws `\GravityPdf\Upload\Exception` rather +than recording a validation error. + +Dots inside the name are not extension separators: `FileInfo::setName()` rewrites them to +hyphens, so `release.config.zip` is stored as `release-config.zip`. `upload()` checks every +dot-separated component regardless, for names reaching it from a `FileInfoInterface` of +your own. + +| Group | Extensions | +|---|---| +| PHP | `php` `php2` `php3` `php4` `php5` `php6` `php7` `php8` `phps` `phtml` `phtm` `phar` `pht` `inc` | +| Server-side includes | `shtml` `shtm` `stm` | +| CGI and scripts | `cgi` `fcgi` `pl` `py` `rb` `sh` `bash` `ps1` | +| Java | `jsp` `jspx` `jspf` `jsw` `jsv` `jshtml` `jar` `war` | +| ASP / ASP.NET | `asp` `aspx` `asa` `asax` `ascx` `ashx` `asmx` `cer` `cshtml` `vbhtml` | +| Windows binaries | `exe` `dll` `com` `bat` `cmd` `msi` `scr` `vbs` `ws` `wsf` `hta` | +| Server configuration | `htaccess` `htpasswd` `ini` `conf` `config` | +| Markup and script | `html` `htm` `xhtml` `xht` `xhtm` `svg` `svgz` `xml` `xsl` `xslt` `js` `mjs` `swf` `mht` `mhtml` | + +The first seven groups are `FileSystem::EXECUTABLE_EXTENSIONS`, which a server runs. The +last is `FileSystem::MARKUP_EXTENSIONS`, which a browser renders — serving one from your own +origin is stored XSS. SVG is in that group because it carries `