Skip to content

fix(#2015): store correct path of symlinks - #2025

Open
h-2 wants to merge 1 commit into
owntone:masterfrom
h-2:fix/2015
Open

fix(#2015): store correct path of symlinks#2025
h-2 wants to merge 1 commit into
owntone:masterfrom
h-2:fix/2015

Conversation

@h-2

@h-2 h-2 commented Jul 29, 2026

Copy link
Copy Markdown

Issue #2015 — "Unknown path" when adding a folder containing symlinks

#2015

Analysed and fixed 2026-07-29 on FreeBSD 15.1 against master (29.3+). The
original report was OwnTone 29.0 on FreeBSD 14.3, so this is neither version-
nor platform-specific.

The bug

When a library directory contains a symlink to a file, the scanner writes
a files row that contradicts itself:

  • directory_id points at the directory that was scanned — where the symlink
    lives.
  • path / virtual_path hold the dereferenced target path.

Those two are then used by different lookups, which consequently disagree:

  • add matches on virtual_pathmpd_queue_add() (src/mpd.c:1581)
    builds f.virtual_path LIKE '/<path>%'.
  • lsinfo resolves via directory_id.

So the very same file is visible when browsing a directory but not
addressable
by that directory's path. add matches zero rows, falls through
to library_queue_item_add() (src/mpd.c:1628), which only understands
library: and http(s) URIs, and returns LIBRARY_PATH_INVALID — surfacing as
unknown path.

This also explains the confusing client-dependent behaviour in the report:
Cantata works because it issues lsinfo first and then adds each file by the
resolved path it got back; ncmpcpp and MALP pass the directory path straight
to add and fail.

Reproduction

mkdir -p "/tmp/lib/files" "/tmp/lib/links"
cp song.mp3 "/tmp/lib/files/"
ln -s "../files/song.mp3" "/tmp/lib/links/song.mp3"
# owntone.conf:  directories = { "/tmp/lib/links/" }
add "file:/tmp/lib/links"     -> ACK ... unknown path      # the bug
lsinfo "file:/tmp/lib/links"  -> returns song.mp3          # ...yet it is there

Resulting DB state — note there is no directories row for files/ at all:

directories:  10 | /file:/tmp/lib/links
files:         1 | directory_id=10 | path=/tmp/lib/files/song.mp3

Note this is not limited to targets outside the library. A symlink whose
target lies inside the library is affected identically (add on the symlink's
directory fails there too); it is merely less visible, because the file remains
reachable via its real directory.

The fix

One line: register the file at the path it was found at, rather than at the
dereferenced path.

--- a/src/library/filescanner.c
+++ b/src/library/filescanner.c
@@ -911,7 +911,14 @@ process_directory(char *path, int parent_id, int flags)
       else if (!(flags & F_SCAN_FAST))
 	{
 	  if (S_ISREG(sb.st_mode) || S_ISFIFO(sb.st_mode))
-	    process_file(resolved_path, &sb, file_type, scan_type, flags, dir_id);
+	    /* Use the path we found the file at, not the dereferenced path. For
+	     * regular files the two are identical, but for symlinks it means the
+	     * file is registered where the user put it, ie below the directory
+	     * we are scanning. Otherwise the file's directory_id and its path
+	     * would disagree, and lookups by path (eg mpd's "add") would not
+	     * find it, even though browsing the directory would show it.
+	     */
+	    process_file(entry, &sb, file_type, scan_type, flags, dir_id);
 	  else
 	    DPRINTF(E_LOG, L_SCAN, "Skipping %s, not a directory, symlink, pipe nor regular file\n", entry);
 	}

Symlinks are still dereferenced for stat() in read_attributes(), so size,
mtime and change detection are unchanged. Only the identity stored in the
database changes, and only for symlinks — for a regular file entry and
resolved_path are the same string (read_attributes() does
strcpy(resolved_path, path), src/library/filescanner.c:805).

After the fix directory_id and virtual_path agree, so path-based and
directory-based lookups return the same thing. Because it corrects the stored
data rather than one query, it fixes every consumer at once — MPD, DAAP and
the JSON API — instead of only the code path that happened to be reported.

What is not affected

This is worth stating plainly, because the change sounds broader than it is:

  • Libraries without symlinks — completely unaffected. For regular files
    entry and resolved_path are identical strings, so the stored path is
    byte-for-byte what it was before.
  • follow_symlinks = false — completely unaffected. That check
    (src/library/filescanner.c:901) continues before the modified code is
    reached, so symlinks never get there.
  • Symlinked directories — completely unaffected. The directory branch
    (push_dir(&dirstack, resolved_path, dir_id)) is deliberately left alone, so
    a symlinked directory is still dereferenced and its contents are still stored
    under the target path exactly as before. This is the usage the documentation
    recommends, and it keeps the tree walk operating on real paths.

The only setups that see any change at all are those that symlink individual
files
.

What changes

For symlinked files, add on the containing directory starts working. Beyond
that:

  • Symlink target outside the library: the entry stays a single entry; it is
    simply now registered where the symlink is instead of at a path outside the
    library. No duplicate arises, because a target outside the library is never
    scanned in its own right.
  • Symlink target inside the library: the file now appears twice — once
    at its real path, once at the symlink's path — where it previously collapsed
    into one row. Insert-vs-update is keyed on the exact path string
    (db_file_id_bypath(), src/library/filescanner.c:591); there is no
    inode-based de-duplication, so two distinct paths yield two rows. Both are
    fully functional, and both keep their artwork (see below).

Artwork

One caveat, affecting only users who symlink individual files from
outside the library — precisely the group whose add is broken today.

Both file-based artwork sources key off files.path:
source_item_own_get() (src/artwork.c:1611) looks for <trackname>.{jpg,png}
beside the stored path, and the album-level directory source derives its
directory by slicing f.path (Q_GROUP_DIRS, src/db.c:2323). Once the
stored path is the symlink's location, external cover files sitting next to
the target are no longer found.

Measured, with properly tagged files:

setup before after
target inside library, cover.png next to the real file artwork artwork (both entries)
target outside library, cover.png next to the real file artwork no artwork
target outside library, embedded artwork artwork artwork

So:

  • Embedded artwork is unaffected — it is read through the stored path, and
    opening a symlink transparently opens the target (verified: "Artwork for
    'Other Song' found in source 'embedded'"
    ).
  • In-library symlinks keep their artwork, including the new duplicate
    entry: the album-level source enumerates the directories of all files in
    the album, which still includes the real file's directory.
  • Only out-of-library symlinks with external cover files lose artwork.
    Those users can symlink the cover alongside the track, or rely on embedded
    art.

That is a real trade-off and should not be hidden — but it applies solely to a
configuration that is broken today anyway, and it trades a hard failure
(tracks that cannot be queued at all) for a cosmetic one.

Testing performed

  • Reproduced the reported failure over the raw MPD protocol and through
    ncmpcpp's browser; the server log matches the original report line for line.
  • Verified after the fix: add succeeds for the symlink directory, with and
    without trailing slash, for ancestors, for file: root, and for the
    individual file; each adds exactly one queue item.
  • Verified every resulting files row has directory_id consistent with its
    path.
  • Verified a library without symlinks is bit-for-bit unchanged, and that
    symlinked directories still behave exactly as before.
  • Artwork: A/B tested pristine vs. patched on identical data for all three
    rows of the table above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant