Skip to content

PROPFIND on /albums/ returns HTTP 404 when an album contains a trashed file #3513

@karolyi

Description

@karolyi

This is a bug that I actually faced and used Claude Code to fix my local DB and generate this issue report. Take it or leave it.

Summary

Listing albums via WebDAV (PROPFIND /remote.php/dav/photos/<user>/albums/) returns HTTP 404 whenever any of the user's albums contains a reference to a file that has been moved to the trash. The Photos UI consequently shows "An error occurred" on the Albums view and no albums are displayed.

Steps to reproduce

  1. Create an album and add a photo to it.
  2. Move that photo to the Nextcloud trash (soft-delete — do not permanently delete it).
  3. Navigate to the Albums view in the Photos app.
  4. The Albums view fails to load: the UI shows "An error occurred".

In the browser network tab, PROPFIND /remote.php/dav/photos/<user>/albums/ returns HTTP 404.

Expected behaviour

Album listing succeeds. Albums that contain trashed photos skip those photos gracefully (the nc:dateRange property is computed from remaining accessible files, or returns null for an empty album).

Actual behaviour

Any album containing a reference to a trashed file causes the entire PROPFIND /albums/ request to return HTTP 404, making it impossible to list albums at all.

Root cause

The bug is a caught-exception type mismatch in lib/Sabre/Album/AlbumRootBase.php.

Call chain during PROPFIND /albums/ Depth:1

When the client requests albums it includes nc:dateRange in its PROPFIND props (see src/store/albums.ts#L30-L35). For each AlbumRoot node, Sabre evaluates the property handler registered in PropFindPlugin.php:

// https://github.com/nextcloud/photos/blob/master/lib/Sabre/PropFindPlugin.php#L146
$propFind->handle(self::DATE_RANGE_PROPERTYNAME, fn () => json_encode($node->getDateRange()));

AlbumRootBase::getDateRange() iterates over the album's photos and calls $child->getFileInfo() to get each photo's mtime:

// https://github.com/nextcloud/photos/blob/master/lib/Sabre/Album/AlbumRootBase.php#L156-L161
foreach ($this->getChildren() as $child) {
    try {
        $childCreationDate = $child->getFileInfo()->getMtime();
    } catch (NotFoundException $e) {   // ← catches OCP\Files\NotFoundException
        continue;
    }

AlbumPhoto::getFileInfo() delegates to getNode():

// https://github.com/nextcloud/photos/blob/master/lib/Sabre/Album/AlbumPhoto.php#L40-L50
private function getNode(): Node {
    $nodes = $this->rootFolder
        ->getUserFolder($this->albumFile->getOwner() ?: $this->album->getUserId())
        ->getById($this->file->getFileId());
    $node = current($nodes);
    if ($node) {
        return $node;
    } else {
        throw new NotFound('Photo not found for user');  // ← Sabre\DAV\Exception\NotFound
    }
}

Why trashed files are not found

getUserFolder() returns only the user's files/ subtree. Files in the trash reside under files_trashbin/ and are not returned by getUserFolder()->getById(), so current($nodes) is false and getNode() throws Sabre\DAV\Exception\NotFound.

Why the exception is not caught

getDateRange() catches OCP\Files\NotFoundException. AlbumPhoto::getNode() throws Sabre\DAV\Exception\NotFound. These are two unrelated exception classes. The Sabre\DAV\Exception\NotFound is not caught, propagates through the lazy property handler, through Sabre's multi-status generation, and Sabre converts it to an HTTP 404 response for the entire PROPFIND.

Both types are already imported in AlbumRootBase.php (line 18 and line 23), so only the catch clause needs updating.

Same mismatch in PropFindPlugin.php

An identical pattern exists in PropFindPlugin::propFind() lines 83–86 (the outer try-catch that guards per-photo propFind handling). This does not affect the /albums/ listing directly, but causes the same HTTP 404 behaviour when listing photos inside a specific album if any photo in that album is inaccessible.

// https://github.com/nextcloud/photos/blob/master/lib/Sabre/PropFindPlugin.php#L83-L86
try {
    $fileInfo = $node->getFileInfo();
} catch (NotFoundException) {   // ← catches OCP\Files\NotFoundException only
    return;
}

Proposed fix

lib/Sabre/Album/AlbumRootBase.php#L159 — extend the catch in getDateRange() (both types already imported):

} catch (NotFoundException | NotFound $e) {
    continue;
}

lib/Sabre/PropFindPlugin.php#L85 — extend the outer try-catch (add Sabre\DAV\Exception\NotFound to imports and catch):

} catch (NotFoundException | SabreNotFound) {
    return;
}

Secondary issue: trashed files are not removed from albums

AlbumsManagementEventListener handles NodeDeletedEvent to clean up photos_albums_files. However, moving a file to trash leaves a dangling row in photos_albums_files pointing to the files_trashbin/ path. This suggests that the trash move either does not fire NodeDeletedEvent in a way the listener catches, or the listener has a gap for this case.

The dangling reference can be confirmed and cleaned up with:

-- Diagnose
SELECT paf.file_id, fc.path
FROM oc_photos_albums_files paf
JOIN oc_filecache fc ON paf.file_id = fc.fileid
WHERE fc.path NOT LIKE 'files/%';

-- Fix (removes all album references to files outside the user's files/ tree)
DELETE FROM oc_photos_albums_files
WHERE file_id IN (
    SELECT fileid FROM oc_filecache WHERE path NOT LIKE 'files/%'
);

Environment

  • App: nextcloud/photos
  • Nextcloud: 34

Metadata

Metadata

Assignees

No one assigned

    Labels

    0. Needs triagePending approval or rejection. This issue is pending approval.bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions