fix(sync): parse RFC 6266 Content-Disposition (filename*) — WOO-553

lib/Service/SynchronizationService.php

@@ -9053,10 +9053,10 @@ private function getFilenameFromHeaders(array $response, ObjectEntity $result):
 		if (isset($response['headers']['Content-Disposition']) === true
 			&& str_contains($response['headers']['Content-Disposition'][0], 'filename') === true
 		) {
-			$explodedContentDisposition = explode('=', $response['headers']['Content-Disposition'][0]);
+			$filename = $this->parseContentDispositionFilename(headerValue: $response['headers']['Content-Disposition'][0]);
+		}
 
-			$filename = trim(string: $explodedContentDisposition[1], characters: '"');
-		} else {
+		if ($filename === null) {
 			// Otherwise, parse the url and content type header. The CallLog is now
 			// an OpenRegister ObjectEntity; the `request` body lives under
 			// `getObject()['request']` instead of the legacy `getRequest()` getter.
@@ -9081,6 +9081,136 @@ private function getFilenameFromHeaders(array $response, ObjectEntity $result):
 		return $filename;
 	}//end getFilenameFromHeaders()
 
+	/**
+	 * Parse a Content-Disposition header value and extract the filename per RFC 6266.
+	 *
+	 * Supports both the traditional `filename="…"` parameter and the RFC 5987
+	 * extended `filename*=charset''pct-encoded-value` form. When both are
+	 * present the extended form wins per RFC 6266 §4.3, with the plain
+	 * `filename` used as fallback when `filename*` is absent or carries an
+	 * unsupported charset.
+	 *
+	 * WOO-552: replaces the naive `explode('=', $header)` that corrupted
+	 * the filename as soon as xxllnc's Zaken API started emitting both
+	 * parameters (release 2026-08-19, temporarily rolled back, feature-
+	 * toggled re-rollout expected). Parameter-name matching is case-
+	 * insensitive; the plain `filename` parameter's surrounding quotes are
+	 * stripped.
+	 *
+	 * @param string $headerValue The raw Content-Disposition header value.
+	 * @return string|null        The extracted filename, or null when
+	 *                            neither `filename*` nor `filename` yielded
+	 *                            a value.
+	 */
+	private function parseContentDispositionFilename(string $headerValue): ?string {
+		// Split the header into parameter segments on `;`, respecting quoted
+		// strings so a `;` inside `filename="..."` is treated as part of the
+		// value (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.
+		$segments = $this->splitHeaderParameters(headerValue: $headerValue);
+
+		$filenameStar = null;
+		$filenamePlain = null;
+
+		foreach ($segments as $segment) {
+			// Split into name/value on the FIRST `=` only — the value side
+			// may legitimately contain further `=` characters (RFC 5987
+			// extended values, base64-ish payloads).
+			$eqPos = strpos($segment, '=');
+			if ($eqPos === false) {
+				continue;
+			}
+			$name = strtolower(trim(substr($segment, 0, $eqPos)));
+			$value = trim(substr($segment, $eqPos + 1));
+
+			if ($name === 'filename*') {
+				$filenameStar = $this->decodeRfc5987ExtendedValue(value: $value);
+			} elseif ($name === 'filename') {
+				// RFC 6266 allows quoted or unquoted `filename`; strip
+				// matching surrounding double quotes when present.
+				$filenamePlain = trim($value, '"');
+			}
+		}
+
+		// RFC 6266 §4.3: `filename*` wins when present and decodable.
+		return $filenameStar ?? $filenamePlain;
+	}//end parseContentDispositionFilename()
+
+	/**
+	 * Split a Content-Disposition header on `;` while preserving semicolons
+	 * that appear INSIDE a quoted-string parameter value.
+	 *
+	 * RFC 6266 §4 uses the HTTP quoted-string grammar for `filename="..."`,
+	 * so a `;` between quotes is part of the value, not a parameter
+	 * separator. A naive `explode(';', $header)` corrupts values like
+	 * `filename="foo;bar.pdf"` to just `foo`.
+	 *
+	 * @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;
+	}//end splitHeaderParameters()
+
+	/**
+	 * Decode an RFC 5987 extended parameter value of shape
+	 * `charset''pct-encoded`.
+	 *
+	 * Only UTF-8 is supported — any other charset (bv. ISO-8859-1)
+	 * triggers a fallback by returning null, causing
+	 * {@see parseContentDispositionFilename()} to use the plain `filename`
+	 * parameter instead. Malformed values also return null.
+	 *
+	 * @param string $value The raw extended value,
+	 *                      e.g. `UTF-8''na%C3%AFef.pdf`.
+	 * @return string|null  The decoded UTF-8 string, or null when
+	 *                      unsupported.
+	 */
+	private function decodeRfc5987ExtendedValue(string $value): ?string {
+		// RFC 5987 shape: charset ' language ' value-chars
+		$parts = explode("'", $value, 3);
+		if (count($parts) !== 3) {
+			return null;
+		}
+		[$charset, $language, $encoded] = $parts;
+		unset($language); // Language tag is accepted but not used.
+
+		if (strcasecmp($charset, 'UTF-8') !== 0) {
+			$this->logger->info(
+				'Ignoring Content-Disposition filename* with unsupported charset; falling back to plain filename',
+				['charset' => $charset]
+			);
+			return null;
+		}
+
+		// Decode via rawurldecode() — implements the RFC 3986 §2.1 pct-decode.
+		return rawurldecode($encoded);
+	}//end decodeRfc5987ExtendedValue()
+
 	/**
 	 * Extracts an endpoint from the given data and optionally retrieves a filename and tags.
 	 *

tests/Unit/Service/SynchronizationServiceContentDispositionTest.php

@@ -0,0 +1,171 @@
+<?php
+
+namespace OCA\Integriq\Tests\Unit\Service;
+
+use OCA\Integriq\Service\SynchronizationService;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
+use ReflectionClass;
+
+/**
+ * Unit tests for the RFC 6266 Content-Disposition parser added in WOO-552.
+ *
+ * The parser lives as a private method on {@see SynchronizationService}
+ * because it is only ever consumed by that one caller. We reach into it
+ * via reflection so we can validate the RFC 6266 header shapes without
+ * standing up the full service graph (mappers, session, container, etc.).
+ *
+ * Scenarios covered (DoD in WOO-552):
+ *   a) `filename` only              — legacy header, must keep working.
+ *   b) `filename*` only, UTF-8      — must decode pct-encoded value.
+ *   c) both present                 — `filename*` wins per RFC 6266 §4.3.
+ *   d) `filename*` Unicode pct-decode — diakriet round-trips correctly.
+ *   e) `filename*` unsupported charset — falls back to plain `filename`.
+ *
+ * @package OCA\Integriq\Tests\Unit\Service
+ */
+class SynchronizationServiceContentDispositionTest extends TestCase
+{
+    /**
+     * Invoke a private method on SynchronizationService without building
+     * the full service graph. We only exercise pure string parsing here,
+     * so ReflectionClass::newInstanceWithoutConstructor() is sufficient —
+     * the parser does not touch any constructor-injected dependency other
+     * than the optional logger, which we inject via reflection for the
+     * charset-fallback path.
+     */
+    private function invokeParser(string $headerValue): ?string
+    {
+        $reflection = new ReflectionClass(SynchronizationService::class);
+        $service = $reflection->newInstanceWithoutConstructor();
+
+        // Populate the readonly logger property so the unsupported-charset
+        // fallback in decodeRfc5987ExtendedValue() can call ->info(...).
+        $loggerProperty = $reflection->getProperty('logger');
+        $loggerProperty->setAccessible(true);
+        $loggerProperty->setValue($service, $this->createMock(LoggerInterface::class));
+
+        $method = $reflection->getMethod('parseContentDispositionFilename');
+        $method->setAccessible(true);
+
+        return $method->invoke($service, $headerValue);
+    }
+
+    public function testFilenameOnlyAsciiRoundTrips(): void
+    {
+        $header = 'attachment; filename="bestand.pdf"';
+        $this->assertSame('bestand.pdf', $this->invokeParser($header));
+    }
+
+    public function testFilenameStarOnlyUtf8IsDecoded(): void
+    {
+        // RFC 5987 extended value form: charset '' language '' pct-encoded.
+        $header = "attachment; filename*=UTF-8''bestand.pdf";
+        $this->assertSame('bestand.pdf', $this->invokeParser($header));
+    }
+
+    public function testFilenameStarWinsOverPlainFilenameWhenBothPresent(): void
+    {
+        // This is the exact xxllnc post-2026-08-19 shape that broke the
+        // pre-WOO-552 naive `explode('=', $header)` extractor.
+        $header = 'attachment; filename="fallback.pdf"; filename*=UTF-8\'\'preferred.pdf';
+        $this->assertSame('preferred.pdf', $this->invokeParser($header));
+    }
+
+    public function testFilenameStarWithUnicodePctEncodingDecodesToUtf8(): void
+    {
+        // "na\xC3\xAFef.pdf" pct-encoded — the diakriet-carrying case that
+        // motivated xxllnc to adopt filename* in the first place.
+        $header = "attachment; filename=\"naief.pdf\"; filename*=UTF-8''na%C3%AFef.pdf";
+        $this->assertSame('naïef.pdf', $this->invokeParser($header));
+    }
+
+    public function testFilenameStarWithUnsupportedCharsetFallsBackToFilename(): void
+    {
+        // Charsets other than UTF-8 (e.g. legacy ISO-8859-1) are not
+        // decoded; RFC 5987 mandates support only for UTF-8, so we fall
+        // back to the plain `filename` parameter which is guaranteed ASCII.
+        $header = "attachment; filename=\"safe.pdf\"; filename*=ISO-8859-1''na%EFef.pdf";
+        $this->assertSame('safe.pdf', $this->invokeParser($header));
+    }
+
+    public function testFilenameStarWithUnsupportedCharsetAndNoFilenameReturnsNull(): void
+    {
+        // Defensive: if the extended value is unusable AND there is no
+        // plain `filename`, the parser must return null so the caller
+        // knows to fall back to its URL/MIME-based path.
+        $header = "attachment; filename*=ISO-8859-1''na%EFef.pdf";
+        $this->assertNull($this->invokeParser($header));
+    }
+
+    public function testFilenameParameterNameIsCaseInsensitive(): void
+    {
+        // RFC 6266 explicitly allows case-insensitive parameter names.
+        $header = 'attachment; FileName="bestand.pdf"';
+        $this->assertSame('bestand.pdf', $this->invokeParser($header));
+    }
+
+    public function testFilenameStarParameterNameIsCaseInsensitive(): void
+    {
+        $header = "attachment; FILENAME*=UTF-8''bestand.pdf";
+        $this->assertSame('bestand.pdf', $this->invokeParser($header));
+    }
+
+    public function testUnquotedFilenameIsAccepted(): void
+    {
+        // Token form (no surrounding quotes) is permitted by RFC 6266
+        // when the filename contains no separators — real-world servers
+        // do emit this shape.
+        $header = 'attachment; filename=bestand.pdf';
+        $this->assertSame('bestand.pdf', $this->invokeParser($header));
+    }
+
+    public function testHeaderWithoutAnyFilenameReturnsNull(): void
+    {
+        // `inline` disposition with no filename parameter — the parser is
+        // only called when the caller has already seen the substring
+        // "filename" in the header, but even so we assert the null path
+        // to guard against future refactors of the calling contract.
+        $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));
+    }
+}