Make the suite pass on Windows

.gitattributes

@@ -7,3 +7,7 @@
 /phpunit.xml    export-ignore
 /phpstan.neon   export-ignore
 /CLAUDE.md      export-ignore
+
+# The catalogue's exact bytes are a guarantee: the `i18n` workflow regenerates it and fails on
+# a diff, and `CatalogueTest` parses it line by line. A CRLF checkout on Windows breaks both.
+/i18n/upload.pot text eol=lf

.github/workflows/phpunit.yml

@@ -93,13 +93,14 @@ jobs:
   # nothing. Excluded by group rather than skipped, so `cross-file-system` keeps the single
   # `markTestSkipped()` its `--fail-on-skipped` guard depends on.
   #
-  # `continue-on-error` until this has been green once: nothing in the project has ever run on
-  # Windows, so the first runs are discovery rather than a gate. Remove it, and this comment,
-  # once the job passes — a red job nobody may merge past is the point of adding it.
+  # It found two library bugs on its first outing, both on the default `$overwrite = false`
+  # path and neither reachable from Linux: the reservation's inode comparison read Windows'
+  # absent inode as a mismatch and refused every upload, and `releaseReservation()` inferred
+  # "not a symlink" from a failed `readlink()`, which answers a regular file with its own path
+  # there. It is a gate now, so a third does not go unnoticed.
   windows:
     name: ${{ matrix.php }} on windows-latest
     runs-on: windows-latest
-    continue-on-error: true
     strategy:
       fail-fast: false
       matrix:

CHANGELOG.md

@@ -98,6 +98,9 @@ Both still work and neither raises a runtime notice.
 
 ## Bug Fixes
 
+* **A reservation that has to be released is released on Windows too.** `releaseReservation()` decided whether the destination was a symlink by whether `readlink()` failed. PHP's Windows `readlink()` answers a regular file with its own canonical path rather than failing, so the 0-byte placeholder took the symlink branch, matched nothing there, and was left behind — holding the caller's name against every later upload of it. `is_link()` asks the question directly, on every platform
+* **`Storage\FileSystem` can store a file on Windows under PHP 7.3 again.** `reserveDestination()` confirms its exclusive create by comparing `fstat()` and `lstat()`, which catches an `x` that followed a symlink. That platform reports `ino` as 0 for both, so the comparison had nothing to compare — and reading no information as a mismatch answered `'Destination is a symbolic link'` to every reservation, which is every upload the default `$overwrite = false` makes. The comparison is skipped where the inode is unavailable; a real file has a real inode, so nothing changes on POSIX, and the symlink protections here were already documented as not load-bearing on Windows. **7.3 only** — 7.4 onwards reports an inode there
+* **A backslash in a client filename is a character, not a path separator.** `FileInfo::setNameWithExtension()` and `Filename::sanitizeNameWithExtension()` split through `pathinfo()`, which treats `\` as a separator on Windows and as an ordinary character on POSIX — so `a\b.txt` was stored as `a-b.txt` on one and `b.txt` on the other, and `..\..\windows\win.ini` as `windows-win.ini` or `win.ini`. `Filename::rewriteCharacters()` rewrites `\` to `-`, so the rule both layers read from `Filename` is that it stays in the name; a rule cannot depend on which platform applies it. `Filename::splitNameAndExtension()` owns the split now and treats `/` alone as a separator, on every platform. **On Windows this keeps name content that was previously discarded**
 * **`$file[] = $fileInfo` appends instead of silently discarding a file.** PHP passes `offsetSet()` a null offset for the append syntax, and assigning it straight through wrote the string key `''` rather than the next integer: the second append overwrote the first, and a key the `ArrayAccess<int, FileInfoInterface>` contract does not admit reached `getUploadedLocators()`, which `store()` keys by collection offset. PHP 8.5 also deprecates the null offset, so every append raised a notice. On `FileList`, the offset the append lands at is read back before the source key is dropped, so `getSourceKeys()[$i]` keeps naming `$list[$i]`
 * **`Validation\Size` rejects a bound that is not a byte count, at construction.** A float — what a limit read out of JSON or arrived at by division actually is — reached the `int`-typed `scale()` and raised a `TypeError` from inside `validate()`, where `File::runValidations()` absorbed it as `Validation could not be completed`: the developer's misconfiguration shown to whoever submitted the file, with nothing anywhere naming the cause. Both bounds are now checked in the constructor, along with a negative bound and a minimum above the maximum, all as `InvalidArgumentException`
 * **`Validation\Mimetype` folds its allow-list.** `Extension` and `FileType` both put theirs through `AsciiCase::toLower()`; this one compared what it was given. A media type is case-insensitive and `FileInfo::getMimetype()` always answers lowercase, so `new Mimetype(['IMAGE/PNG'])` rejected every PNG. The sniffed type is folded too, which only a custom `FileInfoInterface` can arrive with in another case

CLAUDE.md

@@ -47,7 +47,9 @@ The `i18n` workflow regenerates the catalogue and fails on a diff, which is what
 
 The `phpunit` workflow carries a second job, `cross-file-system`, which mounts a tmpfs and points `UPLOAD_TEST_OTHER_FS` at it so `FileSystemTest::testStoresAFileFromAnotherFileSystem()` runs against two real file systems rather than skipping. It runs the **whole suite** with `--fail-on-skipped` rather than filtering to that test: a `--filter` matching nothing exits 0 with "No tests executed!", so it needed a second guard to prove it had run, and renaming the test was enough to trigger exactly that. That test is the suite's only `markTestSkipped()`, which is what makes one guard sufficient — keep it that way, or the job goes quiet. Tests that cannot run in a given configuration are excluded by group instead: `@group mbstring` marks the ones that need the UTF-8 repair, so the `no-mbstring` job drops them with `--exclude-group` rather than adding a second skip, and `@group posix` marks the fourteen that need `symlink()`, `chmod()` or `umask()` to mean something, which the `windows` job drops the same way. Set `UPLOAD_TEST_OTHER_FS` to run it locally: on macOS, `hdiutil attach -nomount ram://8192` then `diskutil erasevolume HFS+ UPLOADTMP <disk>`.
 
