]> source.dussan.org Git - nextcloud-server.git/commitdiff
Move the notmodified check to middleware where it belongs 20939/head
authorRoeland Jago Douma <roeland@famdouma.nl>
Mon, 11 May 2020 14:18:01 +0000 (16:18 +0200)
committerRoeland Jago Douma <roeland@famdouma.nl>
Wed, 13 May 2020 06:11:24 +0000 (08:11 +0200)
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
lib/composer/composer/autoload_classmap.php
lib/composer/composer/autoload_static.php
lib/private/AppFramework/DependencyInjection/DIContainer.php
lib/private/AppFramework/Http.php
lib/private/AppFramework/Http/Dispatcher.php
lib/private/AppFramework/Middleware/NotModifiedMiddleware.php [new file with mode: 0644]
tests/lib/AppFramework/Http/DispatcherTest.php
tests/lib/AppFramework/Http/HttpTest.php
tests/lib/AppFramework/Middleware/NotModifiedMiddlewareTest.php [new file with mode: 0644]

index 718e27c78fa3ae4855593ccc544b94936151a838..c7b966fd595ad012bf79457ffab235781d36f0a6 100644 (file)
@@ -532,6 +532,7 @@ return array(
     'OC\\AppFramework\\Middleware\\AdditionalScriptsMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/AdditionalScriptsMiddleware.php',
     'OC\\AppFramework\\Middleware\\CompressionMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/CompressionMiddleware.php',
     'OC\\AppFramework\\Middleware\\MiddlewareDispatcher' => $baseDir . '/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php',
+    'OC\\AppFramework\\Middleware\\NotModifiedMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/NotModifiedMiddleware.php',
     'OC\\AppFramework\\Middleware\\OCSMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/OCSMiddleware.php',
     'OC\\AppFramework\\Middleware\\PublicShare\\Exceptions\\NeedAuthenticationException' => $baseDir . '/lib/private/AppFramework/Middleware/PublicShare/Exceptions/NeedAuthenticationException.php',
     'OC\\AppFramework\\Middleware\\PublicShare\\PublicShareMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/PublicShare/PublicShareMiddleware.php',
index aa0528b17612223532d98754fc0dfa1296ee1dae..8212d3dafc3d9dae675128e2a5932d6a2ec42c77 100644 (file)
@@ -561,6 +561,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\AppFramework\\Middleware\\AdditionalScriptsMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/AdditionalScriptsMiddleware.php',
         'OC\\AppFramework\\Middleware\\CompressionMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/CompressionMiddleware.php',
         'OC\\AppFramework\\Middleware\\MiddlewareDispatcher' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php',
+        'OC\\AppFramework\\Middleware\\NotModifiedMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/NotModifiedMiddleware.php',
         'OC\\AppFramework\\Middleware\\OCSMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/OCSMiddleware.php',
         'OC\\AppFramework\\Middleware\\PublicShare\\Exceptions\\NeedAuthenticationException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/PublicShare/Exceptions/NeedAuthenticationException.php',
         'OC\\AppFramework\\Middleware\\PublicShare\\PublicShareMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/PublicShare/PublicShareMiddleware.php',
index 15ae8ab6b0bf7633dcf2f7dcdc091605552fa68a..82a2780eb27c3b46ab79517a156d685fdf16e224 100644 (file)
@@ -188,6 +188,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
                                $c->query(OC\AppFramework\Middleware\CompressionMiddleware::class)
                        );
 
+                       $dispatcher->registerMiddleware($c->query(OC\AppFramework\Middleware\NotModifiedMiddleware::class));
+
                        $dispatcher->registerMiddleware(
                                $c->query(OC\AppFramework\Middleware\Security\ReloadExecutionMiddleware::class)
                        );
