Refuse the extension a whitespace-only name hid

CHANGELOG.md

@@ -4,7 +4,7 @@ A security release. New protections are on by default and will refuse some uploa
 
 ## Defaults That Changed
 
-* **An extension deny-list is on.** `FileSystem` refuses to write anything in `FileSystem::getDefaultBlockedExtensions()`: the 59 executable extensions in `EXECUTABLE_EXTENSIONS`, plus 15 markup ones in `MARKUP_EXTENSIONS` (`html`, `htm`, `xhtml`, `xht`, `xhtm`, `svg`, `svgz`, `xml`, `xsl`, `xslt`, `js`, `mjs`, `swf`, `mht`, `mhtml`). A server doesn't execute markup, but serving it from your own origin is stored XSS. **To accept one of them, drop those entries and keep the rest of the list** — `array_diff(FileSystem::getDefaultBlockedExtensions(), ['svg', 'svgz'])` — and sanitize the contents yourself
+* **An extension deny-list is on.** `FileSystem` refuses to write anything in `FileSystem::getDefaultBlockedExtensions()`: the 68 executable extensions in `EXECUTABLE_EXTENSIONS`, plus 15 markup ones in `MARKUP_EXTENSIONS` (`html`, `htm`, `xhtml`, `xht`, `xhtm`, `svg`, `svgz`, `xml`, `xsl`, `xslt`, `js`, `mjs`, `swf`, `mht`, `mhtml`). A server doesn't execute markup, but serving it from your own origin is stored XSS. **To accept one of them, drop those entries and keep the rest of the list** — `array_diff(FileSystem::getDefaultBlockedExtensions(), ['svg', 'svgz'])` — and sanitize the contents yourself
 * **A blocked extension is refused in any dot-separated component of the name**, not only the one `pathinfo()` returns, since a web server does not necessarily treat the last component as the extension. Trailing dots and spaces are removed first, and the first component is never treated as an extension, so a file called `php` is still stored. The shipped `FileInfo` presents one component either way: `setName()` rewrites interior dots to hyphens as it always has, storing `archive.config.zip` as `archive-config.zip`
 * **Stored files get mode `0640`** (`FileSystem::DEFAULT_MODE`). 3.x left the mode to the umask
 * **`File::upload()` throws when no validations have been added**
@@ -17,13 +17,13 @@ A security release. New protections are on by default and will refuse some uploa
 * **`FileInfo::setName()` rewrites the C1 controls (`U+0080`–`U+009F`) and `\x7F` to `-`.** The filter covered C0 only, 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, in `resolveFilename()`
 * **`FileInfo::setName()` deletes bidi and invisible characters** (`Filename::BIDI_CONTROLS`): Unicode's `Bidi_Control` property including `U+061C`, the zero-width marks `U+200B`–`U+200F`, the line and paragraph separators `U+2028` and `U+2029`, `U+206A`–`U+206F`, and the BOM. They let a name display in an admin listing, email or log as something other than what it is. `Storage\FileSystem` refuses a name still carrying one, in `upload()` rather than in the `resolveFilename()` seam. `Filename::hasControlCharacters()` and `hasBidiControls()` are public so an override can apply them itself
 * **`Filename::RESERVED_WINDOWS_NAMES` gained `COM0`, `LPT0` and the superscript variants** (`COM¹`, `COM²`, `COM³`, `LPT¹`, `LPT²`, `LPT³`), which Microsoft lists alongside `COM1`–`COM9`. A matching name is blanked to `unnamed-file`
-* **Storage refuses a destination that is not a plain filename.** The name is reduced to a `basename()`, and `''`, `.`, `..`, a leading dot and names containing a null byte are rejected. `resolveFilename()`, the protected naming seam, derives the name; `upload()` decides whether it may be written, so **an override of the seam cannot drop any of these refusals**. The leading dot mattered most: the deny-list carries `htaccess`, but a dotfile presents no extension for it to match, so `.htaccess` reached the upload directory
+* **Storage refuses a destination that is not a plain filename.** The name is reduced to a `basename()`, and `''`, `.`, `..`, a leading dot and names containing a null byte are rejected. `resolveFilename()`, the protected naming seam, derives the name; `upload()` decides whether it may be written, so **an override of the seam cannot drop any of these refusals**. The shipped `FileInfo` never hands storage one of these names, in 3.x any more than now: `../escape.txt` came out as `escape.txt` and `.htaccess` as `unnamed-file.htaccess`, which the deny-list refuses in turn. A custom implementation had nothing behind it in 3.x — one returning `../escape.txt` wrote outside the upload directory, and one returning `.htaccess` wrote it literally. The leading dot is refused here rather than by the deny-list, which is a list of extensions and does not cover `.env`
 * **`FileSystem::upload()` refuses a destination that is a symbolic link**
 * **Uploads are staged.** The file is written to a temporary name in the destination directory and moved onto the destination in one operation, which doesn't follow a symlink standing there, so no partial content is readable under the final name
 * **The overwrite guard no longer has a check-then-create window.** With `overwrite = false` the existence check and the create are one exclusive operation, so two concurrent requests can no longer both claim the same name, and a placeholder left by a failed move is cleaned up. On POSIX the file created is then verified by inode to be the destination rather than a symlink's target; Windows before PHP 7.4 reports no inode, so that verification does not hold there. A process killed mid-transfer leaves a 0-byte file
 * **A failed `chmod()` aborts the upload** rather than storing the file at the umask's mode while reporting `0640`
 * **`Validation\FileType::allow()` rejects empty and whitespace-only values.** An empty media type is what `getMimetype()` returns for a file it can't read, and it would have matched one. A lone `.` is rejected as an extension; a leading dot is accepted and removed
