Changeset 13778 in josm

Timestamp:

2018-05-16T22:12:13+02:00 (6 years ago)

Author:

wiktorn

Message:

Prefer Cache-Control header over Expires header

According to RFC2616 Cache-Control takes precedence over Expires header.

See: #16249

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJob.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJobTest.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJob.java

@@ -437 +437 @@
         CacheEntryAttributes ret = new CacheEntryAttributes();
 
-        Long lng = urlConn.getExpiration();
-        if (lng.equals(0L)) {
-            try {
-                String str = urlConn.getHeaderField("Cache-Control");
-                if (str != null) {
-                    for (String token: str.split(",")) {
-                        if (token.startsWith("max-age=")) {
-                            lng = TimeUnit.SECONDS.toMillis(Long.parseLong(token.substring(8))) + System.currentTimeMillis();
-                        }
+        /*
+         * according to https://www.ietf.org/rfc/rfc2616.txt Cache-Control takes precedence over max-age
+         * max-age is for private caches, s-max-age is for shared caches. We take any value that is larger
+         */
+        Long expiration = 0L;
+        String cacheControl = urlConn.getHeaderField("Cache-Control");
+        if (cacheControl != null) {
+            for (String token: cacheControl.split(",")) {
+                try {
+                    if (token.startsWith("max-age=")) {
+                        expiration = Math.max(expiration,
+                                TimeUnit.SECONDS.toMillis(Long.parseLong(token.substring("max-age=".length())))
+                                + System.currentTimeMillis()
+                                );
                     }
+                    if (token.startsWith("s-max-age=")) {
+                        expiration = Math.max(expiration,
+                                TimeUnit.SECONDS.toMillis(Long.parseLong(token.substring("s-max-age=".length())))
+                                + System.currentTimeMillis()
+                                );
+                    }
+                } catch (NumberFormatException e) {
+                    // ignore malformed Cache-Control headers
+                    Logging.trace(e);
                 }
-            } catch (NumberFormatException e) {
-                // ignore malformed Cache-Control headers
-                Logging.trace(e);
-            }
-            if (lng.equals(0L)) {
-                lng = System.currentTimeMillis() + DEFAULT_EXPIRE_TIME;
-            }
-        }
-
-        ret.setExpirationTime(Math.max(minimumExpiryTime + System.currentTimeMillis(), lng));
+            }
+        }
+
+        if (expiration.equals(0L)) {
+            expiration = urlConn.getExpiration();
+        }
+
+        // if nothing is found - set default
+        if (expiration.equals(0L)) {
+            expiration = System.currentTimeMillis() + DEFAULT_EXPIRE_TIME;
+        }
+
+        ret.setExpirationTime(Math.max(minimumExpiryTime + System.currentTimeMillis(), expiration));
         ret.setLastModification(now);
         ret.setEtag(urlConn.getHeaderField("ETag"));

trunk/test/unit/org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJobTest.java

@@ -353 +353 @@
 
     /**
+     * Check if Cache-Control takes precedence over max-age
+     * Expires is lower - JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10
+     * Cache control : JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2
+     *
+     * Both are smaller than DEFAULT_EXPIRE_TIME, so we can test, that it's not DEFAULT_EXPIRE_TIME that extended
+     * expiration
+     *
+     * @throws IOException exception
+     */
+
+    @Test
+    public void testCacheControlVsExpires() throws IOException {
+        long testStart = System.currentTimeMillis();
+        int minimumExpiryTimeSeconds = 0;
+
+        tileServer.stubFor(
+                WireMock.get(WireMock.urlEqualTo("/test"))
+                .willReturn(WireMock.aResponse()
+                        .withHeader("Expires", TestUtils.getHTTPDate(testStart + (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10)))
+                        .withHeader("Cache-Control", "max-age=" + TimeUnit.MILLISECONDS.toSeconds((JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2)))
+                        .withBody("mock entry")
+                        )
+                );
+        tileServer.stubFor(
+                WireMock.head(WireMock.urlEqualTo("/test"))
+                .willReturn(WireMock.aResponse()
+                        .withHeader("Expires", TestUtils.getHTTPDate(testStart + (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10)))
+                        .withHeader("Cache-Control", "max-age=" + TimeUnit.MILLISECONDS.toSeconds((JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2)))
+                        )
+                );
+        TestCachedTileLoaderJob job = new TestCachedTileLoaderJob(tileServer.url("/test"), "test", minimumExpiryTimeSeconds);
+        Listener listener = submitJob(job, false);
+        tileServer.verify(1, WireMock.getRequestedFor(WireMock.urlEqualTo("/test")));
+        assertArrayEquals("mock entry".getBytes(StandardCharsets.UTF_8), listener.data);
+
+
+        assertTrue("Cache entry expiration is " + (listener.attributes.getExpirationTime() - testStart) + " which is not larger than " +
+                (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10) + " (Expires header)",
+                listener.attributes.getExpirationTime() >= testStart + (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10));
+
+        assertTrue("Cache entry expiration is " +
+                (listener.attributes.getExpirationTime() - System.currentTimeMillis()) +
+                " which is not less than " +
+                (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2) + " (Cache-Control: max-age=)",
+                listener.attributes.getExpirationTime() <= System.currentTimeMillis() + (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2)
+                );
+    }
+
+    /**
+     * Check if Cache-Control s-max-age is honored
+     * mock returns expiration: JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10
+     * minimum expire time: JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME * 2
+     *
+     * @throws IOException exception
+     */
+
+    @Test
+    public void testMaxAgeVsSMaxAge() throws IOException {
+        long testStart = System.currentTimeMillis();
+        int minimumExpiryTimeSeconds = 0;
+
+
+        tileServer.stubFor(
+                WireMock.get(WireMock.urlEqualTo("/test"))
+                .willReturn(WireMock.aResponse()
+                        .withHeader("Cache-Control", "" +
+                                "max-age=" + TimeUnit.MILLISECONDS.toSeconds((JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10)) + "," +
+                                "s-max-age=" + TimeUnit.MILLISECONDS.toSeconds((JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2))
+                                )
+                        .withBody("mock entry")
+                        )
+                );
+        tileServer.stubFor(
+                WireMock.head(WireMock.urlEqualTo("/test"))
+                .willReturn(WireMock.aResponse()
+                        .withHeader("Cache-Control", "" +
+                                "max-age=" + TimeUnit.MILLISECONDS.toSeconds((JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10)) + "," +
+                                "s-max-age=" + TimeUnit.MILLISECONDS.toSeconds((JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2))
+                        )
+                ));
+        TestCachedTileLoaderJob job = new TestCachedTileLoaderJob(tileServer.url("/test"), "test", minimumExpiryTimeSeconds);
+        Listener listener = submitJob(job, false);
+        tileServer.verify(1, WireMock.getRequestedFor(WireMock.urlEqualTo("/test")));
+        assertArrayEquals("mock entry".getBytes(StandardCharsets.UTF_8), listener.data);
+
+
+        assertTrue("Cache entry expiration is " + (listener.attributes.getExpirationTime() - testStart) + " which is not larger than " +
+                (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10) + " (Cache-Control: max-age)",
+                listener.attributes.getExpirationTime() >= testStart + (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10));
+
+        assertTrue("Cache entry expiration is " +
+                (listener.attributes.getExpirationTime() - System.currentTimeMillis()) +
+                " which is not less than " +
+                (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2) + " (Cache-Control: s-max-age)",
+                listener.attributes.getExpirationTime() <= System.currentTimeMillis() + (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2)
+                );
+    }
+
+
+    /**
      * Check if verifying cache entries using HEAD requests work properly
      * @throws IOException exception