-The `windows` job is `continue-on-error: true` until it has been green once — nothing in this project had ever run on Windows, so the first runs are discovery rather than a gate. **Remove the flag once it passes**, and the comment above it with it: a job nobody may merge past is why it is there. `phpunit.xml` sets `convertDeprecationsToExceptions`, `failOnWarning` and `failOnRisky`, so a deprecation raised from `src/` fails the suite rather than passing through a green run — which is how `$file[] = $fileInfo` wrote a string key and warned about it under PHP 8.5 while the suite reported OK.
+`Filename::splitNameAndExtension()` is what `FileInfo::setNameWithExtension()` and `Filename::sanitizeNameWithExtension()` split with, not `pathinfo()`: that treats `\` as a path separator on Windows and as an ordinary character on POSIX, so one client name split two ways. `rewriteCharacters()` rewrites `\` to `-`, so the shared rule is that a backslash stays in the name. `/` is a separator on every platform. Do not put `pathinfo()` back.
+
+The `windows` job is a gate. It found two bugs on its first outing, both on the default `$overwrite = false` path and neither reachable from Linux: `reserveDestination()` read Windows' absent inode as a mismatch and refused every upload as a symlink, and `releaseReservation()` inferred "not a symlink" from a failed `readlink()`, which on Windows answers a regular file with its own canonical path. Ask `is_link()`, and gate an inode comparison on the inode being available. `phpunit.xml` sets `convertDeprecationsToExceptions`, `failOnWarning` and `failOnRisky`, so a deprecation raised from `src/` fails the suite rather than passing through a green run — which is how `$file[] = $fileInfo` wrote a string key and warned about it under PHP 8.5 while the suite reported OK.
 
 `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.
 

UPGRADE.md

@@ -324,6 +324,15 @@ Each of these is listed in full in the [changelog](CHANGELOG.md).
   sequence no longer reports every error twice.
 * `$file[0] = $value` throws `InvalidArgumentException` unless the value is a
   `FileInfoInterface`.
+* **Windows under PHP 7.3 could not store a file at all with the default `$overwrite = false`,
+  and now can.** The reservation's inode check read that platform's absent inode as a mismatch
+  and refused every upload as `'Destination is a symbolic link'`. Nothing changes on POSIX or
+  on PHP 7.4 and later.
+* **On Windows, a backslash in a client filename is no longer treated as a path separator.**
+  `pathinfo()` splits on it there and not on POSIX, so `a\b.txt` was stored as `b.txt` under
+  Windows and `a-b.txt` everywhere else. It is now `a-b.txt` on both, which is what
+  `Filename`'s documented rules always said. A Windows deployment relying on the implicit
+  basename-ing gets longer names than before; nothing on POSIX changes.
 * `$file[] = $value` appends. In 3.x it wrote the string key `''`, so a second append
   overwrote the first and `getUploadedLocators()` came back with a key that is not an offset.
 * **`Validation\Size` now throws `InvalidArgumentException` for a bound it cannot use**: one

src/Upload/FileInfo.php