-* **Every string `getErrors()` returns is sanitized, message as well as filename.** Raw `$_FILES[…]['name']` reached it from the constructor, a name from a custom `FileInfoInterface` from `isValid()`, and a validation failure's message was reported exactly as thrown, so a line break, terminal escape or bidi override could land in a string the README tells you to render. Both halves now go through `Filename::sanitizeForDisplay()`: bidi controls deleted, control characters collapsed to a space, and the result forced to valid UTF-8 where `ext-mbstring` is loaded, so `json_encode(getErrors())` cannot return `false` on one bad byte. No shipped validator puts anything but configuration into a message, so that half is for a validator of your own. The shipped `FileInfo` was unaffected, and sanitizing is still not escaping. Every append goes through one `protected` recorder, `File::recordError()`, so a `File` subclass recording an error of its own is covered too
+* **Every string `getErrors()` returns is sanitized, message as well as filename.** Raw `$_FILES[…]['name']` reached it from the constructor, a name from a custom `FileInfoInterface` from `isValid()`, and a validation failure's message was reported exactly as thrown, so a line break, terminal escape or bidi override could land in a string the README tells you to render. Both halves now go through `Filename::sanitizeForDisplay()`: bidi controls deleted, control characters collapsed to a space, and the result forced to valid UTF-8 where `ext-mbstring` is loaded, so `json_encode(getErrors())` cannot return `false` on one bad byte. No shipped validator puts anything but configuration into a message, so that half is for a validator of your own. The shipped `FileInfo` was unaffected, and sanitizing is still not escaping. A string is also cut to `Filename::MAX_DISPLAY_LENGTH` (2048 bytes), which is longer than any message this library composes and short enough that a validator of your own cannot flood whatever renders the list. Every append goes through one `protected` recorder, `File::recordError()`, so a `File` subclass recording an error of its own is covered too
 * **`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 warnings
 * **A lifecycle callback can't call back into its own `File`.** `isValid()`, `upload()` and `uploadValid()` throw `\LogicException` when one is already running on that object. Each run resets the error list at its start, and `upload()` and `uploadValid()` reset the locator list too, so a nested call could send a failed file to storage and lose the record of what was already written
 * **A validator throwing something other than `Upload\Exception` no longer aborts the batch.** It records `Validation could not be completed` and nothing else: not the message, which can contain server paths, and not the class name. `\LogicException` is re-thrown, since PHP defines that type as a bug in the program

CLAUDE.md

@@ -6,7 +6,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 
 `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. All three configurations `composer.json` permits are covered by CI — the extension, `symfony/polyfill-mbstring`, and neither — because nothing else sees the difference: an unguarded `mb_*` call once turned a rejected upload into a fatal error on an install without the extension. `forceValidUtf8()` owns that guard now, so it travels with the function rather than with a caller. The polyfill does **not** ship `mb_strcut()`, which is why `finalize()` gates only its truncation on that and calls `forceValidUtf8()` unconditionally; it is also `iconv()`-backed and warns on the input the repair exists for, so that call is silenced with `@`.
+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. All three configurations `composer.json` permits are covered by CI — the extension, `symfony/polyfill-mbstring`, and neither — because nothing else sees the difference: an unguarded `mb_*` call once turned a rejected upload into a fatal error on an install without the extension. `forceValidUtf8()` owns that guard now, so it travels with the function rather than with a caller. The polyfill does **not** ship `mb_strcut()`, which is why `finalize()` gates only its truncation on that and calls `forceValidUtf8()` unconditionally; it is also `iconv()`-backed and warns on the input the repair exists for, so that call is silenced with `@`. `iconv()` answers `false` for a sequence the end of the string cuts short, where the extension drops it and keeps the rest, so `forceValidUtf8()` takes the partial tail off before it converts. Without that, a client name ending mid-character came back as `unnamed-file` on a polyfilled install.
 
 ## Commands
 
