diff --git a/lib/Service/SynchronizationService.php b/lib/Service/SynchronizationService.php index 2a02a549f..e430047cc 100644 --- a/lib/Service/SynchronizationService.php +++ b/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 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. * diff --git a/tests/Unit/Service/SynchronizationServiceContentDispositionTest.php b/tests/Unit/Service/SynchronizationServiceContentDispositionTest.php new file mode 100644 index 000000000..bbbba0bc8 --- /dev/null +++ b/tests/Unit/Service/SynchronizationServiceContentDispositionTest.php @@ -0,0 +1,171 @@ +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)); + } +}