@@ -189,11 +189,15 @@ public function getNameWithExtension(): string
 
     public function setNameWithExtension(string $name): FileInfo
     {
+        /* `Filename` rather than `pathinfo()`, which treats `\` as a separator on Windows and
+           not on POSIX — the same client name split two ways. See splitNameAndExtension(). */
+        list($base, $extension) = Filename::splitNameAndExtension($name);
+
         /* Not setExtension(): that re-fits the name to the new budget, and the setName() below
            overwrites the result unconditionally. Assign the extension, then let setName() do
            the one fit that survives. */
-        $this->extension = $this->acceptExtension(pathinfo($name, PATHINFO_EXTENSION));
-        $this->setName(pathinfo($name, PATHINFO_FILENAME));
+        $this->extension = $this->acceptExtension($extension);
+        $this->setName($base);
 
         return $this;
     }

src/Upload/Filename.php

@@ -143,12 +143,49 @@ public static function sanitizeName(string $name, string $extension = '', ?array
      */
     public static function sanitizeNameWithExtension(string $filename, ?array $reserved = null): string
     {
-        $extension = self::acceptExtension((string) pathinfo($filename, PATHINFO_EXTENSION), $reserved);
-        $name = self::sanitizeName((string) pathinfo($filename, PATHINFO_FILENAME), $extension, $reserved);
+        list($name, $extension) = self::splitNameAndExtension($filename);
+
+        $extension = self::acceptExtension($extension, $reserved);
+        $name = self::sanitizeName($name, $extension, $reserved);
 
         return $extension === '' ? $name : sprintf('%s.%s', $name, $extension);
     }
 
+    /**
+     * Split a client-supplied filename into the name and the extension
+     *
+     * What `pathinfo()`'s `PATHINFO_FILENAME` and `PATHINFO_EXTENSION` answer, except that `/`
+     * is the only separator. `pathinfo()` treats `\` as one on Windows and not on POSIX, so the
+     * same name split two ways: `a\b.txt` was stored as `a-b.txt` here and `b.txt` there.
+     * `rewriteCharacters()` rewrites `\` to `-`, so the rule both layers share is that a
+     * backslash is a character in the name — and a rule cannot depend on which platform is
+     * applying it.
+     *
+     * Trailing slashes go first, as `basename()` drops them, so `photos/` still names `photos`.
+     *
+     * @return array<int, string> The name and the extension, either of which may be `''`
+     * @phpstan-return array{0: string, 1: string}
+     *
+     * @internal Not part of the public API
+     */
+    public static function splitNameAndExtension(string $filename): array
+    {
+        $filename = rtrim($filename, '/');
+
+        $separator = strrpos($filename, '/');
+        $basename = $separator === false ? $filename : substr($filename, $separator + 1);
+
+        /* The last dot, wherever it is: `.htaccess` is all extension and no name, which is what
+           `pathinfo()` answers and what the storage deny-list is then handed. */
+        $dot = strrpos($basename, '.');
+
+        if ($dot === false) {
+            return [$basename, ''];
+        }
+
+        return [substr($basename, 0, $dot), substr($basename, $dot + 1)];
+    }
+
     /**
      * Make a string safe to render as one line of prose
      *

src/Upload/Storage/FileSystem.php

@@ -453,8 +453,9 @@ protected function reserveDestination(string $destinationFile, FileInfoInterface
            is the destination only if the directory entry is that same file; a symlink has an
            inode of its own, so a mismatch means the name was a link.
 
-           POSIX only. Before PHP 7.4 `stat()` on Windows reports `ino` as 0 and `dev` as the
-           drive number, so this comparison degrades to same-drive and detects nothing. Windows
+           POSIX only. Before PHP 7.4 `stat()` on Windows reports `ino` as 0, so this
+           comparison has no information — the guard below skips it rather than reading no
+           information as a mismatch, which refused every upload on that platform. Windows
            symlinks need a privilege an uploading process should not hold, so the residual risk
            is small, but the symlink protections in this class are not load-bearing there. */
         $entry = $this->lstatEntry($destinationFile);
@@ -473,7 +474,17 @@ protected function reserveDestination(string $destinationFile, FileInfoInterface
             );
         }
 
