fix(dav): Do not respond version/trashbin download requests with 404 due to ChunkingV2Plugin

[DerDreschner]

Summary.

This PR fixes an issue in the ChunkingV2Plugin that was introduced with #59780 . It's hard to spot, as it's being hidden in Nextcloud 30 and newer by using a different way of registering to the beforeMethod:GET sabre event.

Nextcloud 30 and newer uses the following way: $server->on('beforeMethod:GET', $this->beforeGet(...));
Nextcloud 29 and older uses the following way: $server->on('beforeMethod:GET', [$this, 'beforeGet']);

This slight change has an effect on the order of the plugins being executed. If the ChunkingV2Plugin is being run too soon - which is the case on Nextcloud 29 and older -, the file versions and trash bin items can't be downloaded due to a 404 being returned. This isn't the case once the PR is merged, as the ChunkingV2Plugin handles that case gracefully now.

This is a summary of the situation by AI:

ChunkingV2Plugin::beforeGet eagerly resolves the request path during beforeMethod:GET to block reading intermediate chunked uploads. App-provided DAV collections (versions, trashbin) are attached to the root lazily in a beforeMethod:* closure in Server.php, while uploads is registered eagerly. When beforeGet runs before that closure, getNodeForPath() throws NotFound and aborts the whole request, turning every GET under /dav/versions/ and /dav/trashbin/ into "404 File not found: versions in 'root'". PROPFIND is unaffected, since nothing resolves the path that early for it.

This does not currently surface on master only by accident of listener ordering: beforeGet and the collection closure share the default priority, and because beforeGet is registered as a first-class callable (a Closure), the wildcard closure happens to sort before it, so the collections are already attached by the time beforeGet resolves the path. That ordering is not guaranteed -- it depends on how equal-priority listeners are tie-broken -- so the unguarded resolution is a latent fault that any reordering can expose. On stable 28 it is already broken, because the backport registered the handler as an array callable, which tie-breaks the other way and runs beforeGet first.

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)
• Labels added where applicable (ex: bug/enhancement, 3. to review, feature component)
• Milestone added for target branch/version (ex: 32.x for stable32)

AI (if applicable)

• The content of this PR was partly or fully generated using AI (ClaudeCode:claude-opus-4-8[1m])

[DerDreschner]

/backport to stable34

[DerDreschner]

/backport to stable33

[DerDreschner]

/backport to stable32

[DerDreschner]

/backport to stable31

[DerDreschner]

/backport to stable30

[DerDreschner]

/backport to stable29

[DerDreschner]

/backport to stable28

[DerDreschner]

/backport to stable27

[DerDreschner]

/backport to stable26

[DerDreschner]

Failing Cypress run is unrelated to this change

[provokateurin]

This slight change has an effect on the order of the plugins being executed

How so? I don't understand.

[DerDreschner]

How so? I don't understand.

It's PHP internal stuff about what wins when an array and a closure have the same priority.

This is an easy description of the issue made by Claude, the array_multisort is what Sabre uses to make the sorting:

Why the registration style changes the order

array_multisort($listenersPriority, SORT_NUMERIC, $listeners) sorts by priority first. But priorities usually tie (everyone defaults to 100), so array_multisort breaks the tie by comparing the second array — the callables themselves. And PHP compares a Closure differently than an [$obj, 'method'] array:

Comparison	<=> result
[$obj,'m'] vs Closure	-1 → array always sorts before
Closure vs [$obj,'m']	+1 → object always sorts after

That type rule is deterministic: an array callable is always "less than" a closure. So the interesting case is the realistic one — in NC30 only ChunkingV2Plugin switched to $this->beforeGet(...) (a Closure) while every sibling GET listener stayed an array. The demo output:

=== MIXED  ->  only ChunkingV2Plugin is a Closure, the rest are arrays ===
after sort:
   #1  ->  Versions/Core GET handler   (serves the file)
   #2  ->  SomeOtherPlugin::beforeGet
   #3  ->  ChunkingV2Plugin::beforeGet (throws MethodNotAllowed for uploads)

vs. the all-array (NC29) form, where the same listeners keep their intended registration order:

=== Nextcloud 29-  ->  [$this,'beforeGet']  (array callables) ===
after sort:
   #1  ->  ChunkingV2Plugin::beforeGet
   #2  ->  Versions/Core GET handler
   #3  ->  SomeOtherPlugin::beforeGet

The single closure gets shoved to a different slot purely because object always sorts after array. Since Sabre's sort isn't a proper stable/consistent ordering when the keys tie, the "shape" of the callable silently decides who runs first — and that reorders the GET pipeline, which is what produces the DAV-versions 404.

[DerDreschner]

Here is a demo that Claude made to illustrate the issue: https://onlinephp.io/c/3690dc

=== Nextcloud 29-  ->  [$this, 'beforeGet']  (array callables) ===
before sort:
   prio=100  type=array   
   prio=100  type=array   
   prio=100  type=array   
after sort (this is the order Sabre will actually call them):
   #1  ->  ChunkingV2Plugin::beforeGet (throws MethodNotAllowed for uploads)
   #2  ->  Versions/Core GET handler (serves the file -> the one we must reach)
   #3  ->  SomeOtherPlugin::beforeGet

=== Nextcloud 30+  ->  $this->beforeGet(...)  (Closure objects) ===
before sort:
   prio=100  type=Closure 
   prio=100  type=Closure 
   prio=100  type=Closure 
after sort (this is the order Sabre will actually call them):
   #1  ->  ChunkingV2Plugin::beforeGet (throws MethodNotAllowed for uploads)
   #2  ->  Versions/Core GET handler (serves the file -> the one we must reach)
   #3  ->  SomeOtherPlugin::beforeGet

=== MIXED  ->  only ChunkingV2Plugin is a Closure, the rest are arrays ===
before sort:
   prio=100  type=array   
   prio=100  type=Closure 
   prio=100  type=array   
after sort (this is the order Sabre will actually call them):
   #1  ->  Versions/Core GET handler (serves the file -> the one we must reach)
   #2  ->  SomeOtherPlugin::beforeGet
   #3  ->  ChunkingV2Plugin::beforeGet (throws MethodNotAllowed for uploads)

=== Why: how PHP compares a Closure vs an array callable ===
   [obj, "method"]  <=>  Closure   =>   -1  (array sorts before object)
   Closure          <=>  [obj,...]  =>   1  (object sorts after array)

   => An [$obj,'method'] callable and a $obj->method(...) Closure are NOT interchangeable
      as far as array_multisort's tie-break is concerned, so equal-priority GET
      listeners end up in a different order -> the wrong handler answers -> 404.