Skip to content

Commit d9a13f9

Browse files
committed
fix(dav): scope contact photo export to local address books
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
1 parent 3b2e743 commit d9a13f9

2 files changed

Lines changed: 151 additions & 7 deletions

File tree

apps/dav/lib/CardDAV/ImageExportPlugin.php

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,27 @@ public function httpGet(RequestInterface $request, ResponseInterface $response)
8080
$addressbookpath = explode('/', $path);
8181
array_pop($addressbookpath);
8282
$addressbookpath = implode('/', $addressbookpath);
83-
/** @var AddressBook $addressbook */
8483
$addressbook = $this->server->tree->getNodeForPath($addressbookpath);
8584

8685
$response->setHeader('Cache-Control', 'private, max-age=3600, must-revalidate');
8786
$response->setHeader('Etag', $node->getETag());
8887

88+
if (!$addressbook instanceof AddressBook) {
89+
$response->setStatus(Http::STATUS_NO_CONTENT);
90+
return false;
91+
}
92+
8993
try {
9094
$file = $this->cache->get($addressbook->getResourceId(), $node->getName(), $size, $node);
91-
$response->setHeader('Content-Type', $file->getMimeType());
92-
$fileName = $node->getName() . '.' . PhotoCache::ALLOWED_CONTENT_TYPES[$file->getMimeType()];
95+
96+
$extension = pathinfo($file->getName(), PATHINFO_EXTENSION);
97+
$contentType = array_search($extension, PhotoCache::ALLOWED_CONTENT_TYPES, true);
98+
if ($contentType === false) {
99+
throw new NotFoundException('Unsupported photo extension ' . $extension);
100+
}
101+
102+
$response->setHeader('Content-Type', $contentType);
103+
$fileName = $node->getName() . '.' . $extension;
93104
$sanitized = str_replace(['/', '\\'], '-', $fileName);
94105
$fallback = str_replace('%', '', (new UnicodeString($sanitized))->ascii()->toString());
95106
$response->setHeader('Content-Disposition', HeaderUtils::makeDisposition(HeaderUtils::DISPOSITION_ATTACHMENT, $sanitized, $fallback));

apps/dav/tests/unit/CardDAV/ImageExportPluginTest.php

Lines changed: 137 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
use OCA\DAV\CardDAV\AddressBook;
1313
use OCA\DAV\CardDAV\ImageExportPlugin;
14+
use OCA\DAV\CardDAV\Integration\ExternalAddressBook;
1415
use OCA\DAV\CardDAV\PhotoCache;
1516
use OCP\AppFramework\Http;
1617
use OCP\Files\NotFoundException;
@@ -122,8 +123,8 @@ public function testCard(?int $size, bool $photo): void {
122123

123124
if ($photo) {
124125
$file = $this->createMock(ISimpleFile::class);
125-
$file->method('getMimeType')
126-
->willReturn('image/jpeg');
126+
$file->method('getName')
127+
->willReturn('photo.jpg');
127128
$file->method('getContent')
128129
->willReturn('imgdata');
129130

@@ -173,6 +174,138 @@ public function testCard(?int $size, bool $photo): void {
173174
$this->assertFalse($result);
174175
}
175176

177+
public function testAppGeneratedAddressBook(): void {
178+
$this->request->method('getQueryParameters')
179+
->willReturn(['photo' => null]);
180+
$this->request->method('getPath')
181+
->willReturn('user/book/card');
182+
183+
$card = $this->createMock(Card::class);
184+
$card->method('getETag')
185+
->willReturn('"myEtag"');
186+
$book = $this->createMock(ExternalAddressBook::class);
187+
188+
$this->tree->method('getNodeForPath')
189+
->willReturnCallback(function ($path) use ($card, $book) {
190+
if ($path === 'user/book/card') {
191+
return $card;
192+
} elseif ($path === 'user/book') {
193+
return $book;
194+
}
195+
$this->fail();
196+
});
197+
198+
$this->cache->expects($this->never())
199+
->method('get');
200+
$this->response->expects($this->once())
201+
->method('setStatus')
202+
->with(Http::STATUS_NO_CONTENT);
203+
$this->response->expects($this->never())
204+
->method('setBody');
205+
206+
$result = $this->plugin->httpGet($this->request, $this->response);
207+
$this->assertFalse($result);
208+
}
209+
210+
public function testCardWithAvifPhoto(): void {
211+
$this->request->method('getQueryParameters')
212+
->willReturn(['photo' => null]);
213+
$this->request->method('getPath')
214+
->willReturn('user/book/card');
215+
216+
$card = $this->createMock(Card::class);
217+
$card->method('getETag')
218+
->willReturn('"myEtag"');
219+
$card->method('getName')
220+
->willReturn('card');
221+
$book = $this->createMock(AddressBook::class);
222+
$book->method('getResourceId')
223+
->willReturn(1);
224+
225+
$this->tree->method('getNodeForPath')
226+
->willReturnCallback(function ($path) use ($card, $book) {
227+
if ($path === 'user/book/card') {
228+
return $card;
229+
} elseif ($path === 'user/book') {
230+
return $book;
231+
}
232+
$this->fail();
233+
});
234+
235+
$file = $this->createMock(ISimpleFile::class);
236+
$file->method('getName')
237+
->willReturn('photo.avif');
238+
$file->method('getContent')
239+
->willReturn('imgdata');
240+
241+
$this->cache->method('get')
242+
->with(1, 'card', -1, $card)
243+
->willReturn($file);
244+
245+
$setHeaderCalls = [
246+
['Cache-Control', 'private, max-age=3600, must-revalidate'],
247+
['Etag', '"myEtag"'],
248+
['Content-Type', 'image/avif'],
249+
['Content-Disposition', 'attachment; filename=card.avif'],
250+
];
251+
$this->response->expects($this->exactly(count($setHeaderCalls)))
252+
->method('setHeader')
253+
->willReturnCallback(function () use (&$setHeaderCalls): void {
254+
$expected = array_shift($setHeaderCalls);
255+
$this->assertEquals($expected, func_get_args());
256+
});
257+
258+
$this->response->expects($this->once())
259+
->method('setStatus')
260+
->with(200);
261+
262+
$result = $this->plugin->httpGet($this->request, $this->response);
263+
$this->assertFalse($result);
264+
}
265+
266+
public function testCardWithUnsupportedPhotoExtension(): void {
267+
$this->request->method('getQueryParameters')
268+
->willReturn(['photo' => null]);
269+
$this->request->method('getPath')
270+
->willReturn('user/book/card');
271+
272+
$card = $this->createMock(Card::class);
273+
$card->method('getETag')
274+
->willReturn('"myEtag"');
275+
$card->method('getName')
276+
->willReturn('card');
277+
$book = $this->createMock(AddressBook::class);
278+
$book->method('getResourceId')
279+
->willReturn(1);
280+
281+
$this->tree->method('getNodeForPath')
282+
->willReturnCallback(function ($path) use ($card, $book) {
283+
if ($path === 'user/book/card') {
284+
return $card;
285+
} elseif ($path === 'user/book') {
286+
return $book;
287+
}
288+
$this->fail();
289+
});
290+
291+
$file = $this->createMock(ISimpleFile::class);
292+
$file->method('getName')
293+
->willReturn('photo.tiff');
294+
295+
$this->cache->method('get')
296+
->with(1, 'card', -1, $card)
297+
->willReturn($file);
298+
299+
$this->response->expects($this->once())
300+
->method('setStatus')
301+
->with(Http::STATUS_NO_CONTENT);
302+
$this->response->expects($this->never())
303+
->method('setBody');
304+
305+
$result = $this->plugin->httpGet($this->request, $this->response);
306+
$this->assertFalse($result);
307+
}
308+
176309
public function testCardWithSpecialCharactersInName(): void {
177310
$this->request->method('getQueryParameters')
178311
->willReturn(['photo' => null]);
@@ -199,8 +332,8 @@ public function testCardWithSpecialCharactersInName(): void {
199332
});
200333

201334
$file = $this->createMock(ISimpleFile::class);
202-
$file->method('getMimeType')
203-
->willReturn('image/png');
335+
$file->method('getName')
336+
->willReturn('photo.png');
204337
$file->method('getContent')
205338
->willReturn('imgdata');
206339

0 commit comments

Comments
 (0)