-        if ($opened['dev'] !== $entry['dev'] || $opened['ino'] !== $entry['ino']) {
+        /* An inode of 0 is what Windows reports before PHP 7.4, for both stats, so the
+           comparison below has nothing to compare. Treating that as a mismatch answered
+           'Destination is a symbolic link' to every reservation, which is every upload the
+           default configuration makes — the platform could not store a file at all. Skipped
+           rather than failed: nothing has been established either way, and the symlink
+           protections here were never load-bearing on Windows, where a symlink needs a
+           privilege an uploading process should not hold. A real file has a real inode, so
+           this gives up nothing on POSIX. */
+        $identified = $opened['ino'] !== 0 && $entry['ino'] !== 0;
+
+        if ($identified && ($opened['dev'] !== $entry['dev'] || $opened['ino'] !== $entry['ino'])) {
             /* The write is already refused at this point and nothing of the victim's was
                overwritten, but `x` has created a file at the far end of the link, outside the
                upload directory. Take that back too. */
@@ -512,15 +523,29 @@ protected function lstatEntry(string $path)
      */
     private function releaseReservation(string $destinationFile, $opened): void
     {
-        $target = @readlink($destinationFile);
-
-        if ($target === false) {
+        /* `is_link()` rather than a failed `readlink()`, which is not the same question on
+           every platform: PHP's Windows `readlink()` answers a *regular file* with its own
+           canonical path instead of failing, so the placeholder took the link branch below,
+           matched nothing there and was never removed — leaving the caller's name held
+           against every later upload. The stat cache is cleared because `reserveDestination()`
+           has already lstat'd this path. */
+        clearstatcache(true, $destinationFile);
+
+        if (!is_link($destinationFile)) {
             /* Not a link, so the name is the file. `unlink()` does not follow one in any case. */
             @unlink($destinationFile);
 
             return;
         }
 
+        $target = @readlink($destinationFile);
+
+        /* A link this cannot read the target of: neither it nor whatever it points at is this
+           upload's to remove. */
+        if ($target === false) {
+            return;
+        }
+
         /* A symlink was already here and `x` created its target. Remove that file and only that
            file: the inode has to be the one this call opened, so a link re-pointed between the
            create and this check cannot make us delete a bystander. Failing that test leaves the

tests/Upload/FileInfoTest.php

@@ -277,6 +277,15 @@ public function providerSetNameSanitizing(): array
             /* Neither is a control character, so both survive */
             75 => ["caf\u{00E9}", 'txt', "caf\u{00E9}.txt"],
             76 => ["10\u{20AC}", 'txt', "10\u{20AC}.txt"],
+
+            /* A backslash is a character in the name, not a separator, on every platform.
+               `pathinfo()` splits on it under Windows and not under POSIX, so these came back
+               as `b` and `windows-win` respectively until `Filename::splitNameAndExtension()`
+               took the split over. A forward slash *is* a separator on both. */
+            82 => ['a-b', 'txt', 'a\b.txt'],
+            83 => ['windows-win', 'ini', '..\..\windows\win.ini'],
+            84 => ['b', 'txt', 'a/b.txt'],
+            85 => ['passwd', '', '../../etc/passwd'],
         ];
     }
 

tests/Upload/FileListTest.php

@@ -421,8 +421,9 @@ public function testAVouchedFileIsValidatedSanitizedAndStored(): void
 
         $this->assertTrue($list->upload());
 
-        /* The interior dot is rewritten by `FileInfo::setName()`, as on the `$_FILES` path */
-        $stored = $workingDirectory . '/holiday-photo.txt';
+        /* The interior dot is rewritten by `FileInfo::setName()`, as on the `$_FILES` path.
+           Joined with `DIRECTORY_SEPARATOR` because that is how storage composes a locator. */
+        $stored = $workingDirectory . DIRECTORY_SEPARATOR . 'holiday-photo.txt';
 
         $this->assertSame([$stored], $list->getUploadedLocators());
         $this->assertFileExists($stored);
@@ -503,7 +504,7 @@ public function testTheLifecycleCallbacksFirePerFile(): void
 
         foreach (['beforeValidate', 'afterValidate', 'beforeUpload', 'afterUpload'] as $hook) {
             $list->$hook(static function (FileInfoInterface $fileInfo) use ($hook): void {
-                echo ucfirst($hook) . ': ' . $fileInfo->getName(), PHP_EOL;
+                echo ucfirst($hook) . ': ' . $fileInfo->getName(), "\n";
             });
         }
 

tests/Upload/FileTest.php

@@ -277,19 +277,19 @@ public function testCallbacks(): void
         );
 
         $callbackBeforeValidate = function (FileInfoInterface $fileInfo) {
-            echo 'BeforeValidate: ' . $fileInfo->getName(), PHP_EOL;
+            echo 'BeforeValidate: ' . $fileInfo->getName(), "\n";
         };
 
         $callbackAfterValidate = function (FileInfoInterface $fileInfo) {
-            echo 'AfterValidate: ' . $fileInfo->getName(), PHP_EOL;
+            echo 'AfterValidate: ' . $fileInfo->getName(), "\n";
         };
 
         $callbackBeforeUpload = function (FileInfoInterface $fileInfo) {
-            echo 'BeforeUpload: ' . $fileInfo->getName(), PHP_EOL;
+            echo 'BeforeUpload: ' . $fileInfo->getName(), "\n";
         };
 
         $callbackAfterUpload = function (FileInfoInterface $fileInfo) {
-            echo 'AfterUpload: ' . $fileInfo->getName(), PHP_EOL;
+            echo 'AfterUpload: ' . $fileInfo->getName(), "\n";
         };
 
         $file = new File('multiple', $this->storage);