From 762028805e9883e682e806436c4edb88f29c9992 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sun, 15 Oct 2023 19:28:33 +0200 Subject: [PATCH] Moved delta-second parsing routine to CacheSupport --- .../impl/cache/CacheControlHeaderParser.java | 15 ++---- .../client5/http/impl/cache/CacheSupport.java | 21 ++++++++ .../http/impl/cache/CacheValidityPolicy.java | 29 +++-------- .../cache/CachedHttpResponseGenerator.java | 4 +- .../http/impl/cache/TestCacheSupport.java | 52 +++++++++++++++++++ .../impl/cache/TestCacheValidityPolicy.java | 2 +- .../TestCachedHttpResponseGenerator.java | 4 +- 7 files changed, 89 insertions(+), 38 deletions(-) create mode 100644 httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheSupport.java diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheControlHeaderParser.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheControlHeaderParser.java index ddf7ac296..a03e9310b 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheControlHeaderParser.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheControlHeaderParser.java @@ -250,18 +250,11 @@ class CacheControlHeaderParser { } private static long parseSeconds(final String name, final String value) { - if (TextUtils.isEmpty(value)) { - return -1; + final long delta = CacheSupport.deltaSeconds(value); + if (delta == -1 && LOG.isDebugEnabled()) { + LOG.debug("Directive {} was malformed: {}", name, value); } - try { - final long n = Long.parseLong(value); - return n < 0 ? -1 : n; - } catch (final NumberFormatException e) { - if (LOG.isDebugEnabled()) { - LOG.debug("Directive {} was malformed: {}", name, value); - } - } - return 0; + return delta; } } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheSupport.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheSupport.java index 439b613ad..f4ec0604d 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheSupport.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheSupport.java @@ -45,6 +45,8 @@ import org.apache.hc.core5.net.URIAuthority; import org.apache.hc.core5.net.URIBuilder; import org.apache.hc.core5.util.Args; import org.apache.hc.core5.util.CharArrayBuffer; +import org.apache.hc.core5.util.TextUtils; +import org.apache.hc.core5.util.TimeValue; import org.apache.hc.core5.util.Tokenizer; /** @@ -192,4 +194,23 @@ public final class CacheSupport { return targetURI.isAbsolute() && Objects.equals(requestURI.getAuthority(), targetURI.getAuthority()); } + public static final TimeValue MAX_AGE = TimeValue.ofSeconds(Integer.MAX_VALUE + 1L); + + public static long deltaSeconds(final String s) { + if (TextUtils.isEmpty(s)) { + return -1; + } + try { + long ageValue = Long.parseLong(s); + if (ageValue < 0) { + ageValue = -1; // Handle negative age values as invalid + } else if (ageValue > Integer.MAX_VALUE) { + ageValue = MAX_AGE.toSeconds(); + } + return ageValue; + } catch (final NumberFormatException ignore) { + } + return 0; + } + } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java index 553cfa190..74c0e2239 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java @@ -42,10 +42,6 @@ class CacheValidityPolicy { private static final Logger LOG = LoggerFactory.getLogger(CacheValidityPolicy.class); - - public static final TimeValue MAX_AGE = TimeValue.ofSeconds(Integer.MAX_VALUE + 1L); - - private final float heuristicCoefficient; private final TimeValue heuristicDefaultLifetime; @@ -198,7 +194,7 @@ class CacheValidityPolicy { protected TimeValue getApparentAge(final HttpCacheEntry entry) { final Instant dateValue = entry.getInstant(); if (dateValue == null) { - return MAX_AGE; + return CacheSupport.MAX_AGE; } final Duration diff = Duration.between(dateValue, entry.getResponseInstant()); if (diff.isNegative()) { @@ -225,24 +221,13 @@ class CacheValidityPolicy { protected long getAgeValue(final HttpCacheEntry entry) { final Header age = entry.getFirstHeader(HttpHeaders.AGE); if (age != null) { - try { - final AtomicReference firstToken = new AtomicReference<>(); - CacheSupport.parseTokens(age, token -> firstToken.compareAndSet(null, token)); - final String s = firstToken.get(); - if (s != null) { - long ageValue = Long.parseLong(s); - if (ageValue < 0) { - ageValue = 0; // Handle negative age values as invalid - } else if (ageValue > Integer.MAX_VALUE) { - ageValue = MAX_AGE.toSeconds(); - } - return ageValue; - } - } catch (final NumberFormatException ex) { - if (LOG.isDebugEnabled()) { - LOG.debug("Invalid Age header: '{}'. Ignoring.", age, ex); - } + final AtomicReference firstToken = new AtomicReference<>(); + CacheSupport.parseTokens(age, token -> firstToken.compareAndSet(null, token)); + final long delta = CacheSupport.deltaSeconds(firstToken.get()); + if (delta == -1 && LOG.isDebugEnabled()) { + LOG.debug("Malformed Age value: {}", age); } + return delta > 0 ? delta : 0; } // If we've got here, there were no valid Age headers return 0; diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedHttpResponseGenerator.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedHttpResponseGenerator.java index 388387121..da2ed762c 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedHttpResponseGenerator.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedHttpResponseGenerator.java @@ -81,8 +81,8 @@ class CachedHttpResponseGenerator { final TimeValue age = this.validityStrategy.getCurrentAge(entry, now); if (TimeValue.isPositive(age)) { - if (age.compareTo(CacheValidityPolicy.MAX_AGE) >= 0) { - response.setHeader(HttpHeaders.AGE, Long.toString(CacheValidityPolicy.MAX_AGE.toSeconds())); + if (age.compareTo(CacheSupport.MAX_AGE) >= 0) { + response.setHeader(HttpHeaders.AGE, Long.toString(CacheSupport.MAX_AGE.toSeconds())); } else { response.setHeader(HttpHeaders.AGE, Long.toString(age.toSeconds())); } diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheSupport.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheSupport.java new file mode 100644 index 000000000..3be79ad2f --- /dev/null +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheSupport.java @@ -0,0 +1,52 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ + +package org.apache.hc.client5.http.impl.cache; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link CacheSupport}. + */ +public class TestCacheSupport { + + @Test + public void testParseDeltaSeconds() throws Exception { + Assertions.assertEquals(1234L, CacheSupport.deltaSeconds("1234")); + Assertions.assertEquals(0L, CacheSupport.deltaSeconds("0")); + Assertions.assertEquals(-1L, CacheSupport.deltaSeconds("-1")); + Assertions.assertEquals(-1L, CacheSupport.deltaSeconds("-100")); + Assertions.assertEquals(-1L, CacheSupport.deltaSeconds("")); + Assertions.assertEquals(-1L, CacheSupport.deltaSeconds(null)); + Assertions.assertEquals(0L, CacheSupport.deltaSeconds("huh?")); + Assertions.assertEquals(2147483648L, CacheSupport.deltaSeconds("2147483648")); + Assertions.assertEquals(2147483648L, CacheSupport.deltaSeconds("2147483649")); + Assertions.assertEquals(2147483648L, CacheSupport.deltaSeconds("214748364712")); + } + +} diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java index 17ec204a0..a8e9f407f 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java @@ -67,7 +67,7 @@ public class TestCacheValidityPolicy { new BasicHeader("Server", "MockServer/1.0") }; final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(headers); - assertEquals(CacheValidityPolicy.MAX_AGE, impl.getApparentAge(entry)); + assertEquals(CacheSupport.MAX_AGE, impl.getApparentAge(entry)); } @Test diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedHttpResponseGenerator.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedHttpResponseGenerator.java index 602785e4d..3a9cc6ff8 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedHttpResponseGenerator.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedHttpResponseGenerator.java @@ -106,7 +106,7 @@ public class TestCachedHttpResponseGenerator { @Test public void testAgeHeaderIsPopulatedWithMaxAgeIfCurrentAgeTooBig() throws Exception { - currentAge(TimeValue.ofSeconds(CacheValidityPolicy.MAX_AGE.toSeconds() + 1L)); + currentAge(TimeValue.ofSeconds(CacheSupport.MAX_AGE.toSeconds() + 1L)); final SimpleHttpResponse response = impl.generateResponse(request, entry); @@ -114,7 +114,7 @@ public class TestCachedHttpResponseGenerator { final Header ageHdr = response.getFirstHeader("Age"); Assertions.assertNotNull(ageHdr); - Assertions.assertEquals(CacheValidityPolicy.MAX_AGE.toSeconds(), Long.parseLong(ageHdr.getValue())); + Assertions.assertEquals(CacheSupport.MAX_AGE.toSeconds(), Long.parseLong(ageHdr.getValue())); } private void currentAge(final TimeValue age) {