fix(sync): quoted-string-aware header tokenizer + bump 0.2.24-woo-2 — WOO-552

Address Barry's review comment on the WOO-553 dev-branch PR
(#1840 line 9111) by porting the same fix to the WOO stable
hotfix line:

`explode(';', $header)` also splits semicolons that live inside a
quoted filename. `attachment; filename="rapport; versie 2.pdf"`
corrupts to `rapport` instead of `rapport; versie 2.pdf`. The fix
is a small quoted-string-aware tokenizer `splitHeaderParameters()`
that tracks a `"`-open state and only splits `;` outside quotes,
per RFC 6266 §4 grammar.

Changes:
- New private helper `splitHeaderParameters(string): array` on
  `SynchronizationService`.
- `parseContentDispositionFilename()` now delegates the `;`-split
  to that helper instead of `explode(';', ...)` — behaviour on
  well-formed xxllnc headers is unchanged; corrupt scenarios that
  the naive split would drop are now preserved verbatim.
- Rename local `$eq` → `$eqPos` in the parser (matches the
  post-review dev-branch style, avoids the phpmd ShortVariable
  rule if the hotfix line ever gets those checks).
- 4 new unit tests locking behaviour:
  * `testFilenameWithSemicolonInsideQuotedValuePreservesFilename`
    — the general quoted-`;` case.
  * `testFilenameWithSemicolonAndSpaceInsideQuotedValuePreservesFilename`
    — Barry's exact example from PR #1840 (rapport; versie 2.pdf).
  * `testFilenameWithPathTraversalPayloadIsReturnedVerbatim` —
    contract: sanitization is downstream's responsibility.
  * `testFilenameWithWhitespaceAroundEqualsIsAccepted` — locks
    `filename = "…"` handling.
  Total: 14 tests (was 10), all green locally.

`appinfo/info.xml` version bumped `0.2.24-woo-1` → `0.2.24-woo-2`
so the next push cuts a fresh prerelease `v0.2.24-woo-2` that
supersedes the currently-installed `v0.2.24-woo-1` on
openwoo.commonground.nu. Deploy via `app_versions` app pin (same
mechanic Wilco used on r17).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: WilcoLouwerse

appinfo/info.xml

@@ -15,7 +15,7 @@ The OpenConnector Nextcloud app provides a ESB-framework to work together in an
 - 🆓 Map and translate API calls
 
     ]]></description>
-	<version>0.2.24-woo-1</version>
+	<version>0.2.24-woo-2</version>
 	<licence>agpl</licence>
 	<category>integration</category>
 	<author mail="info@conduction.nl" homepage="https://www.conduction.nl/">Conduction</author>

lib/Service/SynchronizationService.php

