Apache
/
httpcomponents-client
mirror of https://github.com/apache/httpcomponents-client.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Enhancements to HTTP Conditional Request Handling: 

1. Added support for HTTP 412 (Precondition Failed) status code. This status code is returned when one or more conditions given in the request header fields evaluated to false when tested on the server.
2. Ensured all instances of `If-Match` and `If-None-Match` headers are considered as required by the specification.
3. Added handling for cases where a weak ETag is used with a range request.
This commit is contained in:
Arturo Bernal 2023-05-29 21:39:12 +02:00 committed by Oleg Kalnichevski
parent 38b8398a20
commit b7a39b3040
7 changed files with 192 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java vendored
View File

@ -230,12 +230,6 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler
return;
}
final SimpleHttpResponse fatalErrorResponse = getFatallyNonCompliantResponse(request, context);
if (fatalErrorResponse != null) {
triggerResponse(fatalErrorResponse, scope, asyncExecCallback);
return;
}
requestCompliance.makeRequestCompliant(request);
request.addHeader(HttpHeaders.VIA,via);
@ -266,6 +260,12 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler
@Override
public void completed(final HttpCacheEntry entry) {
final SimpleHttpResponse fatalErrorResponse = getFatallyNonCompliantResponse(request, context, entry != null);
if (fatalErrorResponse != null) {
triggerResponse(fatalErrorResponse, scope, asyncExecCallback);
return;
}
if (entry == null) {
LOG.debug("Cache miss");
handleCacheMiss(requestCacheControl, target, request, entityProducer, scope, chain, asyncExecCallback);

2
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedHttpResponseGenerator.java vendored
View File

@ -189,7 +189,7 @@ class CachedHttpResponseGenerator {
"Weak eTag not compatible with byte range", ContentType.DEFAULT_TEXT);
case WEAK_ETAG_ON_PUTDELETE_METHOD_ERROR:
return SimpleHttpResponse.create(HttpStatus.SC_BAD_REQUEST,
return SimpleHttpResponse.create(HttpStatus.SC_PRECONDITION_FAILED,
"Weak eTag not compatible with PUT or DELETE requests");
default:

5
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java vendored
View File

@ -208,8 +208,9 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
setResponseStatus(context, CacheResponseStatus.CACHE_MODULE_RESPONSE);
return new BasicClassicHttpResponse(HttpStatus.SC_NOT_IMPLEMENTED);
}
final HttpCacheEntry entry = responseCache.getCacheEntry(target, request);
final SimpleHttpResponse fatalErrorResponse = getFatallyNonCompliantResponse(request, context);
final SimpleHttpResponse fatalErrorResponse = getFatallyNonCompliantResponse(request, context, entry != null);
if (fatalErrorResponse != null) {
return convert(fatalErrorResponse, scope);
}
@ -224,7 +225,7 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
return callBackend(target, request, scope, chain);
}
final HttpCacheEntry entry = responseCache.getCacheEntry(target, request);
if (entry == null) {
LOG.debug("Cache miss");
return handleCacheMiss(target, request, requestCacheControl, scope, chain);

5
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java vendored
View File

@ -147,8 +147,9 @@ public class CachingExecBase {
*/
SimpleHttpResponse getFatallyNonCompliantResponse(
final HttpRequest request,
final HttpContext context) {
final List<RequestProtocolError> fatalError = requestCompliance.requestIsFatallyNonCompliant(request);
final HttpContext context,
final boolean resourceExists) {
final List<RequestProtocolError> fatalError = requestCompliance.requestIsFatallyNonCompliant(request, resourceExists);
if (fatalError != null && !fatalError.isEmpty()) {
setResponseStatus(context, CacheResponseStatus.CACHE_MODULE_RESPONSE);
return responseGenerator.getErrorForRequest(fatalError.get(0));

80
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/RequestProtocolCompliance.java vendored
View File

@ -26,7 +26,11 @@
*/
package org.apache.hc.client5.http.impl.cache;
import static org.apache.hc.client5.http.utils.DateUtils.parseStandardDate;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import org.apache.hc.client5.http.cache.HeaderConstants;
@ -35,10 +39,14 @@ import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpVersion;
import org.apache.hc.core5.http.ProtocolVersion;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
class RequestProtocolCompliance {
private final boolean weakETagOnPutDeleteAllowed;
private static final Logger LOG = LoggerFactory.getLogger(RequestProtocolCompliance.class);
public RequestProtocolCompliance() {
super();
this.weakETagOnPutDeleteAllowed = false;
@ -56,7 +64,7 @@ class RequestProtocolCompliance {
* @param request the HttpRequest Object
* @return list of {@link RequestProtocolError}
*/
public List<RequestProtocolError> requestIsFatallyNonCompliant(final HttpRequest request) {
public List<RequestProtocolError> requestIsFatallyNonCompliant(final HttpRequest request, final boolean resourceExists) {
final List<RequestProtocolError> theErrors = new ArrayList<>();
RequestProtocolError anError = requestHasWeakETagAndRange(request);
@ -65,7 +73,7 @@ class RequestProtocolCompliance {
}
if (!weakETagOnPutDeleteAllowed) {
anError = requestHasWeekETagForPUTOrDELETEIfMatch(request);
anError = requestHasWeekETagForPUTOrDELETEIfMatch(request, resourceExists);
if (anError != null) {
theErrors.add(anError);
}
@ -122,52 +130,66 @@ class RequestProtocolCompliance {
}
private RequestProtocolError requestHasWeakETagAndRange(final HttpRequest request) {
// TODO: Should these be looking at all the headers marked as Range?
final String method = request.getMethod();
if (!(HeaderConstants.GET_METHOD.equals(method))) {
if (!(HeaderConstants.GET_METHOD.equals(method) || HeaderConstants.HEAD_METHOD.equals(method))) {
return null;
}
final Header range = request.getFirstHeader(HttpHeaders.RANGE);
if (range == null) {
if (!request.containsHeader(HttpHeaders.RANGE)) {
return null;
}
final Header ifRange = request.getFirstHeader(HttpHeaders.IF_RANGE);
if (ifRange == null) {
return null;
}
final Instant ifRangeInstant = parseStandardDate(request, HttpHeaders.IF_RANGE);
final Instant lastModifiedInstant = parseStandardDate(request, HttpHeaders.LAST_MODIFIED);
final String val = ifRange.getValue();
if (val.startsWith("W/")) {
return RequestProtocolError.WEAK_ETAG_AND_RANGE_ERROR;
for (final Iterator<Header> it = request.headerIterator(HttpHeaders.IF_RANGE); it.hasNext(); ) {
final String val = it.next().getValue();
if (val.startsWith("W/")) {
if (LOG.isDebugEnabled()) {
LOG.debug("Weak ETag found in If-Range header");
}
return RequestProtocolError.WEAK_ETAG_AND_RANGE_ERROR;
} else {
// Not a strong validator or doesn't match Last-Modified
if (ifRangeInstant != null && lastModifiedInstant != null
&& !ifRangeInstant.equals(lastModifiedInstant)) {
if (LOG.isDebugEnabled()) {
LOG.debug("If-Range does not match Last-Modified");
}
return RequestProtocolError.WEAK_ETAG_AND_RANGE_ERROR;
}
}
}
return null;
}
private RequestProtocolError requestHasWeekETagForPUTOrDELETEIfMatch(final HttpRequest request) {
// TODO: Should these be looking at all the headers marked as If-Match/If-None-Match?
private RequestProtocolError requestHasWeekETagForPUTOrDELETEIfMatch(final HttpRequest request, final boolean resourceExists) {
final String method = request.getMethod();
if (!(HeaderConstants.PUT_METHOD.equals(method) || HeaderConstants.DELETE_METHOD.equals(method))) {
if (!(HeaderConstants.PUT_METHOD.equals(method) || HeaderConstants.DELETE_METHOD.equals(method)
|| HeaderConstants.POST_METHOD.equals(method))
) {
return null;
}
final Header ifMatch = request.getFirstHeader(HttpHeaders.IF_MATCH);
if (ifMatch != null) {
final String val = ifMatch.getValue();
if (val.startsWith("W/")) {
return RequestProtocolError.WEAK_ETAG_ON_PUTDELETE_METHOD_ERROR;
}
} else {
final Header ifNoneMatch = request.getFirstHeader(HttpHeaders.IF_NONE_MATCH);
if (ifNoneMatch == null) {
for (final Iterator<Header> it = request.headerIterator(HttpHeaders.IF_MATCH); it.hasNext();) {
final String val = it.next().getValue();
if (val.equals("*") && !resourceExists) {
return null;
}
if (val.startsWith("W/")) {
if (LOG.isDebugEnabled()) {
LOG.debug("Weak ETag found in If-Match header");
}
return RequestProtocolError.WEAK_ETAG_ON_PUTDELETE_METHOD_ERROR;
}
}
final String val2 = ifNoneMatch.getValue();
if (val2.startsWith("W/")) {
for (final Iterator<Header> it = request.headerIterator(HttpHeaders.IF_NONE_MATCH); it.hasNext();) {
final String val = it.next().getValue();
if (val.startsWith("W/") || (val.equals("*") && resourceExists)) {
if (LOG.isDebugEnabled()) {
LOG.debug("Weak ETag found in If-None-Match header");
}
return RequestProtocolError.WEAK_ETAG_ON_PUTDELETE_METHOD_ERROR;
}
}

38
httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java vendored
View File

@ -56,6 +56,7 @@ import org.apache.hc.client5.http.cache.HttpCacheStorage;
import org.apache.hc.client5.http.cache.ResourceIOException;
import org.apache.hc.client5.http.classic.ExecChain;
import org.apache.hc.client5.http.classic.ExecRuntime;
import org.apache.hc.client5.http.classic.methods.HttpDelete;
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.classic.methods.HttpOptions;
import org.apache.hc.client5.http.protocol.HttpClientContext;
@ -64,6 +65,7 @@ import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpStatus;
@ -1651,4 +1653,40 @@ public class TestCachingExecChain {
Assertions.assertTrue(found110);
}
@Test
public void testRequestWithWeakETagAndRange() throws Exception {
final ClassicHttpRequest req1 = new HttpGet("http://foo1.example.com/");
req1.addHeader(HttpHeaders.IF_MATCH, "W/\"weak1\"");
req1.addHeader(HttpHeaders.RANGE, "bytes=0-50");
req1.addHeader(HttpHeaders.IF_RANGE, "W/\"weak2\""); // ETag doesn't match with If-Match ETag
final ClassicHttpResponse resp1 = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK");
resp1.setEntity(HttpTestUtils.makeBody(128));
resp1.setHeader("Content-Length", "128");
resp1.setHeader("ETag", "\"etag\"");
resp1.setHeader("Date", DateUtils.formatStandardDate(Instant.now()));
resp1.setHeader("Cache-Control", "public, max-age=3600");
Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1);
final ClassicHttpResponse result = execute(req1);
Assertions.assertEquals(HttpStatus.SC_BAD_REQUEST, result.getCode());
}
@Test
public void testRequestWithWeakETagForPUTOrDELETEIfMatch() throws Exception {
final ClassicHttpRequest req1 = new HttpDelete("http://foo1.example.com/");
req1.addHeader(HttpHeaders.IF_MATCH, "W/\"weak1\"");
final ClassicHttpResponse resp1 = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK");
resp1.setEntity(HttpTestUtils.makeBody(128));
resp1.setHeader("Content-Length", "128");
resp1.setHeader("ETag", "\"etag\"");
resp1.setHeader("Date", DateUtils.formatStandardDate(Instant.now()));
resp1.setHeader("Cache-Control", "public, max-age=3600");
Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1);
final ClassicHttpResponse result = execute(req1);
Assertions.assertEquals(HttpStatus.SC_PRECONDITION_FAILED, result.getCode());
}
}

93
httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestRequestProtocolCompliance.java vendored
View File

@ -31,7 +31,10 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.util.Collections;
import java.util.List;
import org.apache.hc.client5.http.cache.HeaderConstants;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpVersion;
import org.apache.hc.core5.http.ProtocolVersion;
@ -54,14 +57,14 @@ public class TestRequestProtocolCompliance {
final HttpRequest req = new BasicHttpRequest("GET", "/");
req.setHeader("Range", "bytes=0-499");
req.setHeader("If-Range", "W/\"weak\"");
assertEquals(1, impl.requestIsFatallyNonCompliant(req).size());
assertEquals(1, impl.requestIsFatallyNonCompliant(req, false).size());
}
@Test
public void testRequestWithWeekETagForPUTOrDELETEIfMatch() throws Exception {
final HttpRequest req = new BasicHttpRequest("PUT", "http://example.com/");
req.setHeader("If-Match", "W/\"weak\"");
assertEquals(1, impl.requestIsFatallyNonCompliant(req).size());
assertEquals(1, impl.requestIsFatallyNonCompliant(req, false).size());
}
@Test
@ -69,7 +72,7 @@ public class TestRequestProtocolCompliance {
final HttpRequest req = new BasicHttpRequest("PUT", "http://example.com/");
req.setHeader("If-Match", "W/\"weak\"");
impl = new RequestProtocolCompliance(true);
assertEquals(Collections.emptyList(), impl.requestIsFatallyNonCompliant(req));
assertEquals(Collections.emptyList(), impl.requestIsFatallyNonCompliant(req, false));
}
@Test
@ -98,4 +101,88 @@ public class TestRequestProtocolCompliance {
assertEquals(HttpVersion.HTTP_1_1, wrapper.getVersion());
}
@Test
public void testRequestWithMultipleIfMatchHeaders() {
final HttpRequest req = new BasicHttpRequest(HeaderConstants.PUT_METHOD, "http://example.com/");
req.addHeader(HttpHeaders.IF_MATCH, "W/\"weak1\"");
req.addHeader(HttpHeaders.IF_MATCH, "W/\"weak2\"");
assertEquals(1, impl.requestIsFatallyNonCompliant(req, false).size());
}
@Test
public void testRequestWithMultipleIfNoneMatchHeaders() {
final HttpRequest req = new BasicHttpRequest(HeaderConstants.PUT_METHOD, "http://example.com/");
req.addHeader(HttpHeaders.IF_NONE_MATCH, "W/\"weak1\"");
req.addHeader(HttpHeaders.IF_NONE_MATCH, "W/\"weak2\"");
assertEquals(1, impl.requestIsFatallyNonCompliant(req, false).size());
}
@Test
public void testRequestWithPreconditionFailed() {
final HttpRequest req = new BasicHttpRequest(HeaderConstants.GET_METHOD, "http://example.com/");
req.addHeader(HttpHeaders.IF_MATCH, "W/\"weak1\"");
req.addHeader(HttpHeaders.RANGE, "1");
req.addHeader(HttpHeaders.IF_RANGE, "W/\"weak2\""); // ETag doesn't match with If-Match ETag
// This will cause the precondition If-Match to fail because the ETags are different
final List<RequestProtocolError> requestProtocolErrors = impl.requestIsFatallyNonCompliant(req, false);
assertTrue(requestProtocolErrors.contains(RequestProtocolError.WEAK_ETAG_AND_RANGE_ERROR));
}
@Test
public void testRequestWithValidIfRangeDate() {
final HttpRequest req = new BasicHttpRequest(HeaderConstants.GET_METHOD, "http://example.com/");
req.addHeader(HttpHeaders.RANGE, "bytes=0-499");
req.addHeader(HttpHeaders.LAST_MODIFIED, "Wed, 21 Oct 2023 07:28:00 GMT");
req.addHeader(HttpHeaders.IF_RANGE, "Wed, 21 Oct 2023 07:28:00 GMT");
assertTrue(impl.requestIsFatallyNonCompliant(req, false).isEmpty());
}
@Test
public void testRequestWithInvalidDateFormat() {
final HttpRequest req = new BasicHttpRequest(HeaderConstants.GET_METHOD, "http://example.com/");
req.addHeader(HttpHeaders.RANGE, "bytes=0-499");
req.addHeader(HttpHeaders.LAST_MODIFIED, "Wed, 21 Oct 2023 07:28:00 GMT");
req.addHeader(HttpHeaders.IF_RANGE, "20/10/2023");
assertTrue(impl.requestIsFatallyNonCompliant(req, false).isEmpty());
}
@Test
public void testRequestWithMissingIfRangeDate() {
final HttpRequest req = new BasicHttpRequest(HeaderConstants.GET_METHOD, "http://example.com/");
req.addHeader(HttpHeaders.RANGE, "bytes=0-499");
req.addHeader(HttpHeaders.LAST_MODIFIED, "Wed, 21 Oct 2023 07:28:00 GMT");
assertTrue(impl.requestIsFatallyNonCompliant(req, false).isEmpty());
}
@Test
public void testRequestWithWeakETagAndRangeAndDAte() {
// Setup request with GET method, Range header, If-Range header starting with "W/",
// and a Last-Modified date that doesn't match the If-Range date
final HttpRequest req = new BasicHttpRequest(HeaderConstants.GET_METHOD, "http://example.com/");
req.addHeader(HttpHeaders.RANGE, "bytes=0-499");
req.addHeader(HttpHeaders.LAST_MODIFIED, "Fri, 20 Oct 2023 07:28:00 GMT");
req.addHeader(HttpHeaders.IF_RANGE, "Wed, 18 Oct 2023 07:28:00 GMT");
// Use your implementation to check the request
final List<RequestProtocolError> errors = impl.requestIsFatallyNonCompliant(req, false);
// Assert that the WEAK_ETAG_AND_RANGE_ERROR is in the list of errors
assertTrue(errors.contains(RequestProtocolError.WEAK_ETAG_AND_RANGE_ERROR));
}
@Test
public void testRequestWithWeekETagForPUTOrDELETEIfMatchWithStart() {
final HttpRequest req = new BasicHttpRequest("PUT", "http://example.com/");
req.setHeader(HttpHeaders.IF_MATCH, "*");
assertEquals(0, impl.requestIsFatallyNonCompliant(req, false).size());
}
@Test
public void testRequestOkETagForPUTOrDELETEIfMatch() {
final HttpRequest req = new BasicHttpRequest("PUT", "http://example.com/");
req.setHeader(HttpHeaders.IF_MATCH, "1234");
assertEquals(0, impl.requestIsFatallyNonCompliant(req, false).size());
}
}