Skip to content

Commit b872208

Browse files
joshtrichardsjoshtrichards
authored
refactor(files_versions): tidy up UA download header logic and modernize class structure
Signed-off-by: Josh <[email protected]>
1 parent a366ec3 commit b872208

File tree

1 file changed

+96
-35
lines changed

1 file changed

+96
-35
lines changed

‎apps/files_versions/lib/Sabre/Plugin.php‎

Lines changed: 96 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
declare(strict_types=1);
44

55
/**
6-
* SPDX-FileCopyrightText: 2019 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-FileCopyrightText: 2019-2025 Nextcloud GmbH and Nextcloud contributors
77
* SPDX-License-Identifier: AGPL-3.0-or-later
88
*/
99
namespace OCA\Files_Versions\Sabre;
@@ -21,35 +21,48 @@
2121
use Sabre\HTTP\RequestInterface;
2222
use Sabre\HTTP\ResponseInterface;
2323

24+
/**
25+
* SabreDAV plugin for managing versioned file access and metadata.
26+
*
27+
* Handles WebDAV requests related to file versions, including download headers,
28+
* version metadata properties, and compatibility for various clients and browsers.
29+
*/
2430
class Plugin extends ServerPlugin {
25-
private Server $server;
26-
2731
public const LABEL = 'label';
28-
2932
public const AUTHOR = 'author';
30-
3133
public const VERSION_LABEL = '{http://nextcloud.org/ns}version-label';
32-
33-
public const VERSION_AUTHOR = '{http://nextcloud.org/ns}version-author'; // dav property for author
34+
public const VERSION_AUTHOR = '{http://nextcloud.org/ns}version-author';
35+
private const LEGACY_FILENAME_HEADER_USER_AGENTS = [ // Quirky clients
36+
Request::USER_AGENT_IE,
37+
Request::USER_AGENT_ANDROID_MOBILE_CHROME,
38+
Request::USER_AGENT_FREEBOX,
39+
];
40+
private Server $server;
3441

3542
public function __construct(
36-
private IRequest $request,
37-
private IPreview $previewManager,
43+
private readonly IRequest $request,
44+
private readonly IPreview $previewManager,
3845
) {
39-
$this->request = $request;
4046
}
4147

42-
public function initialize(Server $server) {
48+
public function initialize(Server $server): void {
4349
$this->server = $server;
4450

4551
$server->on('afterMethod:GET', [$this, 'afterGet']);
4652
$server->on('propFind', [$this, 'propFind']);
4753
$server->on('propPatch', [$this, 'propPatch']);
4854
}
4955

50-
public function afterGet(RequestInterface $request, ResponseInterface $response) {
56+
/**
57+
* Handles the GET request for versioned files.
58+
*
59+
* Validates the request path, checks node type, and sets appropriate download headers
60+
* to ensure compatibility across different clients and browsers.
61+
*/
62+
public function afterGet(RequestInterface $request, ResponseInterface $response): void {
5163
$path = $request->getPath();
52-
if (!str_starts_with($path, 'versions')) {
64+
65+
if (!str_starts_with($path, 'versions/')) {
5366
return;
5467
}
5568

@@ -64,36 +77,84 @@ public function afterGet(RequestInterface $request, ResponseInterface $response)
6477
}
6578

6679
$filename = $node->getVersion()->getSourceFileName();
67-
68-
if ($this->request->isUserAgent(
69-
[
70-
Request::USER_AGENT_IE,
71-
Request::USER_AGENT_ANDROID_MOBILE_CHROME,
72-
Request::USER_AGENT_FREEBOX,
73-
])) {
74-
$response->addHeader('Content-Disposition', 'attachment; filename="' . rawurlencode($filename) . '"');
75-
} else {
76-
$response->addHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . rawurlencode($filename)
77-
. '; filename="' . rawurlencode($filename) . '"');
78-
}
80+
$this->addContentDispositionHeader($response, $filename);
7981
}
8082

83+
/**
84+
* WebDAV PROPFIND event handler for versioned files.
85+
*
86+
* Provides read-only access to version-related information, upon request, if the
87+
* current node is a VersionFile.
88+
*/
8189
public function propFind(PropFind $propFind, INode $node): void {
82-
if ($node instanceof VersionFile) {
83-
$propFind->handle(self::VERSION_LABEL, fn () => $node->getMetadataValue(self::LABEL));
84-
$propFind->handle(self::VERSION_AUTHOR, fn () => $node->getMetadataValue(self::AUTHOR));
85-
$propFind->handle(
86-
FilesPlugin::HAS_PREVIEW_PROPERTYNAME,
87-
fn (): string => $this->previewManager->isMimeSupported($node->getContentType()) ? 'true' : 'false',
88-
);
90+
if (!($node instanceof VersionFile)) {
91+
return;
8992
}
93+
94+
$propFind->handle(
95+
self::VERSION_LABEL,
96+
fn () => $node->getMetadataValue(self::LABEL)
97+
);
98+
$propFind->handle(
99+
self::VERSION_AUTHOR,
100+
fn () => $node->getMetadataValue(self::AUTHOR)
101+
);
102+
$propFind->handle(
103+
FilesPlugin::HAS_PREVIEW_PROPERTYNAME,
104+
fn (): string => $this->previewManager->isMimeSupported($node->getContentType()) ? 'true' : 'false',
105+
);
90106
}
91107

92-
public function propPatch($path, PropPatch $propPatch): void {
108+
/**
109+
* WebDAV PROPPATCH event handler for versioned files.
110+
*
111+
* Updates version related properties (such as the label) on VersionFile nodes.
112+
*/
113+
public function propPatch(string $path, PropPatch $propPatch): void {
93114
$node = $this->server->tree->getNodeForPath($path);
94115

95-
if ($node instanceof VersionFile) {
96-
$propPatch->handle(self::VERSION_LABEL, fn (string $label) => $node->setMetadataValue(self::LABEL, $label));
116+
if (!($node instanceof VersionFile)) {
117+
return;
97118
}
119+
120+
$propPatch->handle(
121+
self::VERSION_LABEL,
122+
fn (string $label) => $node->setMetadataValue(self::LABEL, $label)
123+
);
124+
}
125+
126+
/**
127+
* Add a Content-Disposition header in a way that attempts to be broadly compatible with various user agents.
128+
*
129+
* Sends both 'filename' (legacy quoted) and 'filename*' (UTF-8 encoded) per RFC 6266,
130+
* except for known quirky agents known to mishandle the `filename*`, which only get `filename`.
131+
*
132+
* Note: The quoting/escaping should strictly follow RFC 6266 and RFC 5987.
133+
*
134+
* TODO: Currently uses rawurlencode($filename) for both parameters, which is wrong: filename= should be plain
135+
* quoted ASCII (with necessary escaping), while filename* should be UTF-8 percent-encoded.
136+
* TODO: This logic appears elsewhere (sometimes with different quoting/filename handling) and could benefit
137+
* from a shared utility function. See Symfony example:
138+
* - https://github.com/symfony/symfony/blob/175775eb21508becf7e7a16d65959488e522c39a/src/Symfony/Component/HttpFoundation/BinaryFileResponse.php#L146-L155
139+
* - https://github.com/symfony/symfony/blob/175775eb21508becf7e7a16d65959488e522c39a/src/Symfony/Component/HttpFoundation/HeaderUtils.php#L152-L165
140+
*
141+
* @param ResponseInterface $response HTTP response object to add the header to
142+
* @param string $filename Download filename
143+
*/
144+
private function addContentDispositionHeader(ResponseInterface $response, string $filename): void {
145+
if (!$this->request->isUserAgent(self::LEGACY_FILENAME_HEADER_USER_AGENTS)) {
146+
// Modern clients will use 'filename*'; older clients will refer to `filename`.
147+
// The older fallback must be listed first per RFC.
148+
// In theory this is all we actually need to handle both client types.
149+
$response->addHeader(
150+
'Content-Disposition',
151+
'attachment; filename="' . rawurlencode($filename) . '"; filename*=UTF-8\'\'' . rawurlencode($filename)
152+
);
153+
} else {
154+
// Quirky clients that choke on `filename*`: only send `filename=`
155+
$response->addHeader(
156+
'Content-Disposition',
157+
'attachment; filename="' . rawurlencode($filename) . '"');
158+
}
98159
}
99160
}

0 commit comments

Comments
 (0)