Skip to content

Commit

Permalink
Allow chunked responses without payload body (#1060)
Browse files Browse the repository at this point in the history
Motivation:

Some older implementations may use connection closures as a marker of the
end of HTTP/1.1 messages if the receiver did not receive any part of the payload
body before the connection closure.

Modifications:

- Introduce `H1SpecExceptions` that allow configuring special exceptions
for HTTP/1.1 specification on `H1ProtocolConfig`;
- Add a `H1SpecExceptions#allowPrematureClosureBeforePayloadBody()` option;
- Add tests to verify that the new exception works as expected on the client side;

Result:

`HttpObjectDecoder` marks the chunked response as complete if server closes
the connection right after the headers.
  • Loading branch information
idelpivnitskiy authored May 21, 2020
1 parent 1a7498d commit 97f0d60
Show file tree
Hide file tree
Showing 10 changed files with 556 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Copyright © 2020 Apple Inc. and the ServiceTalk project authors
*
* Licensed 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.
*/
package io.servicetalk.http.api;

import org.hamcrest.Description;
import org.hamcrest.Matcher;
import org.hamcrest.TypeSafeMatcher;

import static io.servicetalk.http.api.CharSequences.contentEquals;
import static java.util.Objects.requireNonNull;

/**
* Custom {@link Matcher}s specific to http-api.
*/
public final class Matchers {

private Matchers() {
// No instances
}

/**
* {@link Matcher} representation for {@link CharSequences#contentEquals(CharSequence, CharSequence)}.
*
* @param expected expected {@link CharSequence} value
* @return a {@link Matcher} to verify content equality of two {@link CharSequence}s
*/
public static Matcher<CharSequence> contentEqualTo(final CharSequence expected) {
requireNonNull(expected);
return new TypeSafeMatcher<CharSequence>() {

@Override
protected boolean matchesSafely(final CharSequence item) {
if (item == null) {
return false;
}
return contentEquals(expected, item);
}

@Override
public void describeTo(final Description description) {
description.appendValue(expected);
}
};
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2019 Apple Inc. and the ServiceTalk project authors
* Copyright © 2019-2020 Apple Inc. and the ServiceTalk project authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -87,4 +87,11 @@ default String alpnId() {
* <a href="https://tools.ietf.org/html/rfc7230#section-4.1.2trailers">trailer fields</a>
*/
int trailersEncodedSizeEstimate();

/**
* Additional exceptions for <a href="https://tools.ietf.org/html/rfc7230">HTTP/1.1</a> specification.
*
* @return exceptions for <a href="https://tools.ietf.org/html/rfc7230">HTTP/1.1</a> specification
*/
H1SpecExceptions specExceptions();
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2019 Apple Inc. and the ServiceTalk project authors
* Copyright © 2019-2020 Apple Inc. and the ServiceTalk project authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -29,12 +29,15 @@
*/
public final class H1ProtocolConfigBuilder {

private static final H1SpecExceptions DEFAULT_H1_SPEC_EXCEPTIONS = new H1SpecExceptions.Builder().build();

private int maxPipelinedRequests = 1;
private int maxStartLineLength = 4096;
private int maxHeaderFieldLength = 8192;
private HttpHeadersFactory headersFactory = DefaultHttpHeadersFactory.INSTANCE;
private int headersEncodedSizeEstimate = 256;
private int trailersEncodedSizeEstimate = 256;
private H1SpecExceptions specExceptions = DEFAULT_H1_SPEC_EXCEPTIONS;

H1ProtocolConfigBuilder() {
}
Expand Down Expand Up @@ -130,14 +133,25 @@ public H1ProtocolConfigBuilder trailersEncodedSizeEstimate(final int trailersEnc
return this;
}

/**
* Sets additional exceptions for <a href="https://tools.ietf.org/html/rfc7230">HTTP/1.1</a> specification.
*
* @param specExceptions exceptions for <a href="https://tools.ietf.org/html/rfc7230">HTTP/1.1</a> specification
* @return {@code this}
*/
public H1ProtocolConfigBuilder specExceptions(final H1SpecExceptions specExceptions) {
this.specExceptions = requireNonNull(specExceptions);
return this;
}

/**
* Builds {@link H1ProtocolConfig}.
*
* @return a new {@link H1ProtocolConfig}
*/
public H1ProtocolConfig build() {
return new DefaultH1ProtocolConfig(headersFactory, maxPipelinedRequests, maxStartLineLength,
maxHeaderFieldLength, headersEncodedSizeEstimate, trailersEncodedSizeEstimate);
maxHeaderFieldLength, headersEncodedSizeEstimate, trailersEncodedSizeEstimate, specExceptions);
}

private static final class DefaultH1ProtocolConfig implements H1ProtocolConfig {
Expand All @@ -148,16 +162,19 @@ private static final class DefaultH1ProtocolConfig implements H1ProtocolConfig {
private final int maxHeaderFieldLength;
private final int headersEncodedSizeEstimate;
private final int trailersEncodedSizeEstimate;
private final H1SpecExceptions specExceptions;

DefaultH1ProtocolConfig(final HttpHeadersFactory headersFactory, final int maxPipelinedRequests,
final int maxStartLineLength, final int maxHeaderFieldLength,
final int headersEncodedSizeEstimate, final int trailersEncodedSizeEstimate) {
final int headersEncodedSizeEstimate, final int trailersEncodedSizeEstimate,
final H1SpecExceptions specExceptions) {
this.headersFactory = headersFactory;
this.maxPipelinedRequests = maxPipelinedRequests;
this.maxStartLineLength = maxStartLineLength;
this.maxHeaderFieldLength = maxHeaderFieldLength;
this.headersEncodedSizeEstimate = headersEncodedSizeEstimate;
this.trailersEncodedSizeEstimate = trailersEncodedSizeEstimate;
this.specExceptions = specExceptions;
}

@Override
Expand Down Expand Up @@ -189,5 +206,10 @@ public int headersEncodedSizeEstimate() {
public int trailersEncodedSizeEstimate() {
return trailersEncodedSizeEstimate;
}

@Override
public H1SpecExceptions specExceptions() {
return specExceptions;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright © 2020 Apple Inc. and the ServiceTalk project authors
*
* Licensed 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.
*/
package io.servicetalk.http.netty;

/**
* Additional exceptions for <a href="https://tools.ietf.org/html/rfc7230">HTTP/1.1</a> specification.
*/
public final class H1SpecExceptions {

private final boolean allowPrematureClosureBeforePayloadBody;

private H1SpecExceptions(final boolean allowPrematureClosureBeforePayloadBody) {
this.allowPrematureClosureBeforePayloadBody = allowPrematureClosureBeforePayloadBody;
}

/**
* Allows interpreting connection closures as the end of HTTP/1.1 messages if the receiver did not receive any part
* of the payload body before the connection closure.
*
* @return {@code true} if the receiver should interpret connection closures as the end of HTTP/1.1 messages if it
* did not receive any part of the payload body before the connection closure
*/
public boolean allowPrematureClosureBeforePayloadBody() {
return allowPrematureClosureBeforePayloadBody;
}

/**
* Builder for {@link H1SpecExceptions}.
*/
public static final class Builder {

private boolean allowPrematureClosureBeforePayloadBody;

/**
* Allows interpreting connection closures as the end of HTTP/1.1 messages if the receiver did not receive any
* part of the payload body before the connection closure.
*
* @return {@code this}
*/
public Builder allowPrematureClosureBeforePayloadBody() {
this.allowPrematureClosureBeforePayloadBody = true;
return this;
}

/**
* Builds {@link H1SpecExceptions}.
*
* @return a new {@link H1SpecExceptions}
*/
public H1SpecExceptions build() {
return new H1SpecExceptions(allowPrematureClosureBeforePayloadBody);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ final class HttpClientChannelInitializer implements ChannelInitializer {
final Queue<HttpRequestMethod> methodQueue = new ArrayDeque<>(min(8, config.maxPipelinedRequests()));
final ChannelPipeline pipeline = channel.pipeline();
pipeline.addLast(new HttpResponseDecoder(methodQueue, alloc, config.headersFactory(),
config.maxStartLineLength(), config.maxHeaderFieldLength(), closeHandler));
config.maxStartLineLength(), config.maxHeaderFieldLength(),
config.specExceptions().allowPrematureClosureBeforePayloadBody(), closeHandler));
pipeline.addLast(new HttpRequestEncoder(methodQueue,
config.headersEncodedSizeEstimate(), config.trailersEncodedSizeEstimate(), closeHandler));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ abstract class HttpObjectDecoder<T extends HttpMetaData> extends ByteToMessageDe

private final HttpHeadersFactory headersFactory;
private final CloseHandler closeHandler;
private final boolean allowPrematureClosureBeforePayloadBody;
@Nullable
private T message;
@Nullable
Expand Down Expand Up @@ -158,7 +159,7 @@ private enum State {
*/
protected HttpObjectDecoder(final ByteBufAllocator alloc, final HttpHeadersFactory headersFactory,
final int maxStartLineLength, final int maxHeaderFieldLength,
final CloseHandler closeHandler) {
final boolean allowPrematureClosureBeforePayloadBody, final CloseHandler closeHandler) {
super(alloc);
this.closeHandler = closeHandler;
if (maxStartLineLength <= 0) {
Expand All @@ -170,6 +171,7 @@ protected HttpObjectDecoder(final ByteBufAllocator alloc, final HttpHeadersFacto
this.headersFactory = requireNonNull(headersFactory);
this.maxStartLineLength = maxStartLineLength;
this.maxHeaderFieldLength = maxHeaderFieldLength;
this.allowPrematureClosureBeforePayloadBody = allowPrematureClosureBeforePayloadBody;
}

final HttpHeadersFactory headersFactory() {
Expand Down Expand Up @@ -448,7 +450,9 @@ protected final void decodeLast(final ChannelHandlerContext ctx, final ByteBuf i
// Handle the last unfinished message.
if (message != null) {
boolean chunked = isTransferEncodingChunked(message.headers());
if (currentState == State.READ_VARIABLE_LENGTH_CONTENT && !in.isReadable() && !chunked) {
if (!in.isReadable() && (
(currentState == State.READ_VARIABLE_LENGTH_CONTENT && !chunked) ||
(currentState == State.READ_CHUNK_SIZE && chunked && allowPrematureClosureBeforePayloadBody))) {
// End of connection.
ctx.fireChannelRead(EmptyHttpHeaders.INSTANCE);
closeHandler.protocolPayloadEndInbound(ctx);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,15 @@ final class HttpRequestDecoder extends HttpObjectDecoder<HttpRequestMetaData> {
final HttpHeadersFactory headersFactory, final int maxStartLineLength,
final int maxHeaderFieldLength) {
this(methodQueue, alloc, headersFactory, maxStartLineLength, maxHeaderFieldLength,
UNSUPPORTED_PROTOCOL_CLOSE_HANDLER);
false, UNSUPPORTED_PROTOCOL_CLOSE_HANDLER);
}

HttpRequestDecoder(final Queue<HttpRequestMethod> methodQueue, final ByteBufAllocator alloc,
final HttpHeadersFactory headersFactory, final int maxStartLineLength,
final int maxHeaderFieldLength, final CloseHandler closeHandler) {
super(alloc, headersFactory, maxStartLineLength, maxHeaderFieldLength, closeHandler);
final int maxHeaderFieldLength, final boolean allowPrematureClosureBeforePayloadBody,
final CloseHandler closeHandler) {
super(alloc, headersFactory, maxStartLineLength, maxHeaderFieldLength, allowPrematureClosureBeforePayloadBody,
closeHandler);
this.methodQueue = requireNonNull(methodQueue);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,14 @@ final class HttpResponseDecoder extends HttpObjectDecoder<HttpResponseMetaData>
HttpResponseDecoder(final Queue<HttpRequestMethod> methodQueue, final ByteBufAllocator alloc,
final HttpHeadersFactory headersFactory, int maxStartLineLength, int maxHeaderFieldLength) {
this(methodQueue, alloc, headersFactory, maxStartLineLength, maxHeaderFieldLength,
UNSUPPORTED_PROTOCOL_CLOSE_HANDLER);
false, UNSUPPORTED_PROTOCOL_CLOSE_HANDLER);
}

HttpResponseDecoder(final Queue<HttpRequestMethod> methodQueue, final ByteBufAllocator alloc,
final HttpHeadersFactory headersFactory, final int maxStartLineLength, int maxHeaderFieldLength,
final CloseHandler closeHandler) {
super(alloc, headersFactory, maxStartLineLength, maxHeaderFieldLength, closeHandler);
final boolean allowPrematureClosureBeforePayloadBody, final CloseHandler closeHandler) {
super(alloc, headersFactory, maxStartLineLength, maxHeaderFieldLength, allowPrematureClosureBeforePayloadBody,
closeHandler);
this.methodQueue = requireNonNull(methodQueue);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ private static ChannelInitializer getChannelInitializer(final ByteBufAllocator a
Queue<HttpRequestMethod> methodQueue = new ArrayDeque<>(2);
final ChannelPipeline pipeline = channel.pipeline();
pipeline.addLast(new HttpRequestDecoder(methodQueue, alloc, config.headersFactory(),
config.maxStartLineLength(), config.maxHeaderFieldLength(), closeHandler));
config.maxStartLineLength(), config.maxHeaderFieldLength(),
config.specExceptions().allowPrematureClosureBeforePayloadBody(), closeHandler));
pipeline.addLast(new HttpResponseEncoder(methodQueue, config.headersEncodedSizeEstimate(),
config.trailersEncodedSizeEstimate(), closeHandler));
});
Expand Down
Loading

0 comments on commit 97f0d60

Please sign in to comment.