From b7b4ef91a4847a75836187443dc742e83401e23d Mon Sep 17 00:00:00 2001 From: Jonathan Moore Date: Fri, 30 Aug 2013 16:01:37 +0000 Subject: [PATCH] HTTPCLIENT-1371: Weak ETag Validation is Useful On PUT With If-Match Contributed by James Leigh git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1519005 13f79535-47bb-0310-9956-ffa450edef68 --- RELEASE_NOTES.txt | 2 + .../impl/client/cache/CachingHttpClient.java | 2 +- .../http/impl/client/cache/CacheConfig.java | 44 +++++++++++++++++-- .../http/impl/client/cache/CachingExec.java | 2 +- .../cache/RequestProtocolCompliance.java | 19 ++++++-- .../cache/TestRequestProtocolCompliance.java | 33 +++++++++++++- 6 files changed, 93 insertions(+), 9 deletions(-) diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index 5fb871e84..4588e539b 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -34,6 +34,8 @@ This release also includes all fixes from the stable 4.2.x release branch. Changelog ------------------- +* [HTTPCLIENT-1371] Weak ETag Validation is Useful On PUT With If-Match + Contributed by James Leigh * [HTTPCLIENT-1384] Expose CacheInvalidator interface. Contributed by Nicolas Richeton diff --git a/httpclient-cache/src/main/java-deprecated/org/apache/http/impl/client/cache/CachingHttpClient.java b/httpclient-cache/src/main/java-deprecated/org/apache/http/impl/client/cache/CachingHttpClient.java index 9a068846f..76a7be202 100644 --- a/httpclient-cache/src/main/java-deprecated/org/apache/http/impl/client/cache/CachingHttpClient.java +++ b/httpclient-cache/src/main/java-deprecated/org/apache/http/impl/client/cache/CachingHttpClient.java @@ -182,7 +182,7 @@ public class CachingHttpClient implements HttpClient { this.conditionalRequestBuilder = new ConditionalRequestBuilder(); this.responseCompliance = new ResponseProtocolCompliance(); - this.requestCompliance = new RequestProtocolCompliance(); + this.requestCompliance = new RequestProtocolCompliance(config.isWeakETagOnPutDeleteAllowed()); this.asynchRevalidator = makeAsynchronousValidator(config); } diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheConfig.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheConfig.java index b2ad348a0..1346a82bd 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheConfig.java +++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheConfig.java @@ -65,6 +65,14 @@ import org.apache.http.util.Args; * adherence to the existing standard, but you may want to * {@link Builder#setAllow303Caching(boolean) enable it}. * + *

Weak ETags on PUT/DELETE If-Match requests. RFC2616 explicitly + * prohibits the use of weak validators in non-GET requests, however, the + * HTTPbis working group says while the limitation for weak validators on ranged + * requests makes sense, weak ETag validation is useful on full non-GET + * requests; e.g., PUT with If-Match. This behavior is off by default, to err on + * the side of a conservative adherence to the existing standard, but you may + * want to {@link Builder#setWeakETagOnPutDeleteAllowed(boolean) enable it}. + * *

Heuristic caching. Per RFC2616, a cache may cache certain cache * entries even if no explicit cache control headers are set by the origin. * This behavior is off by default, but you may want to turn this on if you @@ -113,6 +121,10 @@ public class CacheConfig implements Cloneable { */ public final static boolean DEFAULT_303_CACHING_ENABLED = false; + /** Default setting to allow weak tags on PUT/DELETE methods + */ + public final static boolean DEFAULT_WEAK_ETAG_ON_PUTDELETE_ALLOWED = false; + /** Default setting for heuristic caching */ public final static boolean DEFAULT_HEURISTIC_CACHING_ENABLED = false; @@ -153,6 +165,7 @@ public class CacheConfig implements Cloneable { private int maxCacheEntries; private int maxUpdateRetries; private boolean allow303Caching; + private boolean weakETagOnPutDeleteAllowed; private boolean heuristicCachingEnabled; private float heuristicCoefficient; private long heuristicDefaultLifetime; @@ -172,8 +185,9 @@ public class CacheConfig implements Cloneable { this.maxObjectSize = DEFAULT_MAX_OBJECT_SIZE_BYTES; this.maxCacheEntries = DEFAULT_MAX_CACHE_ENTRIES; this.maxUpdateRetries = DEFAULT_MAX_UPDATE_RETRIES; - this.allow303Caching = false; - this.heuristicCachingEnabled = false; + this.allow303Caching = DEFAULT_303_CACHING_ENABLED; + this.weakETagOnPutDeleteAllowed = DEFAULT_WEAK_ETAG_ON_PUTDELETE_ALLOWED; + this.heuristicCachingEnabled = DEFAULT_HEURISTIC_CACHING_ENABLED; this.heuristicCoefficient = DEFAULT_HEURISTIC_COEFFICIENT; this.heuristicDefaultLifetime = DEFAULT_HEURISTIC_LIFETIME; this.isSharedCache = true; @@ -188,6 +202,7 @@ public class CacheConfig implements Cloneable { final int maxCacheEntries, final int maxUpdateRetries, final boolean allow303Caching, + final boolean weakETagOnPutDeleteAllowed, final boolean heuristicCachingEnabled, final float heuristicCoefficient, final long heuristicDefaultLifetime, @@ -202,6 +217,7 @@ public class CacheConfig implements Cloneable { this.maxCacheEntries = maxCacheEntries; this.maxUpdateRetries = maxUpdateRetries; this.allow303Caching = allow303Caching; + this.weakETagOnPutDeleteAllowed = weakETagOnPutDeleteAllowed; this.heuristicCachingEnabled = heuristicCachingEnabled; this.heuristicCoefficient = heuristicCoefficient; this.heuristicDefaultLifetime = heuristicDefaultLifetime; @@ -312,6 +328,14 @@ public class CacheConfig implements Cloneable { return allow303Caching; } + /** + * Returns whether weak etags is allowed with PUT/DELETE methods. + * @return {@code true} if it is allowed. + */ + public boolean isWeakETagOnPutDeleteAllowed() { + return weakETagOnPutDeleteAllowed; + } + /** * Returns whether heuristic caching is enabled. * @return {@code true} if it is enabled. @@ -519,6 +543,7 @@ public class CacheConfig implements Cloneable { private int maxCacheEntries; private int maxUpdateRetries; private boolean allow303Caching; + private boolean weakETagOnPutDeleteAllowed; private boolean heuristicCachingEnabled; private float heuristicCoefficient; private long heuristicDefaultLifetime; @@ -533,7 +558,8 @@ public class CacheConfig implements Cloneable { this.maxObjectSize = DEFAULT_MAX_OBJECT_SIZE_BYTES; this.maxCacheEntries = DEFAULT_MAX_CACHE_ENTRIES; this.maxUpdateRetries = DEFAULT_MAX_UPDATE_RETRIES; - this.allow303Caching = false; + this.allow303Caching = DEFAULT_303_CACHING_ENABLED; + this.weakETagOnPutDeleteAllowed = DEFAULT_WEAK_ETAG_ON_PUTDELETE_ALLOWED; this.heuristicCachingEnabled = false; this.heuristicCoefficient = DEFAULT_HEURISTIC_COEFFICIENT; this.heuristicDefaultLifetime = DEFAULT_HEURISTIC_LIFETIME; @@ -579,6 +605,16 @@ public class CacheConfig implements Cloneable { return this; } + /** + * Allows or disallows weak etags to be used with PUT/DELETE If-Match requests. + * @param weakETagOnPutDeleteAllowed should be {@code true} to + * permit weak etags, {@code false} to reject them. + */ + public Builder setWeakETagOnPutDeleteAllowed(final boolean weakETagOnPutDeleteAllowed) { + this.weakETagOnPutDeleteAllowed = weakETagOnPutDeleteAllowed; + return this; + } + /** * Enables or disables heuristic caching. * @param heuristicCachingEnabled should be {@code true} to @@ -690,6 +726,7 @@ public class CacheConfig implements Cloneable { maxCacheEntries, maxUpdateRetries, allow303Caching, + weakETagOnPutDeleteAllowed, heuristicCachingEnabled, heuristicCoefficient, heuristicDefaultLifetime, @@ -710,6 +747,7 @@ public class CacheConfig implements Cloneable { .append(", maxCacheEntries=").append(this.maxCacheEntries) .append(", maxUpdateRetries=").append(this.maxUpdateRetries) .append(", 303CachingEnabled=").append(this.allow303Caching) + .append(", weakETagOnPutDeleteAllowed=").append(this.weakETagOnPutDeleteAllowed) .append(", heuristicCachingEnabled=").append(this.heuristicCachingEnabled) .append(", heuristicCoefficient=").append(this.heuristicCoefficient) .append(", heuristicDefaultLifetime=").append(this.heuristicDefaultLifetime) diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java index ef0304cfb..2b6f035e6 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java +++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java @@ -139,7 +139,7 @@ public class CachingExec implements ClientExecChain { this.suitabilityChecker = new CachedResponseSuitabilityChecker(this.validityPolicy, config); this.conditionalRequestBuilder = new ConditionalRequestBuilder(); this.responseCompliance = new ResponseProtocolCompliance(); - this.requestCompliance = new RequestProtocolCompliance(); + this.requestCompliance = new RequestProtocolCompliance(config.isWeakETagOnPutDeleteAllowed()); this.responseCachingPolicy = new ResponseCachingPolicy( this.cacheConfig.getMaxObjectSize(), this.cacheConfig.isSharedCache(), this.cacheConfig.isNeverCacheHTTP10ResponsesWithQuery(), this.cacheConfig.is303CachingEnabled()); diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolCompliance.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolCompliance.java index d16a9a748..c74a607df 100644 --- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolCompliance.java +++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/RequestProtocolCompliance.java @@ -54,6 +54,17 @@ import org.apache.http.protocol.HTTP; */ @Immutable class RequestProtocolCompliance { + private final boolean weakETagOnPutDeleteAllowed; + + public RequestProtocolCompliance() { + super(); + this.weakETagOnPutDeleteAllowed = false; + } + + public RequestProtocolCompliance(final boolean weakETagOnPutDeleteAllowed) { + super(); + this.weakETagOnPutDeleteAllowed = weakETagOnPutDeleteAllowed; + } private static final List disallowedWithNoCache = Arrays.asList(HeaderConstants.CACHE_CONTROL_MIN_FRESH, HeaderConstants.CACHE_CONTROL_MAX_STALE, HeaderConstants.CACHE_CONTROL_MAX_AGE); @@ -73,9 +84,11 @@ class RequestProtocolCompliance { theErrors.add(anError); } - anError = requestHasWeekETagForPUTOrDELETEIfMatch(request); - if (anError != null) { - theErrors.add(anError); + if (!weakETagOnPutDeleteAllowed) { + anError = requestHasWeekETagForPUTOrDELETEIfMatch(request); + if (anError != null) { + theErrors.add(anError); + } } anError = requestContainsNoCacheDirectiveWithFieldName(request); diff --git a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestRequestProtocolCompliance.java b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestRequestProtocolCompliance.java index 0184da4f4..a07f1802f 100644 --- a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestRequestProtocolCompliance.java +++ b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestRequestProtocolCompliance.java @@ -30,10 +30,13 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import java.util.Arrays; + import org.apache.http.HttpEntityEnclosingRequest; import org.apache.http.HttpRequest; import org.apache.http.HttpVersion; import org.apache.http.ProtocolVersion; +import org.apache.http.client.methods.HttpPut; import org.apache.http.client.methods.HttpRequestWrapper; import org.apache.http.message.BasicHttpEntityEnclosingRequest; import org.apache.http.message.BasicHttpRequest; @@ -48,7 +51,35 @@ public class TestRequestProtocolCompliance { @Before public void setUp() { req = HttpTestUtils.makeDefaultRequest(); - impl = new RequestProtocolCompliance(); + impl = new RequestProtocolCompliance(false); + } + + @Test + public void testRequestWithWeakETagAndRange() throws Exception { + req.setHeader("Range", "bytes=0-499"); + req.setHeader("If-Range", "W/\"weak\""); + assertEquals(1, impl.requestIsFatallyNonCompliant(req).size()); + } + + @Test + public void testRequestWithWeekETagForPUTOrDELETEIfMatch() throws Exception { + req = new HttpPut("http://example.com/"); + req.setHeader("If-Match", "W/\"weak\""); + assertEquals(1, impl.requestIsFatallyNonCompliant(req).size()); + } + + @Test + public void testRequestWithWeekETagForPUTOrDELETEIfMatchAllowed() throws Exception { + req = new HttpPut("http://example.com/"); + req.setHeader("If-Match", "W/\"weak\""); + impl = new RequestProtocolCompliance(true); + assertEquals(Arrays.asList(), impl.requestIsFatallyNonCompliant(req)); + } + + @Test + public void testRequestContainsNoCacheDirectiveWithFieldName() throws Exception { + req.setHeader("Cache-Control", "no-cache=false"); + assertEquals(1, impl.requestIsFatallyNonCompliant(req).size()); } @Test