index 3c4a52fe37c1d81d849b5b375f50ae620d131304..88e49816eb9caca3ad74c054f19f8585c0a974a9 100644 (file)
@@ -116,24 +116,7 @@ class Http extends BaseHttp {
         * @param string $ETag the etag
         * @return string
         */
-       public function getStatusHeader($status, \DateTime $lastModified=null,
-                                                                       $ETag=null) {
-               if (!is_null($lastModified)) {
-                       $lastModified = $lastModified->format(\DateTime::RFC2822);
-               }
-
-               // if etag or lastmodified have not changed, return a not modified
-               if ((isset($this->server['HTTP_IF_NONE_MATCH'])
-                       && trim(trim($this->server['HTTP_IF_NONE_MATCH']), '"') === (string)$ETag)
-
-                       ||
-
-                       (isset($this->server['HTTP_IF_MODIFIED_SINCE'])
-                       && trim($this->server['HTTP_IF_MODIFIED_SINCE']) ===
-                               $lastModified)) {
-                       $status = self::STATUS_NOT_MODIFIED;
-               }
-
+       public function getStatusHeader($status) {
                // we have one change currently for the http 1.0 header that differs
                // from 1.1: STATUS_TEMPORARY_REDIRECT should be STATUS_FOUND
                // if this differs any more, we want to create childclasses for this
index 629905e5da861c2745174f4c785c981009d352d5..3892bb667f0cb581a85569dad1fde36d5b477104 100644 (file)
@@ -116,8 +116,7 @@ class Dispatcher {
                        $controller, $methodName, $response);
 
                // depending on the cache object the headers need to be changed
-               $out[0] = $this->protocol->getStatusHeader($response->getStatus(),
-                       $response->getLastModified(), $response->getETag());
+               $out[0] = $this->protocol->getStatusHeader($response->getStatus());
                $out[1] = array_merge($response->getHeaders());
                $out[2] = $response->getCookies();
                $out[3] = $this->middlewareDispatcher->beforeOutput(
diff --git a/lib/private/AppFramework/Middleware/NotModifiedMiddleware.php b/lib/private/AppFramework/Middleware/NotModifiedMiddleware.php
new file mode 100644 (file)
index 0000000..e34310b
--- /dev/null
@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2020, Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @author Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\AppFramework\Middleware;
+
+use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Response;
+use OCP\AppFramework\Middleware;
+use OCP\IRequest;
+
+class NotModifiedMiddleware extends Middleware {
+       /** @var IRequest */
+       private $request;
+
+       public function __construct(IRequest $request) {
+               $this->request = $request;
+       }
+
+       public function afterController($controller, $methodName, Response $response) {
+               $etagHeader = $this->request->getHeader('IF_NONE_MATCH');
+               if ($etagHeader !== '' && $response->getETag() !== null && trim($etagHeader) === '"' . $response->getETag() . '"') {
+                       $response->setStatus(Http::STATUS_NOT_MODIFIED);
+                       return $response;
+               }
+
+               $modifiedSinceHeader = $this->request->getHeader('IF_MODIFIED_SINCE');
+               if ($modifiedSinceHeader !== '' && $response->getLastModified() !== null && trim($modifiedSinceHeader) === $response->getLastModified()->format(\DateTime::RFC2822)) {
+                       $response->setStatus(Http::STATUS_NOT_MODIFIED);
+                       return $response;
+               }
+
+               return $response;
+       }
+}
index 74dbf68c28a4dbd38f2fd52c1ef626979c01f925..c4c973aec90f28d0004095aecf5f86cd8e81f5ed 100644 (file)
@@ -180,20 +180,12 @@ class DispatcherTest extends \Test\TestCase {
                $this->response->expects($this->once())
                        ->method('getStatus')
                        ->willReturn(Http::STATUS_OK);
-               $this->response->expects($this->once())
-                       ->method('getLastModified')
-                       ->willReturn($this->lastModified);
-               $this->response->expects($this->once())
-                       ->method('getETag')
-                       ->willReturn($this->etag);
                $this->response->expects($this->once())
                        ->method('getHeaders')
                        ->willReturn($responseHeaders);
                $this->http->expects($this->once())
                        ->method('getStatusHeader')
-                       ->with($this->equalTo(Http::STATUS_OK),
-                               $this->equalTo($this->lastModified),
-                               $this->equalTo($this->etag))
+                       ->with($this->equalTo(Http::STATUS_OK))
                        ->willReturn($httpHeaders);
 
                $this->middlewareDispatcher->expects($this->once())
index 009bb39260dac2730535d90073b8efb403c79571..d3d23425f7cb5ba1c5136c1d4b39a4cfc3ea4f42 100644 (file)
@@ -53,38 +53,6 @@ class HttpTest extends \Test\TestCase {
                $this->assertEquals('HTTP/1.0 200 OK', $header);
        }
 
-
-       public function testEtagMatchReturnsNotModified() {
-               $http = new Http(['HTTP_IF_NONE_MATCH' => 'hi']);
-
-               $header = $http->getStatusHeader(Http::STATUS_OK, null, 'hi');
-               $this->assertEquals('HTTP/1.1 304 Not Modified', $header);
-       }
-
-
-       public function testQuotedEtagMatchReturnsNotModified() {
-               $http = new Http(['HTTP_IF_NONE_MATCH' => '"hi"']);
-
-               $header = $http->getStatusHeader(Http::STATUS_OK, null, 'hi');
-               $this->assertEquals('HTTP/1.1 304 Not Modified', $header);
-       }
-
-
-       public function testLastModifiedMatchReturnsNotModified() {
-               $dateTime = new \DateTime(null, new \DateTimeZone('GMT'));
-               $dateTime->setTimestamp('12');
-
-               $http = new Http(
-                       [
-                               'HTTP_IF_MODIFIED_SINCE' => 'Thu, 01 Jan 1970 00:00:12 +0000']
-                       );
-
-               $header = $http->getStatusHeader(Http::STATUS_OK, $dateTime);
-               $this->assertEquals('HTTP/1.1 304 Not Modified', $header);
-       }
-
-
-
        public function testTempRedirectBecomesFoundInHttp10() {
                $http = new Http([], 'HTTP/1.0');
 
diff --git a/tests/lib/AppFramework/Middleware/NotModifiedMiddlewareTest.php b/tests/lib/AppFramework/Middleware/NotModifiedMiddlewareTest.php
new file mode 100644 (file)
index 0000000..89ae75b
--- /dev/null
@@ -0,0 +1,106 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2020, Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @author Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace Test\AppFramework\Middleware;
+
+use OC\AppFramework\Middleware\NotModifiedMiddleware;
+use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http;
+use OCP\IRequest;
+
+class NotModifiedMiddlewareTest extends \Test\TestCase {
+
+       /** @var IRequest */
+       private $request;
+       /** @var Controller */
+       private $controller;
+       /** @var NotModifiedMiddleware */
+       private $middleWare;
+
+       protected function setUp(): void {
+               parent::setUp();
+
+               $this->request = $this->createMock(IRequest::class);
+               $this->middleWare = new NotModifiedMiddleware(
+                       $this->request
+               );
+
+               $this->controller = $this->createMock(Controller::class);
+       }
+
+       public function dataModified(): array {
+               $now = new \DateTime();
+
+               return [
+                       [null, '', null, '', false],
+                       ['etag', 'etag', null, '', false],
+                       ['etag', '"wrongetag"', null, '', false],
+                       ['etag', '', null, '', false],
+                       [null, '"etag"', null, '', false],
+                       ['etag', '"etag"', null, '', true],
+
+                       [null, '', $now, $now->format(\DateTime::RFC2822), true],
+                       [null, '', $now, $now->format(\DateTime::ATOM), false],
+                       [null, '', null, $now->format(\DateTime::RFC2822), false],
+                       [null, '', $now, '', false],
+
+                       ['etag', '"etag"', $now, $now->format(\DateTime::ATOM), true],
+                       ['etag', '"etag"', $now, $now->format(\DateTime::RFC2822), true],
+               ];
+       }
+
+       /**
+        * @dataProvider dataModified
+        */
+       public function testMiddleware(?string $etag, string $etagHeader, ?\DateTime $lastModified, string $lastModifiedHeader, bool $notModifiedSet) {
+               $this->request->method('getHeader')
+                       ->willReturnCallback(function (string $name) use ($etagHeader, $lastModifiedHeader) {
+                               if ($name === 'IF_NONE_MATCH') {
+                                       return $etagHeader;
+                               }
+                               if ($name === 'IF_MODIFIED_SINCE') {
+                                       return $lastModifiedHeader;
+                               }
+                               return '';
+                       });
+
+               $response = new Http\Response();
+               if ($etag !== null) {
+                       $response->setETag($etag);
+               }
+               if ($lastModified !== null) {
+                       $response->setLastModified($lastModified);
+               }
+               $response->setStatus(Http::STATUS_OK);
+
+               $result = $this->middleWare->afterController($this->controller, 'myfunction', $response);
+
+               if ($notModifiedSet) {
+                       $this->assertSame(Http::STATUS_NOT_MODIFIED, $result->getStatus());
+               } else {
+                       $this->assertSame(Http::STATUS_OK, $result->getStatus());
+               }
+       }
+}