@@ -141,6 +141,11 @@ found separately. `MAX_LENGTH`, `MAX_EXTENSION_LENGTH`, `CONTROL_CHARACTERS`, `B
 (`deviceComponent()`, `extensionComponents()`, `normalizeComponents()`) rather than splitting for
 themselves. Add a rule there, not in a caller.
 
+`extensionComponents()` splits on `.` and drops the first field **before** it normalizes.
+Normalizing first drops a component that is nothing but spaces, leaving the extension as the
+only field for the drop to take: `" .php"` normalized to `['php']`, the deny-list was handed
+`[]`, and the file was written.
+
 `Filename::sanitizeForDisplay()` is the same rules applied to prose rather than to a name — the two
 character sets, with controls collapsed to a space instead of `-` and none of the filename
 budget, device-name or `%`/`/` handling. `File` runs a validator's message through it and
@@ -178,7 +183,7 @@ With `overwrite = false`, `reserveDestination()` claims the name first with an e
 
 `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. That is all it does: it decides what the name **is**, and `upload()` decides whether it may be written.
 
-**No refusal is in there.** `resolveFilename()` is protected — the seam for changing how names are chosen — and while a refusal sat inside it, an override that said nothing about the subject dropped it. `upload()` runs all three against whatever the seam returns, and all three are `private`: `refuseUnsafeName()` (`''`, a leading `.`, `Filename::CONTROL_CHARACTERS`, `Filename::BIDI_CONTROLS`), `refuseReservedWindowsName()` and `refuseBlockedExtensions()`. The leading `.` is in the first of those rather than left to the deny-list because the deny-list cannot see one: `extensionComponents()` drops the empty component before the dot along with the first real one, so `.htaccess` presents nothing to match and `.env` is not on the list at all. A refusal rather than a rewrite throughout — inventing a filename is the value object's job, not storage's. **Do not move any of them back into the seam**, and do not answer "a subclass can reproduce it": `return basename($fileInfo->getNameWithExtension())` is the override people actually write. The collision message in `reserveDestination()` treats the seam's output the same way, running the basename through `Filename::sanitizeForDisplay()` before quoting it — the base `resolveFilename()` refuses control and bidi characters, an override need not, and a storage message is written to a log. A name left with nothing by that says `'A file with that name already exists'` rather than quoting an empty string. A blocked extension is refused in **any** dot-separated component, because a web server does not necessarily treat the last component as the extension. Do not narrow that to one component, and do not move the two refusals back.
+**No refusal is in there.** `resolveFilename()` is protected — the seam for changing how names are chosen — and while a refusal sat inside it, an override that said nothing about the subject dropped it. `upload()` runs all three against whatever the seam returns, and all three are `private`: `refuseUnsafeName()` (`''`, a leading `.`, `Filename::CONTROL_CHARACTERS`, `Filename::BIDI_CONTROLS`), `refuseReservedWindowsName()` and `refuseBlockedExtensions()`. The leading `.` is in the first of those rather than left to the deny-list, which is a list of extensions and does not cover `.env` at all. A refusal rather than a rewrite throughout — inventing a filename is the value object's job, not storage's. **Do not move any of them back into the seam**, and do not answer "a subclass can reproduce it": `return basename($fileInfo->getNameWithExtension())` is the override people actually write. The collision message in `reserveDestination()` treats the seam's output the same way, running the basename through `Filename::sanitizeForDisplay()` before quoting it — the base `resolveFilename()` refuses control and bidi characters, an override need not, and a storage message is written to a log. A name left with nothing by that says `'A file with that name already exists'` rather than quoting an empty string. A blocked extension is refused in **any** dot-separated component, because a web server does not necessarily treat the last component as the extension. Do not narrow that to one component, and do not move the two refusals back.
 
 **`move_uploaded_file()` is the storage half of the provenance decision.** It refuses any source PHP did not receive as an upload, so `Storage\FileSystem` cannot store a `FileList` file — the class validates and then fails at the write — until `acceptFilesNotUploadedByPhp()` says otherwise. That is deliberate, and the pairing is the point: `FileInfo::isUploadedFile()` says where the file came from, this says the caller is willing to store it, and neither is a default. Do not make the fallback automatic on a failed `move_uploaded_file()` — a path an attacker steered would take exactly that branch, which is the whole attack the SAPI check exists to stop. With the opt-in on, `moveFile()` renames, with a copy behind it. The copy is **not** the cross-file-system path, whatever the PSR-7 adapter plan says: `rename(2)` does fail with `EXDEV` between a tmpfs tmp directory and a disk upload directory, but PHP's plain-files wrapper catches that itself and copies, so `rename()` returns `true` — verified against two real mounts, and pinned by a test that skips when it cannot find a second file system. The fallback covers what PHP does not absorb, a stream-wrapper source among it. Both send the bytes at the **staging** path rather than the destination, so the copy's symlink-following is closed off by the unguessable name the same way `move_uploaded_file()`'s own copy fallback is. `moveFile()` is `protected` for the same testability reason `moveUploadedFile()` is; `moveIntoStaging()` picks between them and is `private`.