Skip to content

Commit

Permalink
Immutable HttpHeaders (#1731)
Browse files Browse the repository at this point in the history 
Related: #1567
Motivation:

`HttpHeaders` is mutable. It is currently very easy for everyone to update the request or response headers a lot later than expected, causing various hard-to-debug problems.

Modifications:

- Replaced the current `HttpHeaders` API and implementations with the new one.
  - `HttpHeaders` has some type hierarchy now.
    - `HttpHeaders` as a generic HTTP header container.
      - `HttpHeaders` does not have short methods like `path()`, `method()` and `status()`.
        `RequestHeaders` and `ResponseHeaders` have them.
    - `RequestHeaders` as a request-specific HTTP header container.
    - `ResponseHeaders` as a response-specific HTTP header container.
  - There are builders now:
    - `HttpHeadersBuilder`
    - `RequestHeadersBuilder`
    - `ResponseHeadersBuilder`
- Changed the overall API related with specifying HTTP headers.
  - We now accept `CharSequence` instead of `AsciiString` as a header name, so that a user can specify a `String` as a header name.
  - We now accept `Iterable<? extends Map.Entry<...>>` instead of `Headers<...>` so that a user can specify wider range of types.
  - A header name is converted to an `AsciiString` internally.
  - Deprecated `HttpHeaders.EMPTY_HEADERS` in favor of `HttpHeaders.of()`.
- Added `RequestContext.updateRequest()` so that a decorator author can update the request when filtering a request.

Result:

- We provide more secure API for representing HTTP headers.
  • Loading branch information
@trustin
trustin authored May 9, 2019
1 parent 49ce1fb commit 92919ff
Show file tree
Hide file tree
Showing 263 changed files with 8,175 additions and 3,261 deletions.
11 changes: 7 additions & 4 deletions benchmarks/src/jmh/java/com/linecorp/armeria/common/HttpHeadersBenchmark.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,28 @@

package com.linecorp.armeria.common;

import javax.annotation.Nullable;

import org.openjdk.jmh.annotations.Benchmark;

/**
* Microbenchmarks of {@link DefaultHttpHeaders} construction.
*/
public class HttpHeadersBenchmark {

@Nullable
@Benchmark
public MediaType parseKnown() {
final HttpHeaders headers = new DefaultHttpHeaders()
.set(HttpHeaderNames.CONTENT_TYPE, "application/grpc+proto");
final HttpHeaders headers = HttpHeaders.of(HttpHeaderNames.CONTENT_TYPE, "application/grpc+proto");
return headers.contentType();
}

@Nullable
@Benchmark
public MediaType parseUnknown() {
final HttpHeaders headers = new DefaultHttpHeaders()
final HttpHeaders headers = HttpHeaders.of(
// Single letter change to keep theoretical parsing performance the same.
.set(HttpHeaderNames.CONTENT_TYPE, "application/grpc+oroto");
HttpHeaderNames.CONTENT_TYPE, "application/grpc+oroto");
return headers.contentType();
}
}
18 changes: 8 additions & 10 deletions core/src/main/java/com/linecorp/armeria/client/AbstractClientOptionsBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@
import java.util.Collection;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Map.Entry;
import java.util.function.Function;

import com.linecorp.armeria.common.DefaultHttpHeaders;
import com.linecorp.armeria.common.HttpHeaderNames;
import com.linecorp.armeria.common.HttpHeaders;
import com.linecorp.armeria.common.HttpHeadersBuilder;
import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.Request;
Expand All @@ -37,14 +38,11 @@
import com.linecorp.armeria.common.logging.ContentPreviewerFactory;
import com.linecorp.armeria.internal.ArmeriaHttpUtil;

import io.netty.handler.codec.Headers;
import io.netty.util.AsciiString;

class AbstractClientOptionsBuilder<B extends AbstractClientOptionsBuilder<B>> {

private final Map<ClientOption<?>, ClientOptionValue<?>> options = new LinkedHashMap<>();
private final ClientDecorationBuilder decoration = new ClientDecorationBuilder();
private final HttpHeaders httpHeaders = new DefaultHttpHeaders();
private final HttpHeadersBuilder httpHeaders = HttpHeaders.builder();

/**
* Creates a new instance with the default options.
Expand Down Expand Up @@ -399,9 +397,9 @@ public B addHttpHeader(CharSequence name, Object value) {
/**
* Adds the specified HTTP headers.
*/
public B addHttpHeaders(Headers<AsciiString, String, ?> httpHeaders) {
public B addHttpHeaders(Iterable<? extends Entry<? extends CharSequence, ?>> httpHeaders) {
requireNonNull(httpHeaders, "httpHeaders");
this.httpHeaders.add(httpHeaders);
this.httpHeaders.addObject(httpHeaders);
return self();
}

Expand All @@ -418,9 +416,9 @@ public B setHttpHeader(CharSequence name, Object value) {
/**
* Sets the specified HTTP headers.
*/
public B setHttpHeaders(Headers<AsciiString, String, ?> httpHeaders) {
public B setHttpHeaders(Iterable<? extends Entry<? extends CharSequence, ?>> httpHeaders) {
requireNonNull(httpHeaders, "httpHeaders");
this.httpHeaders.setAll(httpHeaders);
this.httpHeaders.setObject(httpHeaders);
return self();
}

Expand All @@ -429,7 +427,7 @@ ClientOptions buildOptions() {
final int numOpts = optVals.size();
final ClientOptionValue<?>[] optValArray = optVals.toArray(new ClientOptionValue[numOpts + 2]);
optValArray[numOpts] = ClientOption.DECORATION.newValue(decoration.build());
optValArray[numOpts + 1] = ClientOption.HTTP_HEADERS.newValue(httpHeaders);
optValArray[numOpts + 1] = ClientOption.HTTP_HEADERS.newValue(httpHeaders.build());

return ClientOptions.of(optValArray);
}
Expand Down
7 changes: 3 additions & 4 deletions core/src/main/java/com/linecorp/armeria/client/ClientOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.util.Optional;
import java.util.function.Function;

import com.linecorp.armeria.common.DefaultHttpHeaders;
import com.linecorp.armeria.common.Flags;
import com.linecorp.armeria.common.HttpHeaderNames;
import com.linecorp.armeria.common.HttpHeaders;
Expand Down Expand Up @@ -73,7 +72,7 @@ public final class ClientOptions extends AbstractOptions {
RESPONSE_TIMEOUT_MILLIS.newValue(Flags.defaultResponseTimeoutMillis()),
MAX_RESPONSE_LENGTH.newValue(Flags.defaultMaxResponseLength()),
DECORATION.newValue(ClientDecoration.NONE),
HTTP_HEADERS.newValue(HttpHeaders.EMPTY_HEADERS)
HTTP_HEADERS.newValue(HttpHeaders.of())
};

/**
Expand Down Expand Up @@ -163,7 +162,7 @@ private static HttpHeaders filterHttpHeaders(HttpHeaders headers) {
}
}

return new DefaultHttpHeaders().add(headers).asImmutable();
return headers;
}

private ClientOptions(ClientOptionValue<?>... options) {
Expand Down Expand Up @@ -273,7 +272,7 @@ public ClientDecoration decoration() {
* {@link SessionProtocol} is HTTP.
*/
public HttpHeaders httpHeaders() {
return getOrElse(HTTP_HEADERS, HttpHeaders.EMPTY_HEADERS);
return getOrElse(HTTP_HEADERS, HttpHeaders.of());
}

public ContentPreviewerFactory requestContentPreviewerFactory() {
Expand Down
21 changes: 9 additions & 12 deletions core/src/main/java/com/linecorp/armeria/client/ClientRequestContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import java.net.URI;
import java.time.Duration;
import java.util.Map.Entry;

import javax.annotation.Nullable;

Expand All @@ -31,9 +32,6 @@
import com.linecorp.armeria.common.Response;
import com.linecorp.armeria.common.RpcRequest;

import io.netty.handler.codec.Headers;
import io.netty.util.AsciiString;

/**
* Provides information about a {@link Request}, its {@link Response} and its related utilities.
* Every client request has its own {@link ClientRequestContext} instance.
Expand Down Expand Up @@ -157,8 +155,7 @@ static ClientRequestContext of(RpcRequest request, URI uri) {
void setMaxResponseLength(long maxResponseLength);

/**
* Returns an immutable {@link HttpHeaders} which is included when a {@link Client} sends an
* {@link HttpRequest}.
* Returns an {@link HttpHeaders} which is included when a {@link Client} sends an {@link HttpRequest}.
*/
HttpHeaders additionalRequestHeaders();

Expand All @@ -167,30 +164,30 @@ static ClientRequestContext of(RpcRequest request, URI uri) {
* associated with the specified {@code name}.
* The header will be included when a {@link Client} sends an {@link HttpRequest}.
*/
void setAdditionalRequestHeader(AsciiString name, String value);
void setAdditionalRequestHeader(CharSequence name, Object value);

/**
* Clears the current header and sets the specified {@link Headers} which is included when a
* Clears the current header and sets the specified {@link HttpHeaders} which is included when a
* {@link Client} sends an {@link HttpRequest}.
*/
void setAdditionalRequestHeaders(Headers<? extends AsciiString, ? extends String, ?> headers);
void setAdditionalRequestHeaders(Iterable<? extends Entry<? extends CharSequence, ?>> headers);

/**
* Adds a header with the specified {@code name} and {@code value}. The header will be included when
* a {@link Client} sends an {@link HttpRequest}.
*/
void addAdditionalRequestHeader(AsciiString name, String value);
void addAdditionalRequestHeader(CharSequence name, Object value);

/**
* Adds the specified {@link Headers} which is included when a {@link Client} sends an
* Adds the specified {@link HttpHeaders} which is included when a {@link Client} sends an
* {@link HttpRequest}.
*/
void addAdditionalRequestHeaders(Headers<? extends AsciiString, ? extends String, ?> headers);
void addAdditionalRequestHeaders(Iterable<? extends Entry<? extends CharSequence, ?>> headers);

/**
* Removes all headers with the specified {@code name}.
*
* @return {@code true} if at least one entry has been removed
*/
boolean removeAdditionalRequestHeader(AsciiString name);
boolean removeAdditionalRequestHeader(CharSequence name);
}
14 changes: 6 additions & 8 deletions core/src/main/java/com/linecorp/armeria/client/ClientRequestContextWrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@
package com.linecorp.armeria.client;

import java.time.Duration;
import java.util.Map.Entry;

import com.linecorp.armeria.common.HttpHeaders;
import com.linecorp.armeria.common.Request;
import com.linecorp.armeria.common.RequestContextWrapper;
import com.linecorp.armeria.server.ServiceRequestContext;

import io.netty.handler.codec.Headers;
import io.netty.util.AsciiString;

/**
* Wraps an existing {@link ServiceRequestContext}.
*/
Expand Down Expand Up @@ -110,27 +108,27 @@ public HttpHeaders additionalRequestHeaders() {
}

@Override
public void setAdditionalRequestHeader(AsciiString name, String value) {
public void setAdditionalRequestHeader(CharSequence name, Object value) {
delegate().setAdditionalRequestHeader(name, value);
}

@Override
public void setAdditionalRequestHeaders(Headers<? extends AsciiString, ? extends String, ?> headers) {
public void setAdditionalRequestHeaders(Iterable<? extends Entry<? extends CharSequence, ?>> headers) {
delegate().setAdditionalRequestHeaders(headers);
}

@Override
public void addAdditionalRequestHeader(AsciiString name, String value) {
public void addAdditionalRequestHeader(CharSequence name, Object value) {
delegate().addAdditionalRequestHeader(name, value);
}

@Override
public void addAdditionalRequestHeaders(Headers<? extends AsciiString, ? extends String, ?> headers) {
public void addAdditionalRequestHeaders(Iterable<? extends Entry<? extends CharSequence, ?>> headers) {
delegate().setAdditionalRequestHeaders(headers);
}

@Override
public boolean removeAdditionalRequestHeader(AsciiString name) {
public boolean removeAdditionalRequestHeader(CharSequence name) {
return delegate().removeAdditionalRequestHeader(name);
}
}
81 changes: 59 additions & 22 deletions core/src/main/java/com/linecorp/armeria/client/Clients.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,9 @@
import java.util.function.Consumer;
import java.util.function.Function;

import com.linecorp.armeria.common.DefaultHttpHeaders;
import com.linecorp.armeria.common.HttpHeaders;
import com.linecorp.armeria.common.util.SafeCloseable;

import io.netty.util.AsciiString;

/**
* Creates a new client that connects to a specified {@link URI}.
* If you are creating an {@link HttpClient}, it is recommended to use the factory methods in
Expand Down Expand Up @@ -262,7 +259,7 @@ private static ClientBuilderParams builderParams(Object client) {

/**
* Sets the specified HTTP header in a thread-local variable so that the header is sent by the client call
* made from the current thread. Use the `try-with-resources` block with the returned
* made from the current thread. Use the {@code try-with-resources} block with the returned
* {@link SafeCloseable} to unset the thread-local variable automatically:
* <pre>{@code
* import static com.linecorp.armeria.common.HttpHeaderNames.AUTHORIZATION;
Expand All @@ -288,23 +285,61 @@ private static ClientBuilderParams builderParams(Object client) {
*
* @see #withHttpHeaders(Function)
*/
public static SafeCloseable withHttpHeader(AsciiString name, String value) {
public static SafeCloseable withHttpHeader(CharSequence name, String value) {
requireNonNull(name, "name");
requireNonNull(value, "value");
return withHttpHeaders(headers -> headers.set(name, value));
return withHttpHeaders(headers -> headers.toBuilder().set(name, value).build());
}

/**
* Sets the specified HTTP header in a thread-local variable so that the header is sent by the client call
* made from the current thread. Use the {@code try-with-resources} block with the returned
* {@link SafeCloseable} to unset the thread-local variable automatically:
* <pre>{@code
* import static com.linecorp.armeria.common.HttpHeaderNames.CONTENT_TYPE;
* import static com.linecorp.armeria.common.MediaType.JSON_UTF_8;
*
* try (SafeCloseable ignored = withHttpHeader(CONTENT_TYPE, JSON_UTF_8)) {
* client.executeSomething(..);
* }
* }</pre>
* You can also nest the header manipulation:
* <pre>{@code
* import static com.linecorp.armeria.common.HttpHeaderNames.AUTHORIZATION;
* import static com.linecorp.armeria.common.HttpHeaderNames.CONTENT_TYPE;
* import static com.linecorp.armeria.common.MediaType.JSON_UTF_8;
*
* try (SafeCloseable ignored = withHttpHeader(CONTENT_TYPE, JSON_UTF_8)) {
* for (String secret : secrets) {
* try (SafeCloseable ignored2 = withHttpHeader(AUTHORIZATION, secret)) {
* // Both CONTENT_TYPE and AUTHORIZATION will be set.
* client.executeSomething(..);
* }
* }
* }
* }</pre>
*
* @see #withHttpHeaders(Function)
*/
public static SafeCloseable withHttpHeader(CharSequence name, Object value) {
requireNonNull(name, "name");
requireNonNull(value, "value");
return withHttpHeaders(headers -> headers.toBuilder().setObject(name, value).build());
}

/**
* Sets the specified HTTP header manipulating function in a thread-local variable so that the manipulated
* headers are sent by the client call made from the current thread. Use the `try-with-resources` block
* with the returned {@link SafeCloseable} to unset the thread-local variable automatically:
* headers are sent by the client call made from the current thread. Use the {@code try-with-resources}
* block with the returned {@link SafeCloseable} to unset the thread-local variable automatically:
* <pre>{@code
* import static com.linecorp.armeria.common.HttpHeaderNames.AUTHORIZATION;
* import static com.linecorp.armeria.common.HttpHeaderNames.USER_AGENT;
*
* try (SafeCloseable ignored = withHttpHeaders(headers -> {
* headers.set(HttpHeaders.AUTHORIZATION, myCredential)
* .set(HttpHeaders.USER_AGENT, myAgent);
* return headers.toBuilder()
* .set(HttpHeaders.AUTHORIZATION, myCredential)
* .set(HttpHeaders.USER_AGENT, myAgent)
* .build();
* })) {
* client.executeSomething(..);
* }
Expand All @@ -314,37 +349,39 @@ public static SafeCloseable withHttpHeader(AsciiString name, String value) {
* import static com.linecorp.armeria.common.HttpHeaderNames.AUTHORIZATION;
* import static com.linecorp.armeria.common.HttpHeaderNames.USER_AGENT;
*
* try (SafeCloseable ignored = withHttpHeaders(h -> h.set(USER_AGENT, myAgent))) {
* try (SafeCloseable ignored = withHttpHeaders(h -> {
* return h.toBuilder()
* .set(USER_AGENT, myAgent)
* .build();
* })) {
* for (String secret : secrets) {
* try (SafeCloseable ignored2 = withHttpHeaders(h -> h.set(AUTHORIZATION, secret))) {
* try (SafeCloseable ignored2 = withHttpHeaders(h -> {
* return h.toBuilder()
* .set(AUTHORIZATION, secret)
* .build();
* })) {
* // Both USER_AGENT and AUTHORIZATION will be set.
* client.executeSomething(..);
* }
* }
* }
* }</pre>
*
* @see #withHttpHeader(AsciiString, String)
* @see #withHttpHeader(CharSequence, String)
*/
public static SafeCloseable withHttpHeaders(Function<HttpHeaders, HttpHeaders> headerManipulator) {
requireNonNull(headerManipulator, "headerManipulator");
return withContextCustomizer(ctx -> {
final HttpHeaders additionalHeaders = ctx.additionalRequestHeaders();
final DefaultHttpHeaders headers = new DefaultHttpHeaders();
if (!additionalHeaders.isEmpty()) {
headers.set(additionalHeaders);
}

final HttpHeaders manipulatedHeaders = headerManipulator.apply(headers);
final HttpHeaders manipulatedHeaders = headerManipulator.apply(ctx.additionalRequestHeaders());
ctx.setAdditionalRequestHeaders(manipulatedHeaders);
});
}

/**
* Sets the specified {@link ClientRequestContext} customization function in a thread-local variable so that
* the customized context is used when the client invokes a request from the current thread. Use the
* `try-with-resources` block with the returned {@link SafeCloseable} to unset the thread-local variable
* automatically:
* {@code try-with-resources} block with the returned {@link SafeCloseable} to unset the thread-local
* variable automatically:
* <pre>{@code
* try (SafeCloseable ignored = withContextCustomizer(ctx -> {
* ctx.attr(USER_ID).set(userId);
Expand Down
Loading

0 comments on commit 92919ff

Please sign in to comment.