Skip to content

Commit

Permalink
GitHub#454 Sanitise HTTP message logs (#457)
Browse files Browse the repository at this point in the history
Feature to hide cookies and headers by in the request logs.  By default requests and responses will not include headers in their toString() methods.  HttpRequestMessageLogger sanitises the headers using the configuration options 'hideHeaders' and 'hideCookies'.
  • Loading branch information
OwenLindsell authored Sep 27, 2019
1 parent caface2 commit ed1b6fd
Show file tree
Hide file tree
Showing 48 changed files with 910 additions and 366 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (C) 2013-2018 Expedia Inc.
Copyright (C) 2013-2019 Expedia Inc.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (C) 2013-2018 Expedia Inc.
Copyright (C) 2013-2019 Expedia Inc.
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 @@ -27,7 +27,6 @@
import java.util.Set;
import java.util.function.Predicate;

import static com.google.common.base.Objects.toStringHelper;
import static com.google.common.base.Preconditions.checkArgument;
import static com.hotels.styx.api.HttpHeaderNames.CONNECTION;
import static com.hotels.styx.api.HttpHeaderNames.CONTENT_LENGTH;
Expand Down Expand Up @@ -346,13 +345,10 @@ public Optional<RequestCookie> cookie(String name) {

@Override
public String toString() {
return toStringHelper(this)
.add("version", version)
.add("method", method)
.add("uri", url)
.add("headers", headers)
.add("id", id)
.toString();
return "{version=" + version
+ ", method=" + method
+ ", uri=" + url
+ ", id=" + id + "}";
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (C) 2013-2018 Expedia Inc.
Copyright (C) 2013-2019 Expedia Inc.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -26,7 +26,6 @@
import java.util.Set;
import java.util.function.Predicate;

import static com.google.common.base.Objects.toStringHelper;
import static com.google.common.base.Preconditions.checkArgument;
import static com.hotels.styx.api.HttpHeaderNames.CONTENT_LENGTH;
import static com.hotels.styx.api.HttpHeaderNames.SET_COOKIE;
Expand Down Expand Up @@ -211,11 +210,8 @@ public Optional<ResponseCookie> cookie(String name) {

@Override
public String toString() {
return toStringHelper(this)
.add("version", version)
.add("status", status)
.add("headers", headers)
.toString();
return "{version=" + version
+ ", status=" + status + "}";
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import java.util.function.Function;
import java.util.function.Predicate;

import static com.google.common.base.Objects.toStringHelper;
import static com.google.common.base.Preconditions.checkArgument;
import static com.hotels.styx.api.HttpHeaderNames.CONNECTION;
import static com.hotels.styx.api.HttpHeaderNames.CONTENT_LENGTH;
Expand Down Expand Up @@ -385,13 +384,10 @@ public Optional<RequestCookie> cookie(String name) {

@Override
public String toString() {
return toStringHelper(this)
.add("version", version)
.add("method", method)
.add("uri", url)
.add("headers", headers)
.add("id", id)
.toString();
return "{version=" + version
+ ", method=" + method
+ ", uri=" + url
+ ", id=" + id + "}";
}

private interface BuilderTransformer {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (C) 2013-2018 Expedia Inc.
Copyright (C) 2013-2019 Expedia Inc.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -26,7 +26,6 @@
import java.util.function.Function;
import java.util.function.Predicate;

import static com.google.common.base.Objects.toStringHelper;
import static com.google.common.base.Preconditions.checkArgument;
import static com.hotels.styx.api.HttpHeaderNames.CONTENT_LENGTH;
import static com.hotels.styx.api.HttpHeaderNames.SET_COOKIE;
Expand Down Expand Up @@ -232,11 +231,8 @@ public Optional<ResponseCookie> cookie(String name) {

@Override
public String toString() {
return toStringHelper(this)
.add("version", version)
.add("status", status)
.add("headers", headers)
.toString();
return "{version=" + version
+ ", status=" + status + "}";
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (C) 2013-2018 Expedia Inc.
Copyright (C) 2013-2019 Expedia Inc.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (C) 2013-2018 Expedia Inc.
Copyright (C) 2013-2019 Expedia Inc.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -23,16 +23,16 @@

import java.util.Optional;

import static com.hotels.styx.api.HttpRequest.get;
import static com.hotels.styx.api.HttpRequest.patch;
import static com.hotels.styx.api.HttpRequest.put;
import static com.hotels.styx.api.HttpHeader.header;
import static com.hotels.styx.api.HttpHeaderNames.CONTENT_LENGTH;
import static com.hotels.styx.api.HttpHeaderNames.COOKIE;
import static com.hotels.styx.api.HttpHeaderNames.HOST;
import static com.hotels.styx.api.HttpMethod.DELETE;
import static com.hotels.styx.api.HttpMethod.GET;
import static com.hotels.styx.api.HttpMethod.POST;
import static com.hotels.styx.api.HttpRequest.get;
import static com.hotels.styx.api.HttpRequest.patch;
import static com.hotels.styx.api.HttpRequest.put;
import static com.hotels.styx.api.HttpVersion.HTTP_1_0;
import static com.hotels.styx.api.HttpVersion.HTTP_1_1;
import static com.hotels.styx.api.RequestCookie.requestCookie;
Expand Down Expand Up @@ -131,10 +131,8 @@ public void canUseBuilderToSetRequestProperties() {
.cookies(requestCookie("cfoo", "bar"))
.build();

assertThat(request.toString(), is("HttpRequest{version=HTTP/1.1, method=PATCH, uri=https://hotels.com, " +
"headers=[headerName=a, Cookie=cfoo=bar, Host=hotels.com], id=id}"));

assertThat(request.headers("headerName"), is(singletonList("a")));
assertThat(request.toString(), is("{version=HTTP/1.1, method=PATCH, uri=https://hotels.com, id=id}"));
assertThat(request.headers().toString(), is("[headerName=a, Cookie=cfoo=bar, Host=hotels.com]"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (C) 2013-2018 Expedia Inc.
Copyright (C) 2013-2019 Expedia Inc.
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 @@ -137,9 +137,8 @@ public void canUseBuilderToSetRequestProperties() {
.cookies(requestCookie("cfoo", "bar"))
.build();

assertThat(request.toString(), is("LiveHttpRequest{version=HTTP/1.0, method=PATCH, uri=https://hotels.com, headers=[headerName=a, Cookie=cfoo=bar, Host=hotels.com], id=id}"));

assertThat(request.headers("headerName"), is(singletonList("a")));
assertThat(request.toString(), is("{version=HTTP/1.0, method=PATCH, uri=https://hotels.com, id=id}"));
assertThat(request.headers().toString(), is("[headerName=a, Cookie=cfoo=bar, Host=hotels.com]"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
import com.hotels.styx.api.metrics.codahale.CodaHaleMetricRegistry;
import com.hotels.styx.client.OriginStatsFactory.CachingOriginStatsFactory;
import com.hotels.styx.client.netty.connectionpool.HttpRequestOperation;
import com.hotels.styx.common.format.DefaultHttpMessageFormatter;
import com.hotels.styx.common.format.HttpMessageFormatter;

import static java.util.Objects.requireNonNull;

/**
* A Factory for creating an HttpRequestOperation from an LiveHttpRequest.
Expand All @@ -41,6 +45,7 @@ class Builder {
boolean flowControlEnabled;
boolean requestLoggingEnabled;
boolean longFormat;
HttpMessageFormatter httpMessageFormatter = new DefaultHttpMessageFormatter();

public static Builder httpRequestOperationFactoryBuilder() {
return new Builder();
Expand Down Expand Up @@ -71,13 +76,19 @@ public Builder longFormat(boolean longFormat) {
return this;
}

public Builder httpMessageFormatter(HttpMessageFormatter httpMessageFormatter) {
this.httpMessageFormatter = requireNonNull(httpMessageFormatter);
return this;
}

public HttpRequestOperationFactory build() {
return request -> new HttpRequestOperation(
request,
originStatsFactory,
responseTimeoutMillis,
requestLoggingEnabled,
longFormat);
longFormat,
httpMessageFormatter);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,9 @@ private static String hosts(Iterable<RemoteHost> origins) {
}
}

private static void logError(LiveHttpRequest rewrittenRequest, Throwable throwable) {
private static void logError(LiveHttpRequest request, Throwable throwable) {
LOGGER.error("Error Handling request={} exceptionClass={} exceptionMessage=\"{}\"",
new Object[]{rewrittenRequest, throwable.getClass().getName(), throwable.getMessage()});
new Object[]{request, throwable.getClass().getName(), throwable.getMessage()});
}

private LiveHttpResponse removeUnexpectedResponseBody(LiveHttpRequest request, LiveHttpResponse response) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.hotels.styx.api.Url;
import com.hotels.styx.api.extension.Origin;
import com.hotels.styx.api.extension.service.TlsSettings;
import com.hotels.styx.client.netty.connectionpool.HttpRequestOperation;
import com.hotels.styx.client.netty.connectionpool.NettyConnectionFactory;
import com.hotels.styx.client.ssl.SslContextFactory;
import io.netty.handler.ssl.SslContext;
Expand All @@ -40,6 +39,7 @@
import static com.hotels.styx.api.HttpHeaderNames.USER_AGENT;
import static com.hotels.styx.api.extension.Origin.newOriginBuilder;
import static com.hotels.styx.client.HttpConfig.newHttpConfigBuilder;
import static com.hotels.styx.client.HttpRequestOperationFactory.Builder.httpRequestOperationFactoryBuilder;
import static java.util.Objects.requireNonNull;

/**
Expand Down Expand Up @@ -316,15 +316,13 @@ Builder copy() {
* @return a new instance
*/
public StyxHttpClient build() {

NettyConnectionFactory connectionFactory = new NettyConnectionFactory.Builder()
.httpConfig(newHttpConfigBuilder().setMaxHeadersSize(maxHeaderSize).build())
.tlsSettings(tlsSettings)
.httpRequestOperationFactory(request -> new HttpRequestOperation(
request,
null,
responseTimeout,
false,
false))
.httpRequestOperationFactory(httpRequestOperationFactoryBuilder()
.responseTimeoutMillis(responseTimeout)
.build())
.build();

return new StyxHttpClient(connectionFactory, this.copy());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,16 @@
import com.google.common.annotations.VisibleForTesting;
import com.hotels.styx.api.Buffers;
import com.hotels.styx.api.HttpMethod;
import com.hotels.styx.api.HttpVersion;
import com.hotels.styx.api.LiveHttpRequest;
import com.hotels.styx.api.LiveHttpResponse;
import com.hotels.styx.api.HttpVersion;
import com.hotels.styx.api.Requests;
import com.hotels.styx.api.exceptions.TransportLostException;
import com.hotels.styx.api.extension.Origin;
import com.hotels.styx.client.Operation;
import com.hotels.styx.client.OriginStatsFactory;
import com.hotels.styx.common.format.HttpMessageFormatter;
import com.hotels.styx.common.format.SanitisedHttpMessageFormatter;
import com.hotels.styx.common.logging.HttpRequestMessageLogger;
import io.netty.buffer.ByteBuf;
import io.netty.channel.Channel;
Expand Down Expand Up @@ -79,8 +81,8 @@ public class HttpRequestOperation implements Operation<NettyConnection, LiveHttp
* @param originStatsFactory OriginStats factory
*/
@VisibleForTesting
public HttpRequestOperation(LiveHttpRequest request, OriginStatsFactory originStatsFactory) {
this(request, originStatsFactory, DEFAULT_RESPONSE_TIMEOUT_MILLIS, false, false);
public HttpRequestOperation(LiveHttpRequest request, OriginStatsFactory originStatsFactory, SanitisedHttpMessageFormatter sanitisedHttpMessageFormatter) {
this(request, originStatsFactory, DEFAULT_RESPONSE_TIMEOUT_MILLIS, false, false, sanitisedHttpMessageFormatter);
}

/**
Expand All @@ -91,12 +93,12 @@ public HttpRequestOperation(LiveHttpRequest request, OriginStatsFactory originSt
* @param requestLoggingEnabled
*/
public HttpRequestOperation(LiveHttpRequest request, OriginStatsFactory originStatsFactory,
int responseTimeoutMillis, boolean requestLoggingEnabled, boolean longFormat) {
int responseTimeoutMillis, boolean requestLoggingEnabled, boolean longFormat, HttpMessageFormatter httpMessageFormatter) {
this.request = requireNonNull(request);
this.originStatsFactory = Optional.ofNullable(originStatsFactory);
this.responseTimeoutMillis = responseTimeoutMillis;
this.requestLoggingEnabled = requestLoggingEnabled;
this.httpRequestMessageLogger = new HttpRequestMessageLogger("com.hotels.styx.http-messages.outbound", longFormat);
this.httpRequestMessageLogger = new HttpRequestMessageLogger("com.hotels.styx.http-messages.outbound", longFormat, httpMessageFormatter);
}

@VisibleForTesting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import static org.mockito.Mockito.when;

public class StyxHttpClientTest {

private HttpRequest httpRequest;
private HttpRequest secureRequest;
private WireMockServer server;
Expand Down
Loading

0 comments on commit ed1b6fd

Please sign in to comment.