Minor difference in oc:trashbin-original-location PROPFIND response #39337

Activity

1. phil-davis

   added

   Type:Bug

   on Oct 8, 2021
2. DeepDiver1975 commented on Jun 22, 2026

   

   DeepDiver1975

   on Jun 22, 2026

   Contributor

   More actions

   🤖 Automated issue triage by Claude Code review agent.

   Confirmed: this is a genuine (if minor) inconsistency, and the cause is locatable. The leading-slash difference comes from two different code paths storing the location column in oc_files_trash with different slash handling.

   Where the value is read back

   apps/dav/lib/TrashBin/AbstractTrashBinNode.php getOriginalLocation() just prepends the stored DB location verbatim:

   $location = $this->trashBinManager->getLocation($this->user, $pathParts['filename'], $timestamp);
   if ($location \!== '.') {
       $originalPath = $location . '/' . $originalPath;
   }

   So whatever slash form is in the location column is reflected straight into <oc:trashbin-original-location>.

   Where the divergence is introduced (apps/files_trashbin/lib/Trashbin.php)

   • The deleter's entry (Brian) is written in move():

     $path_parts = \pathinfo($ownerPath);
     $location   = $path_parts['dirname'];   // pathinfo('shared/sub')['dirname'] => "shared"  (no leading /)

   • The share owner's copy (Alice) is written via copyFilesToUser() / copyBackupForOwner():

     $targetLocation = \dirname($targetPath); // dirname('/shared/sub') => "/shared"  (leading /)

   The two paths receive the path argument in different forms (relative vs. owner-absolute) and use pathinfo()['dirname'] vs. dirname(), which differ in leading-slash handling. The stored location therefore differs (shared vs /shared), and getOriginalLocation() faithfully reproduces that difference.

   Assessment / severity

   This is real but cosmetic — both forms point to the same path, and as the reporter notes it "may or may not break a client." A robust client should treat the original-location path as relative regardless of a leading slash. The clean fix is to normalize the value at a single point so both users get an identical, canonical form — easiest at read time in getOriginalLocation() (e.g. '/' . ltrim($originalPath, '/'), consistently with or without a leading slash) so it does not depend on which write path populated the row, and avoids a DB migration. Alternatively normalize on write in both move() and copyFilesToUser()/copyBackupForOwner(), but that leaves already-stored rows inconsistent.

   No code change is being made here (the core repo is excluded from automated fixes); this comment records the confirmed cause and a suggested approach for a maintainer. Related: the acceptance tests in #39336 had to be adjusted to ignore this difference.
3. phil-davis commented on Jun 23, 2026

   

   phil-davis

   on Jun 23, 2026

   ContributorAuthor

   More actions

   It seems to me that whenever a user deletes one of their own files, the code uses move() and the original-location is stored without a leading slash. And so in the vast majority of cases, original-location actually has no leading slash.

   It is only in the case where a deleter needs to put the deleted file into both their own trashbin and the owner's trashbin that the entry in the owner's trashbin is getting a leading slash written.

   So I think that the solution with the least change to regular users of the Trashbin API is to never return a leading slash in original-location, always return a relative path.
4. DeepDiver1975 commented on Jun 23, 2026

   

   DeepDiver1975

   on Jun 23, 2026

   Contributor

   More actions

   🤖 Automated issue triage by Claude Code review agent.

   Thanks @phil-davis — agreed on the root cause and the direction. The leading slash only appears on the owner's copy written via the copyFilesToUser() / copyBackupForOwner() path (\dirname($targetPath) on an owner-absolute path), whereas the normal self-delete path in move() uses \pathinfo($ownerPath)['dirname'] and yields a relative, no-slash value. Since the no-slash form is what the vast majority of entries already carry, normalising to always relative / no leading slash is the lowest-impact fix and matches existing client expectations.

   Tracking the fix in #41649. One thing to make sure is covered there: the root case where the original location is the user's root — getOriginalLocation() short-circuits on $location === '.' (and the stored value can be . for a top-level deleted item), so the normalisation should leave that path untouched rather than stripping/prepending a slash. That was the gap flagged on the PR review; once the root case is handled the responses will be consistent for both deleter and owner.

   Leaving this open to track #41649 to merge.

   👍React with 👍1Reacted by Phil Davis
5. phil-davis

   added 2 commits that reference this issue on Jun 23, 2026

   fix: normalise trashbin original-location PROPFIND response

   ...

   4ccdb8f

   fix: normalise trashbin original-location PROPFIND response

   ...

   b7558ee
6. phil-davis

   added a commit that references this issue on Jul 8, 2026

   fix: normalise trashbin original-location PROPFIND response

   ...

   5a932b7
7. DeepDiver1975

   closed this as completedin #41649on Jul 14, 2026
8. DeepDiver1975

   added a commit that references this issue on Jul 14, 2026

   fix: normalise trashbin-original-location PROPFIND response (full-ci) (…

   ...

   72b3ad0