Changeset 13778 in josm


Ignore:
Timestamp:
2018-05-16T22:12:13+02:00 (5 months 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:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJob.java

    r13733 r13778  
    437437        CacheEntryAttributes ret = new CacheEntryAttributes();
    438438
    439         Long lng = urlConn.getExpiration();
    440         if (lng.equals(0L)) {
    441             try {
    442                 String str = urlConn.getHeaderField("Cache-Control");
    443                 if (str != null) {
    444                     for (String token: str.split(",")) {
    445                         if (token.startsWith("max-age=")) {
    446                             lng = TimeUnit.SECONDS.toMillis(Long.parseLong(token.substring(8))) + System.currentTimeMillis();
    447                         }
     439        /*
     440         * according to https://www.ietf.org/rfc/rfc2616.txt Cache-Control takes precedence over max-age
     441         * max-age is for private caches, s-max-age is for shared caches. We take any value that is larger
     442         */
     443        Long expiration = 0L;
     444        String cacheControl = urlConn.getHeaderField("Cache-Control");
     445        if (cacheControl != null) {
     446            for (String token: cacheControl.split(",")) {
     447                try {
     448                    if (token.startsWith("max-age=")) {
     449                        expiration = Math.max(expiration,
     450                                TimeUnit.SECONDS.toMillis(Long.parseLong(token.substring("max-age=".length())))
     451                                + System.currentTimeMillis()
     452                                );
    448453                    }
     454                    if (token.startsWith("s-max-age=")) {
     455                        expiration = Math.max(expiration,
     456                                TimeUnit.SECONDS.toMillis(Long.parseLong(token.substring("s-max-age=".length())))
     457                                + System.currentTimeMillis()
     458                                );
     459                    }
     460                } catch (NumberFormatException e) {
     461                    // ignore malformed Cache-Control headers
     462                    Logging.trace(e);
    449463                }
    450             } catch (NumberFormatException e) {
    451                 // ignore malformed Cache-Control headers
    452                 Logging.trace(e);
    453             }
    454             if (lng.equals(0L)) {
    455                 lng = System.currentTimeMillis() + DEFAULT_EXPIRE_TIME;
    456             }
    457         }
    458 
    459         ret.setExpirationTime(Math.max(minimumExpiryTime + System.currentTimeMillis(), lng));
     464            }
     465        }
     466
     467        if (expiration.equals(0L)) {
     468            expiration = urlConn.getExpiration();
     469        }
     470
     471        // if nothing is found - set default
     472        if (expiration.equals(0L)) {
     473            expiration = System.currentTimeMillis() + DEFAULT_EXPIRE_TIME;
     474        }
     475
     476        ret.setExpirationTime(Math.max(minimumExpiryTime + System.currentTimeMillis(), expiration));
    460477        ret.setLastModification(now);
    461478        ret.setEtag(urlConn.getHeaderField("ETag"));
  • trunk/test/unit/org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJobTest.java

    r13742 r13778  
    353353
    354354    /**
     355     * Check if Cache-Control takes precedence over max-age
     356     * Expires is lower - JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10
     357     * Cache control : JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2
     358     *
     359     * Both are smaller than DEFAULT_EXPIRE_TIME, so we can test, that it's not DEFAULT_EXPIRE_TIME that extended
     360     * expiration
     361     *
     362     * @throws IOException exception
     363     */
     364
     365    @Test
     366    public void testCacheControlVsExpires() throws IOException {
     367        long testStart = System.currentTimeMillis();
     368        int minimumExpiryTimeSeconds = 0;
     369
     370        tileServer.stubFor(
     371                WireMock.get(WireMock.urlEqualTo("/test"))
     372                .willReturn(WireMock.aResponse()
     373                        .withHeader("Expires", TestUtils.getHTTPDate(testStart + (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10)))
     374                        .withHeader("Cache-Control", "max-age=" + TimeUnit.MILLISECONDS.toSeconds((JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2)))
     375                        .withBody("mock entry")
     376                        )
     377                );
     378        tileServer.stubFor(
     379                WireMock.head(WireMock.urlEqualTo("/test"))
     380                .willReturn(WireMock.aResponse()
     381                        .withHeader("Expires", TestUtils.getHTTPDate(testStart + (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10)))
     382                        .withHeader("Cache-Control", "max-age=" + TimeUnit.MILLISECONDS.toSeconds((JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2)))
     383                        )
     384                );
     385        TestCachedTileLoaderJob job = new TestCachedTileLoaderJob(tileServer.url("/test"), "test", minimumExpiryTimeSeconds);
     386        Listener listener = submitJob(job, false);
     387        tileServer.verify(1, WireMock.getRequestedFor(WireMock.urlEqualTo("/test")));
     388        assertArrayEquals("mock entry".getBytes(StandardCharsets.UTF_8), listener.data);
     389
     390
     391        assertTrue("Cache entry expiration is " + (listener.attributes.getExpirationTime() - testStart) + " which is not larger than " +
     392                (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10) + " (Expires header)",
     393                listener.attributes.getExpirationTime() >= testStart + (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10));
     394
     395        assertTrue("Cache entry expiration is " +
     396                (listener.attributes.getExpirationTime() - System.currentTimeMillis()) +
     397                " which is not less than " +
     398                (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2) + " (Cache-Control: max-age=)",
     399                listener.attributes.getExpirationTime() <= System.currentTimeMillis() + (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2)
     400                );
     401    }
     402
     403    /**
     404     * Check if Cache-Control s-max-age is honored
     405     * mock returns expiration: JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10
     406     * minimum expire time: JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME * 2
     407     *
     408     * @throws IOException exception
     409     */
     410
     411    @Test
     412    public void testMaxAgeVsSMaxAge() throws IOException {
     413        long testStart = System.currentTimeMillis();
     414        int minimumExpiryTimeSeconds = 0;
     415
     416
     417        tileServer.stubFor(
     418                WireMock.get(WireMock.urlEqualTo("/test"))
     419                .willReturn(WireMock.aResponse()
     420                        .withHeader("Cache-Control", "" +
     421                                "max-age=" + TimeUnit.MILLISECONDS.toSeconds((JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10)) + "," +
     422                                "s-max-age=" + TimeUnit.MILLISECONDS.toSeconds((JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2))
     423                                )
     424                        .withBody("mock entry")
     425                        )
     426                );
     427        tileServer.stubFor(
     428                WireMock.head(WireMock.urlEqualTo("/test"))
     429                .willReturn(WireMock.aResponse()
     430                        .withHeader("Cache-Control", "" +
     431                                "max-age=" + TimeUnit.MILLISECONDS.toSeconds((JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10)) + "," +
     432                                "s-max-age=" + TimeUnit.MILLISECONDS.toSeconds((JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2))
     433                        )
     434                ));
     435        TestCachedTileLoaderJob job = new TestCachedTileLoaderJob(tileServer.url("/test"), "test", minimumExpiryTimeSeconds);
     436        Listener listener = submitJob(job, false);
     437        tileServer.verify(1, WireMock.getRequestedFor(WireMock.urlEqualTo("/test")));
     438        assertArrayEquals("mock entry".getBytes(StandardCharsets.UTF_8), listener.data);
     439
     440
     441        assertTrue("Cache entry expiration is " + (listener.attributes.getExpirationTime() - testStart) + " which is not larger than " +
     442                (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10) + " (Cache-Control: max-age)",
     443                listener.attributes.getExpirationTime() >= testStart + (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 10));
     444
     445        assertTrue("Cache entry expiration is " +
     446                (listener.attributes.getExpirationTime() - System.currentTimeMillis()) +
     447                " which is not less than " +
     448                (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2) + " (Cache-Control: s-max-age)",
     449                listener.attributes.getExpirationTime() <= System.currentTimeMillis() + (JCSCachedTileLoaderJob.DEFAULT_EXPIRE_TIME / 2)
     450                );
     451    }
     452
     453
     454    /**
    355455     * Check if verifying cache entries using HEAD requests work properly
    356456     * @throws IOException exception
Note: See TracChangeset for help on using the changeset viewer.