fix: Optimize Etag handling and response on 304 status

OpenGemeenten · Apr 5, 2023 · 69ccae6 · 69ccae6
1 parent f468e85
commit 69ccae6
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 11 deletions.
diff --git a/Classes/Middleware/CacheHeaders.php b/Classes/Middleware/CacheHeaders.php
@@ -7,6 +7,7 @@
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 use TYPO3\CMS\Core\Http\NullResponse;
+use TYPO3\CMS\Core\Http\Response;
 use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
 use function in_array;
 use function md5;
@@ -49,6 +50,8 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
         $response = $handler->handle($request);
 
         if (!($response instanceof NullResponse)
+            && isset($GLOBALS['TSFE']->config['config']['OpenGemeenten\CmsFrontend.']['sendCacheHeaders'])
+            && (bool)$GLOBALS['TSFE']->config['config']['OpenGemeenten\CmsFrontend.']['sendCacheHeaders'] === true
             && $GLOBALS['TSFE'] instanceof TypoScriptFrontendController
             && $GLOBALS['TSFE']->isStaticCacheble()
             && !$GLOBALS['TSFE']->isBackendUserLoggedIn()
@@ -57,8 +60,6 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
                 empty($GLOBALS['TSFE']->config['config']['sendCacheHeaders_onlyWhenLoginDeniedInBranch'])
                 || empty($GLOBALS['TSFE']->checkIfLoginAllowedInBranch())
             )
-            && isset($GLOBALS['TSFE']->config['config']['OpenGemeenten\CacheHeaders.']['sendCacheHeaders'])
-            && (bool)$GLOBALS['TSFE']->config['config']['OpenGemeenten\CacheHeaders.']['sendCacheHeaders'] === true
         ) {
             $configuration = $GLOBALS['TSFE']->config['config'];
 
@@ -69,22 +70,24 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
                 $response = $response->withoutHeader('pragma');
             }
 
-            $currentETag = '"' . md5($GLOBALS['TSFE']->content) . '"';
+            $currentETag = md5($GLOBALS['TSFE']->content);
 
             // Browser sends the header 'if-none-match' with the Etag
             if ($request->hasHeader('if-none-match')) {
                 $oldETags = $request->getHeader('if-none-match');
 
-                $pattern = $configuration['OpenGemeenten\CacheHeaders.']['pattern'] ?? '^("[0-9a-fA-F]*)(\b-gzip\b|)(")$';
-                $replacement = $configuration['OpenGemeenten\CacheHeaders.']['replacement'] ?? '$1$3';
+                // Use only the value of the Etag (W/"<Etag-value>-gzip")
+                // W/ means a weak Etag, -gzip is added by an Apache server when the content is served gzipped
+                $pattern = $configuration['OpenGemeenten\CmsFrontend.']['pattern'] ?? '^([W]\/|)(")([0-9a-fA-F]*)(\b-gzip\b|)(")$';
+                $replacement = $configuration['OpenGemeenten\CmsFrontend.']['replacement'] ?? '$3';
 
                 foreach ($oldETags as $key => $oldETag) {
                     $oldETags[$key] = preg_replace('/' . $pattern . '/', $replacement, $oldETag);
                 }
 
                 // If the current Etag is the same as one of the old, return 304, Not Modified
                 if (in_array($currentETag, $oldETags)) {
-                    $response = $response->withStatus(304);
+                    return new Response(null, 304);
 
                 // Else return a 200 OK
                 } else {
@@ -97,9 +100,9 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
             }
 
             // Applying “no-cache” does not mean that there is no cache at all.
-            // it simply tells the browser to validate resources on the server before use it from the cache
+            // It simply tells the browser to validate resources on the server before use it from the cache
             $response = $response->withHeader('cache-control', 'no-cache');
-            $response = $response->withHeader('ETag', $currentETag);
+            $response = $response->withHeader('ETag', '"' . $currentETag . '"');
         }
 
         return $response;

diff --git a/Configuration/RequestMiddlewares.php b/Configuration/RequestMiddlewares.php
@@ -4,10 +4,13 @@
 
 return [
     'frontend' => [
-        'opengemeenten/cache-headers' => [
+        'opengemeenten/cms-frontend/cache-headers' => [
             'target' => CacheHeaders::class,
             'after' => [
                 'typo3/cms-frontend/tsfe'
+            ],
+            'before' => [
+                'typo3/cms-frontend/prepare-tsfe-rendering'
             ]
         ]
     ]

diff --git a/Configuration/Services.yaml b/Configuration/Services.yaml
@@ -0,0 +1,8 @@
+services:
+  _defaults:
+    autowire: true
+    autoconfigure: true
+    public: false
+
+  OpenGemeenten\CacheHeaders\:
+    resource: '../Classes/*'
diff --git a/README.md b/README.md
@@ -88,9 +88,19 @@ of this setting. Related to this is the setting `config.cache_period`, which
 a page can stay in the backend, and therefor in the browser cache.
 
 **FUN FACT:** The provided `.htaccess` file, which comes with the TYPO3 core, is removing the `ETag` cache header
-again. So we are left with the cache headers `Last-Modified`, `Expires`, `Cache-Control` (with `max-age` when the page
+again. 
+```
+# ETag removal
+<IfModule mod_headers.c>
+    Header unset ETag
+</IfModule>
+```
+So we are left with the cache headers `Last-Modified`, `Expires`, `Cache-Control` (with `max-age` when the page
 can be cached) and the deprecated `Pragma`. They probably have done this since the workings of the `Etag` is not fully 
-implemented in the TYPO3 core, like the HTTP status codes of the response.
+implemented in the TYPO3 core, like the HTTP status codes of the response. 
+
+IMPORTANT NOTE: If you are going to use this extension on an Apache webserver, remove or comment the part in the 
+`.htaccess` file where the `ETag` cache header is being removed.
 
 ### What's wrong with this?