Merge pull request #2058 from scireum/ili/OX-11230-url-caching

Ensure better caching of redirected Blob URLs
scireum · Dec 6, 2024 · 5424099 · 5424099
2 parents 6ed9e8a + 0d29f2f
commit 5424099
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 25 deletions.
diff --git a/pom.xml b/pom.xml
@@ -18,7 +18,7 @@
 
     <properties>
         <sirius.kernel>dev-44.0.0</sirius.kernel>
-        <sirius.web>dev-86.0.0</sirius.web>
+        <sirius.web>dev-86.1.0</sirius.web>
         <sirius.db>dev-59.0.0</sirius.db>
     </properties>
 

diff --git a/src/main/java/sirius/biz/storage/layer2/BlobDispatcher.java b/src/main/java/sirius/biz/storage/layer2/BlobDispatcher.java
@@ -165,11 +165,11 @@ private void executeHook(String uri, String hook, String payload) {
      */
     private void physicalDelivery(WebContext request, BlobUri blobUri) {
         Response response = request.respondWith();
-        Integer cacheSeconds = computeCacheDurationFromHash(response,
-                                                            blobUri.getAccessToken(),
+        Integer cacheSeconds = computeCacheDurationFromHash(blobUri.getAccessToken(),
                                                             blobUri.getPhysicalKey(),
                                                             blobUri.getStorageSpace());
         if (cacheSeconds == null) {
+            response.error(HttpResponseStatus.UNAUTHORIZED);
             return;
         }
         response.cachedForSeconds(cacheSeconds);
@@ -190,17 +190,15 @@ private void physicalDelivery(WebContext request, BlobUri blobUri) {
     /**
      * Checks if the provided accessToken is invalid and return the cache time in seconds based on the hash validity.
      *
-     * @param response    the response to return
      * @param accessToken the security token to verify
      * @param key         the key to verify
      * @param space       the space which is accessed
      * @return the cache time in seconds or <tt>null</tt> if the hash is invalid
      */
-    private Integer computeCacheDurationFromHash(Response response, String accessToken, String key, String space) {
+    private Integer computeCacheDurationFromHash(String accessToken, String key, String space) {
         BlobStorageSpace storageSpace = blobStorage.getSpace(space);
         Optional<Integer> optionalHashDays = utils.verifyHash(key, accessToken, storageSpace.getUrlValidityDays());
         if (optionalHashDays.isEmpty()) {
-            response.error(HttpResponseStatus.UNAUTHORIZED);
             return null;
         }
 
@@ -238,34 +236,33 @@ private void virtualDelivery(WebContext request, BlobUri blobUri) {
         String effectiveKey = Strings.isFilled(variant) ? blobKey + "-" + variant : blobKey;
 
         Response response = request.respondWith();
-        Integer cacheSeconds = computeCacheDurationFromHash(response,
-                                                            blobUri.getAccessToken(),
-                                                            effectiveKey,
-                                                            blobUri.getStorageSpace());
+        Integer cacheSeconds =
+                computeCacheDurationFromHash(blobUri.getAccessToken(), effectiveKey, blobUri.getStorageSpace());
         if (cacheSeconds == null) {
+            response.error(HttpResponseStatus.UNAUTHORIZED);
             return;
         }
-        response.cachedForSeconds(cacheSeconds);
-
-        BlobStorageSpace storageSpace = blobStorage.getSpace(blobUri.getStorageSpace());
 
         if (blobUri.isCacheable()) {
-            // If a virtual request is marked as cacheable, we try to redirect to the proper physical blob key
-            // as this will remain in cache much longer (and the redirect itself will also be cached). The additional
-            // HTTP round-trip for the redirect shouldn't hurt too much, as it is most probably optimized away due to
-            // keep-alive. However, using a physical delivery with infinite cache settings will enable any downstream
-            // reverse-proxies to maximize their cache utilization...
-            URLBuilder.UrlResult urlResult =
-                    buildPhysicalRedirectUrl(storageSpace, blobUri, cacheSeconds == Response.HTTP_CACHE_INFINITE);
-
-            if (urlResult.urlType() == URLBuilder.UrlType.PHYSICAL) {
-                response.redirectTemporarily(urlResult.url());
-                return;
-            }
+            response.cachedForSeconds(cacheSeconds);
         } else {
             response.notCached();
         }
 
+        // Check if we have an actual file to deliver ...
+        BlobStorageSpace storageSpace = blobStorage.getSpace(blobUri.getStorageSpace());
+        URLBuilder.UrlResult urlResult =
+                buildPhysicalRedirectUrl(storageSpace, blobUri, cacheSeconds == Response.HTTP_CACHE_INFINITE);
+        if (urlResult.urlType() == URLBuilder.UrlType.PHYSICAL) {
+            // ... and if so, redirect to the physical URL ...
+            if (blobUri.isCacheable()) {
+                // ... but only caches the redirect URL to maximum 1 hour, so the underlying URL can be changed.
+                response.cachedForSeconds(Math.min(cacheSeconds, 3600));
+            }
+            response.redirectTemporarily(urlResult.url());
+            return;
+        }
+
         String filename = blobUri.getFilename();
 
         if (blobUri.isDownload()) {