Skip to content

Commit

Permalink
HTTPCORE-756: Support for Configurable 417 Expectation Failed Respons…
Browse files Browse the repository at this point in the history
…e in RequestExpectContinue

Introduce a configurable `shouldRespond417` flag to enable or disable responding with HTTP 417 status code for invalid Expect headers.

This change allows HTTP/1.1 endpoints to more accurately handle the "Expect" header field as per RFC 9110 section 10.1.1. by optionally responding with a 417 status code.
  • Loading branch information
arturobernalg committed Oct 13, 2023
1 parent 115a216 commit 624c92f
Show file tree
Hide file tree
Showing 2 changed files with 155 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,23 @@
import org.apache.hc.core5.annotation.Contract;
import org.apache.hc.core5.annotation.ThreadingBehavior;
import org.apache.hc.core5.http.EntityDetails;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HeaderElements;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpRequestInterceptor;
import org.apache.hc.core5.http.HttpVersion;
import org.apache.hc.core5.http.ProtocolException;
import org.apache.hc.core5.http.ProtocolVersion;
import org.apache.hc.core5.util.Args;

/**
* This request interceptor is responsible for activation of the 'expect-continue'
* handshake by adding a {@code Expect} header describing client expectations.
* Additionally, it provides an option to respond with a 417 (Expectation Failed)
* status for unexpected {@code Expect} headers.
* <p>
* This interceptor is recommended for the HTTP protocol conformance and
* This interceptor is recommended for HTTP protocol conformance and
* the correct operation of the client-side message processing pipeline.
* </p>
*
Expand All @@ -54,31 +57,62 @@
@Contract(threading = ThreadingBehavior.IMMUTABLE)
public class RequestExpectContinue implements HttpRequestInterceptor {


/**
* Flag to control if a 417 (Expectation Failed) should be returned for unexpected Expect header.
*
* @since 5.3
*/
private final boolean shouldRespond417;

/**
* Creates an instance with custom behavior for "Expect" header.
*
* @param shouldRespond417 if true, will respond with 417 for unexpected "Expect" headers.
* @since 5.3
*/
public RequestExpectContinue(final boolean shouldRespond417) {
this.shouldRespond417 = shouldRespond417;
}

/**
* Singleton instance.
*
* @since 5.2
*/
public static final RequestExpectContinue INSTANCE = new RequestExpectContinue();

/**
* Creates a default instance which will not respond with a 417 status for unexpected "Expect" headers.
* <p>
* The flag {@code shouldRespond417} is set to false, meaning that this instance will not throw a ProtocolException
* resulting in a 417 (Expectation Failed) status code when encountering an "Expect" header value other than "100-continue".
* </p>
*
*/
public RequestExpectContinue() {
super();
this(false);
}

@Override
public void process(final HttpRequest request, final EntityDetails entity, final HttpContext context)
throws HttpException, IOException {
Args.notNull(request, "HTTP request");

if (!request.containsHeader(HttpHeaders.EXPECT)) {
final Header expectHeader = request.getFirstHeader(HttpHeaders.EXPECT);

if (expectHeader != null) {
final String expectValue = expectHeader.getValue();
if (!HeaderElements.CONTINUE.equalsIgnoreCase(expectValue) && shouldRespond417) {
throw new ProtocolException("417 Expectation Failed");
}
} else {
if (entity != null) {
final ProtocolVersion ver = context.getProtocolVersion();
// Do not send the expect header if request body is known to be empty
if (entity.getContentLength() != 0 && !ver.lessEquals(HttpVersion.HTTP_1_0)) {
request.addHeader(HttpHeaders.EXPECT, HeaderElements.CONTINUE);
}
}
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/*
* ====================================================================
* 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
* <http://www.apache.org/>.
*
*/

package org.apache.hc.core5.http.protocol;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import org.apache.hc.core5.http.EntityDetails;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HeaderElements;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.ProtocolException;
import org.apache.hc.core5.http.message.BasicHttpRequest;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

class TestRequestExpectContinue {

private HttpContext context;
private BasicHttpRequest request;
private EntityDetails entity;

@BeforeEach
public void setup() {
context = new BasicHttpContext();
request = new BasicHttpRequest("GET", "/");
entity = mock(EntityDetails.class);
}

@Test
public void testDefaultExpectHeaderAdded() throws Exception {
when(entity.getContentLength()).thenReturn(10L);

final RequestExpectContinue interceptor = new RequestExpectContinue(false);
interceptor.process(request, entity, context);

final Header expectHeader = request.getFirstHeader(HttpHeaders.EXPECT);
assert (expectHeader != null);
assert (expectHeader.getValue().equalsIgnoreCase(HeaderElements.CONTINUE));
}

@Test
public void testShouldRespond417() throws Exception {
request.addHeader(HttpHeaders.EXPECT, "random-value");
final RequestExpectContinue interceptor = new RequestExpectContinue(true);

try {
interceptor.process(request, entity, context);
} catch (final ProtocolException e) {
assert (e.getMessage().equals("417 Expectation Failed"));
}
}

@Test
public void testShouldNotRespond417() throws Exception {
request.addHeader(HttpHeaders.EXPECT, "random-value");
final RequestExpectContinue interceptor = new RequestExpectContinue(false);

try {
interceptor.process(request, entity, context);
} catch (final ProtocolException e) {
// Should not throw this exception
assert (false);
}
}

@Test
public void testExpectContinueHeaderAlreadyPresent() throws Exception {
request.addHeader(HttpHeaders.EXPECT, HeaderElements.CONTINUE);
when(entity.getContentLength()).thenReturn(10L);

final RequestExpectContinue interceptor = new RequestExpectContinue(false);
interceptor.process(request, entity, context);

final Header[] headers = request.getHeaders(HttpHeaders.EXPECT);
assert (headers.length == 1);
assert (headers[0].getValue().equalsIgnoreCase(HeaderElements.CONTINUE));
}

@Test
public void testContentLengthZero() throws Exception {
when(entity.getContentLength()).thenReturn(0L);

final RequestExpectContinue interceptor = new RequestExpectContinue(false);
interceptor.process(request, entity, context);

final Header expectHeader = request.getFirstHeader(HttpHeaders.EXPECT);
assert (expectHeader == null);
}
}

0 comments on commit 624c92f

Please sign in to comment.