From 35bb020aebee628973fb9b5062cc68c189738ba5 Mon Sep 17 00:00:00 2001 From: DrGalio Date: Thu, 26 Mar 2026 22:09:33 +0000 Subject: [PATCH] fix: parse Not independently for token and etag in If header The WebDAV If header parser incorrectly applied the Not keyword to the entire (token etag) group. Per RFC 4918, Not applies only to the immediately following resource (token or etag). For example, (Not ["etag"]) should be interpreted as: NOT AND ["etag"] not as: NOT ( AND ["etag"]) This caused the litmus test locks:20 (fail_complex_cond_put) to incorrectly return 204 instead of 412 Precondition Failed. The fix splits the token and etag into separate condition entries when both are present with a Not prefix, so negation applies only to the token. Fixes sabre-io/dav#715 --- lib/DAV/Server.php | 46 ++++++++++++++----- tests/Sabre/DAV/GetIfConditionsTest.php | 61 +++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 12 deletions(-) diff --git a/lib/DAV/Server.php b/lib/DAV/Server.php index e122b2447a..807741a288 100644 --- a/lib/DAV/Server.php +++ b/lib/DAV/Server.php @@ -1587,15 +1587,43 @@ public function getIfConditions(RequestInterface $request) $conditions = []; foreach ($matches as $match) { + $negate = $match['not'] ? true : false; + $token = $match['token']; + $etag = isset($match['etag']) ? $match['etag'] : ''; + + // Build the token entries. Per RFC 4918, 'Not' applies only to + // the immediately following resource (token or etag). When both + // a token and an etag appear in the same list and 'Not' is + // present, we must split them so that the negation applies only + // to the token, not the etag. + $tokenEntries = []; + if ($negate && $token && $etag) { + // (Not [etag]) means: NOT token AND etag + $tokenEntries[] = [ + 'negate' => true, + 'token' => $token, + 'etag' => '', + ]; + $tokenEntries[] = [ + 'negate' => false, + 'token' => '', + 'etag' => $etag, + ]; + } else { + $tokenEntries[] = [ + 'negate' => $negate, + 'token' => $token, + 'etag' => $etag, + ]; + } + // If there was no uri specified in this match, and there were // already conditions parsed, we add the condition to the list of // conditions for the previous uri. if (!$match['uri'] && count($conditions)) { - $conditions[count($conditions) - 1]['tokens'][] = [ - 'negate' => $match['not'] ? true : false, - 'token' => $match['token'], - 'etag' => isset($match['etag']) ? $match['etag'] : '', - ]; + foreach ($tokenEntries as $entry) { + $conditions[count($conditions) - 1]['tokens'][] = $entry; + } } else { if (!$match['uri']) { $realUri = $request->getPath(); @@ -1605,13 +1633,7 @@ public function getIfConditions(RequestInterface $request) $conditions[] = [ 'uri' => $realUri, - 'tokens' => [ - [ - 'negate' => $match['not'] ? true : false, - 'token' => $match['token'], - 'etag' => isset($match['etag']) ? $match['etag'] : '', - ], - ], + 'tokens' => $tokenEntries, ]; } } diff --git a/tests/Sabre/DAV/GetIfConditionsTest.php b/tests/Sabre/DAV/GetIfConditionsTest.php index afee554377..13861c1f76 100644 --- a/tests/Sabre/DAV/GetIfConditionsTest.php +++ b/tests/Sabre/DAV/GetIfConditionsTest.php @@ -232,6 +232,67 @@ public function test2Etags() self::assertEquals($compare, $conditions); } + public function testNotWithTokenAndEtag() + { + // Per RFC 4918, (Not [etag]) means: NOT token AND etag + // The Not applies only to the token, not the etag. + $request = new HTTP\Request('GET', '/foo', [ + 'If' => '(Not ["c3d40b11121145ec31f5c1acc078b658"])', + ]); + + $conditions = $this->server->getIfConditions($request); + + $compare = [ + [ + 'uri' => 'foo', + 'tokens' => [ + [ + 'negate' => true, + 'token' => 'DAV:no-lock', + 'etag' => '', + ], + [ + 'negate' => false, + 'token' => '', + 'etag' => '"c3d40b11121145ec31f5c1acc078b658"', + ], + ], + ], + ]; + + self::assertEquals($compare, $conditions); + } + + public function testNotWithTokenAndEtagUrl() + { + // Same test but with a URI prefix + $request = new HTTP\Request('GET', '/foo', [ + 'If' => ' (Not ["etag1"])', + ]); + + $conditions = $this->server->getIfConditions($request); + + $compare = [ + [ + 'uri' => '', + 'tokens' => [ + [ + 'negate' => true, + 'token' => 'DAV:no-lock', + 'etag' => '', + ], + [ + 'negate' => false, + 'token' => '', + 'etag' => '"etag1"', + ], + ], + ], + ]; + + self::assertEquals($compare, $conditions); + } + public function testComplexIf() { $request = new HTTP\Request('GET', '/foo', [