@@ -3086,12 +3086,15 @@ private function getFilenameFromHeaders(array $response, CallLog $result): ?stri
      */
     private function parseContentDispositionFilename(string $headerValue): ?string
     {
-        // Split the header into parameter segments on `;`. The first segment
-        // is the disposition-type (attachment / inline), the rest are
-        // parameters. Splitting on `;` (instead of `=`) is what the naive
-        // pre-WOO-552 code got wrong: any `=` inside a value (bv. the
-        // charset''value shape of filename*) fooled the extractor.
-        $segments = array_map('trim', explode(';', $headerValue));
+        // Split the header into parameter segments on `;` while respecting
+        // RFC 6266 §4 quoted-string grammar. Splitting on `;` (instead of
+        // `=`) is what the naive pre-WOO-552 code got wrong: any `=` inside
+        // a value (bv. the charset''value shape of filename*) fooled the
+        // extractor. And a NAIVE `explode(';', $header)` in turn corrupts
+        // filenames that legitimately contain a `;` inside quotes
+        // (bv. `filename="rapport; versie 2.pdf"`) — that's why we use
+        // the quoted-string-aware splitHeaderParameters() tokenizer.
+        $segments = $this->splitHeaderParameters($headerValue);
 
         $filenameStar = null;
         $filenamePlain = null;
@@ -3100,12 +3103,12 @@ private function parseContentDispositionFilename(string $headerValue): ?string
             // Split into name/value on the FIRST `=` only — the value side
             // may legitimately contain further `=` characters (RFC 5987
             // extended values, base64-ish payloads).
-            $eq = strpos($segment, '=');
-            if ($eq === false) {
+            $eqPos = strpos($segment, '=');
+            if ($eqPos === false) {
                 continue;
             }
-            $name = strtolower(trim(substr($segment, 0, $eq)));
-            $value = trim(substr($segment, $eq + 1));
+            $name = strtolower(trim(substr($segment, 0, $eqPos)));
+            $value = trim(substr($segment, $eqPos + 1));
 
             if ($name === 'filename*') {
                 $filenameStar = $this->decodeRfc5987ExtendedValue($value);
@@ -3120,6 +3123,47 @@ private function parseContentDispositionFilename(string $headerValue): ?string
         return $filenameStar ?? $filenamePlain;
     }
 
+    /**
+     * Split a Content-Disposition header value into parameter segments,
+     * respecting RFC 6266 §4 quoted-string grammar.
+     *
+     * A naive `explode(';', $header)` also splits semicolons that are
+     * inside a quoted filename. For example,
+     * `attachment; filename="rapport; versie 2.pdf"` would corrupt to
+     * `rapport` instead of `rapport; versie 2.pdf`. This tokenizer
+     * tracks quote-open state and only splits on `;` outside quotes.
+     * The returned segments are trimmed.
+     *
+     * @param string $headerValue The raw Content-Disposition header value.
+     * @return array<int, string> Trimmed parameter segments, starting with
+     *                            the disposition-type (attachment/inline).
+     */
+    private function splitHeaderParameters(string $headerValue): array
+    {
+        $segments = [];
+        $current = '';
+        $inQuotes = false;
+        $length = strlen($headerValue);
+        for ($i = 0; $i < $length; $i++) {
+            $char = $headerValue[$i];
+            if ($char === '"') {
+                $inQuotes = ($inQuotes === false);
+                $current .= $char;
+                continue;
+            }
+            if ($char === ';' && $inQuotes === false) {
+                $segments[] = trim($current);
+                $current = '';
+                continue;
+            }
+            $current .= $char;
+        }
+        if ($current !== '') {
+            $segments[] = trim($current);
+        }
+        return $segments;
+    }
+
     /**
      * Decode an RFC 5987 extended parameter value of shape `charset''pct-encoded`.
      *

tests/Unit/Service/SynchronizationServiceContentDispositionTest.php

@@ -129,4 +129,43 @@ public function testHeaderWithoutAnyFilenameReturnsNull(): void
         $header = 'inline';
         $this->assertNull($this->invokeParser($header));
     }
+
+    public function testFilenameWithSemicolonInsideQuotedValuePreservesFilename(): void
+    {
+        // RFC 6266 §4 quoted-string grammar: a `;` between quotes is part
+        // of the value, not a parameter separator. A naive
+        // `explode(';', $header)` corrupts this to just `foo`; the
+        // quoted-string-aware tokenizer preserves the full filename.
+        $header = 'attachment; filename="foo;bar.pdf"';
+        $this->assertSame('foo;bar.pdf', $this->invokeParser($header));
+    }
+
+    public function testFilenameWithSemicolonAndSpaceInsideQuotedValuePreservesFilename(): void
+    {
+        // Barry's concrete example on PR #1840. Locks the exact string he
+        // raised so the guarantee is explicit in the test suite, not only
+        // implied by the more abstract `foo;bar.pdf` case above.
+        $header = 'attachment; filename="rapport; versie 2.pdf"';
+        $this->assertSame('rapport; versie 2.pdf', $this->invokeParser($header));
+    }
+
+    public function testFilenameWithPathTraversalPayloadIsReturnedVerbatim(): void
+    {
+        // Contract: the parser extracts the filename as declared upstream;
+        // path-separator / `..` sanitization is the responsibility of the
+        // downstream FileService::saveFile() writer. Locking this contract
+        // guards against a future refactor silently sanitizing here (which
+        // would hide malicious input from the writer's audit surface).
+        $header = 'attachment; filename="../../etc/passwd"';
+        $this->assertSame('../../etc/passwd', $this->invokeParser($header));
+    }
+
+    public function testFilenameWithWhitespaceAroundEqualsIsAccepted(): void
+    {
+        // Well-behaved servers do not emit whitespace around `=`, but the
+        // tokenizer's `trim()` handles it gracefully. Locks the behaviour
+        // so a future refactor does not silently regress it.
+        $header = 'attachment; filename = "bestand.pdf"';
+        $this->assertSame('bestand.pdf', $this->invokeParser($header));
+    }
 }