Fix the thirteen findings from the repository audit

.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/CONTRIBUTING.md

@@ -4,37 +4,88 @@ Contributing
 Issue tracker
 -------------
 
-The Issue tracker serves mainly as a place to report bugs and request new features.
-Please do not abuse it as a general questions or troubleshooting location.
+The issue tracker is for bug reports and feature requests. Please do not use it for general
+questions or troubleshooting, and please report one bug or one feature per issue.
 
 Bug reports
 -------------
 
-* Bug reports **MUST** contain a small example in php/html that reproduces the bug.
-* The code example **MUST** be reproducible by copy&paste assuming composer dependencies are installed. That means:
-    * No calling unrelated funcions,
-    * an actual final HTML code has to be present, pasting a template file is not enough,
-* Failing to provide necessary information or not using the issue template will cause the issue to be closed until required information is provided.
-* Please report one feature or one bug per issue.
+A report has to be reproducible by copy and paste, assuming `composer require gravitypdf/upload`
+and nothing else. That means it includes:
+
+* **The input.** The `$_FILES` entry as a literal array, or the `FileInfoInterface` objects
+  handed to `FileList`. A filename that triggers the bug matters down to the byte — say so if
+  it carries a control character, a bidi mark or invalid UTF-8, since those do not survive a
+  paste into a browser.
+* **The configuration.** The validations added, and any of the opt-outs in
+  [Turning the defaults off](../docs/turning-the-defaults-off.md) that are in play.
+* **What happened and what you expected**, as the return value, the `getErrors()` entries or
+  the exception — not a description of them.
+* **The PHP version**, and whether `ext-mbstring` is loaded. Several behaviours differ across
+  the supported 7.3 to 8.5 range, and filename truncation differs without `mbstring`.
+
+Security issues do not belong in the tracker. Report those privately through
+[GitHub's security advisories](https://github.com/GravityPDF/Upload/security/advisories/new).
 
 Feature requests
 -------------
 
-Feature requests have to be labeled as such and have to include reasoning for the change in question.
+Label the issue as a feature request and say what problem it solves. This library is a
+security boundary before it is a convenience, so a request that relaxes a default needs to say
+what it stops applying and why that is acceptable.
 
 Pull requests
 -------------
 
-Pull requests should be always based on the default [main](https://github.com/GravityPDF/upload/tree/main)
-branch except for backports to older versions. **Note: When you first open a PR GitHub automatically references the upstream package and you'll need to change it.**
+Pull requests target the default [main](https://github.com/GravityPDF/upload/tree/main) branch,
+except for backports to older versions. **When you first open a PR, GitHub sets the base to the
+upstream `codeguy/upload` repository; you have to change it.**
+
+Before opening one, run the checks CI will run:
+
+```bash
+composer phpunit         # the test suite
+composer lint            # PHPCS, PSR-12
+composer phpstan         # PHPStan level 9, over src and tests
+composer check-syntax    # parallel-lint, all PHP files
+```
+
+Change an example in `README.md` or `docs/` and two more apply, since those snippets are read
+out of the Markdown and executed:
+
+```bash
+composer psr7-readme        # docs/psr7.md, against nyholm/psr7 and guzzlehttp/psr7
+composer base64-docs        # docs/base64-uploads.md
+composer translator-readme  # docs/translation/, against the real translation libraries
+```
+
+Change or add an error message and regenerate the catalogue, or the `i18n` workflow fails on
+the diff:
+
+```bash
+composer i18n:pot
+```
 
 Guidelines:
 
-* Use an aptly named feature branch for the Pull request.
-* Only files and lines affecting the scope of the Pull request must be affected.
-* Make small, *atomic* commits that keep the smallest possible related code changes together.
-* Code must be accompanied by a unit test testing expected behaviour whenever possible.
-* To be incorporated, the PR should contain a change in the CHANGELOG.md file describing itself
+* Use an aptly named feature branch.
+* Only the lines within the scope of the pull request should change.
+* Make small, atomic commits that keep related changes together.
+* Code must be accompanied by a test whenever one can be written. New behaviour on the storage
+  or filename path needs a test of the refusal as well as of the acceptance — a rule that
+  refuses everything passes a one-sided test.
+* User-facing changes need an entry in `CHANGELOG.md` in the same pull request. It is the
+  documented record of what breaks between major versions.
+
+Supported PHP versions
+-------------
+
+7.3 through 8.5, tested on all eight. Anything that will not parse on 7.3 cannot go in: no
+typed properties, no union types, no `mixed`, no arrow functions, no constructor promotion.
+Property and parameter types beyond what 7.3 allows live in docblocks.
+
+`ext-fileinfo` is the only required extension. `ext-mbstring` is a suggestion, so every `mb_*`
+call has to be guarded by the function that makes it — CI runs the suite with the extension,
+with `symfony/polyfill-mbstring`, and with neither.
 
-When updating a PR, do not create a new one, just `git push --force` to your former feature branch, the PR will
-update itself.
+To update a pull request, push to the same branch rather than opening a new one.

.github/workflows/composer-manifest.yml

@@ -0,0 +1,49 @@
+name: Composer Manifest
+
+on:
+  push:
+    branches:
+      - 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
+  # or the commit hash for any other events.
+  group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.head_ref || github.sha }}
+  cancel-in-progress: true
+
+# `package.yml` guards what the dist archive contains. This guards the manifest that describes
+# it: `composer.json` is what Packagist reads and what resolves the supported PHP range, and a
+# schema error there is not something any other job would notice.
+jobs:
+  run:
+    name: Valid and free of advisories
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.0.0
+
+      - name: Install PHP
+        uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2.35.5
+        with:
+          php-version: '8.5'
+
+      # --strict, because this manifest is published: a missing `name` or an unbound version
+      # constraint is an error here where it is only a warning for an application. The three
+      # manifests under tools/ are not published and pin exact versions, so they are not
+      # checked.
+      - name: Validate composer.json
+        run: composer validate --strict --no-check-lock
+
+      - name: Install Composer dependencies
+        run: composer install --no-scripts --no-interaction
+
+      # The library itself requires nothing but ext-fileinfo, so this is about require-dev:
+      # PHPCS, PHPUnit and parallel-lint run over this repository on every push.
+      - name: Check for security advisories
+        run: composer audit

.github/workflows/phpunit.yml

@@ -83,6 +83,45 @@ jobs:
           files: tmp/coverage/report-xml/php-coverage1.xml
           token: ${{ secrets.CODECOV_TOKEN }}
 
+  # Reserved device names, `:` naming an NTFS alternate data stream, trailing dots and spaces
+  # resolving away, `DIRECTORY_SEPARATOR`, `stat()` reporting `ino` as 0 — Windows behaviour
+  # this library reasons about and had never executed against. It found two bugs on its first
+  # outing, both on the default `$overwrite = false` path and neither reachable from Linux, so
+  # it is a gate.
+  #
+  # `@group posix` marks what cannot run there: `symlink()` needs a privilege an uploading
+  # process should not hold, `chmod()` is close to a no-op, and `umask()` means nothing.
+  # Excluded by group rather than skipped, so `cross-file-system` keeps the single
+  # `markTestSkipped()` its `--fail-on-skipped` guard depends on.
+  windows:
+    name: ${{ matrix.php }} on windows-latest
+    runs-on: windows-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        # The ends of the supported range. The middle versions differ from these in nothing
+        # this job exercises, and a Windows runner costs several times a Linux one.
+        php: [ '7.3', '8.5' ]
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.0.0
+
+      - name: Install PHP
+        uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2.35.5
+        with:
+          php-version: ${{ matrix.php }}
+          # Named because the Windows builds enable neither by default, where the Linux ones
+          # ship both: `ext-fileinfo` is this library's only requirement and PHPUnit declares
+          # `ext-mbstring`, so `composer install` cannot resolve without them.
+          extensions: fileinfo, mbstring
+
+      - name: Install Composer dependencies
+        run: composer install --no-scripts
+
+      - name: Run the test suite
+        run: vendor/bin/phpunit --exclude-group posix
+
   # `ext-mbstring` is `suggest`, not `require`, and every job above runs with it loaded, so an
   # unguarded `mb_*` call fatals on a supported install without any of them noticing.
   #

CHANGELOG.md

@@ -98,6 +98,13 @@ Both still work and neither raises a runtime notice.
 
 ## Bug Fixes
 
+* **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. An entry the fold leaves empty is dropped rather than registered, as `FileType` already dropped one: `getMimetype()` answers `''` for a file it cannot read, so `new Mimetype([' '])` would have accepted exactly those
+* **`Storage\FileSystem` refuses a name longer than `Filename::MAX_LENGTH`.** It applied every other rule `Filename` declares and not the length, so an over-long name from a `FileInfoInterface` of your own travelled to the exclusive create and failed on the file system's own `ENAMETOOLONG` — reported as `'Destination file could not be created'`, which is supposed to mean the directory went away and sends a caller into a retry that cannot succeed. The shipped `FileInfo` truncates, so nothing on that path changes. The rule is `Filename::exceedsMaxLength()`, beside `hasControlCharacters()` and `hasBidiControls()`, so a storage backend of your own can ask for it rather than restate it
+* `Filename::maxNameLength()` floors the name's budget at zero. `acceptExtension()` caps an extension at 32 bytes, so nothing in this library reaches it, but `finalize()` takes the extension from its caller and a longer one made the budget negative — which `mb_strcut()` reads as "cut this many bytes off the end"
+* The upload directory's trailing separator is trimmed on Windows as well as POSIX. `rtrim($directory, '/')` left `C:\uploads\` as `C:\uploads\\` internally, which `getDirectory()` then trimmed and the rest of the class did not
 * **Case folding no longer follows the host locale.** `strtolower()` follows `LC_CTYPE` before PHP 8.2, so on four of the eight supported versions a `setlocale()` call elsewhere in the application 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
 * `ext-mbstring` is detected 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 every polyfilled install was silently on the byte-wise fallback. The check is per function rather than one flag covering five: the polyfill does not ship `mb_strcut()`, so a single flag denied polyfilled installs the UTF-8 repair as well as character-boundary truncation. Its `mb_convert_encoding()` is `iconv()`-backed and warns on the input the repair exists for, so that call is silenced. CI now covers the extension, the polyfill and neither
 * `mb_detect_encoding()` is called with an explicit detect order and falls back to UTF-8, so an application's own `mb_detect_order()` can't change what this library makes of the same bytes. The fallback also covers the orders where detection returns `false` and `mb_strcut()` would raise a `ValueError`