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

[phil-davis]

Steps to reproduce

1. have users Alice and Brian

2. Alice creates a folder shared with sub-folder sub and in that is file shared_file.txt

3. Alice shares folder shared with Brian

4. Brian deletes shared/sub

5. shared/sub/shared_file.txt appears in both the trashbin of Alice, and of Brian - good and expected

6. Alice queries her trashbin:
   REQUEST: PROPFIND http://172.17.0.1:8080/remote.php/dav/trash-bin/Alice

7. Brian queries his trashbin:
   REQUEST: PROPFIND http://172.17.0.1:8080/remote.php/dav/trash-bin/Brian

Expected behaviour

They both see the file shared_file.txt with the same value in oc:trashbin-original-location

Actual behaviour

Alice sees:
<oc:trashbin-original-location>/shared/sub/shared_file.txt</oc:trashbin-original-location>

Brian sees:
<oc:trashbin-original-location>shared/sub/shared_file.txt</oc:trashbin-original-location>

Alice has a leading / and Brian does not.

That may or may not break a client that wants to restore the file to its original location. But even if the clients are working OK, the difference is a bit odd.

I had to adjust the automated tests in #39336 to ignore this little difference, in order to get the overall tests to pass.

It might be better if the responses are consistent.

Server configuration

Current oC10 core master.

[DeepDiver1975]

🤖 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.

[phil-davis]

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.

[DeepDiver1975]

🤖 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.