Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GitHub#454 Sanitise HTTP message logs #457

Merged
merged 26 commits into from
Sep 27, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
08d48d0
GitHub#454 Sanitise HTTP message logs
Sep 5, 2019
a913858
GitHub#454 removed unused import
Sep 6, 2019
5d4e48c
GitHub#454 fixes for PR comments:
Sep 6, 2019
4a114c5
GitHub#454 added documentation for the hideHeaders/hideCookies feature
Sep 6, 2019
2607c2f
GitHub#454 made changes for PR comments:
Sep 16, 2019
fa658a8
GitHub#454
Sep 16, 2019
295f249
GitHub#454
Sep 16, 2019
a12c7a8
GitHub#454 made formatter final
Sep 16, 2019
1e3bfbe
GitHub#454 Sanitise HTTP message logs
Sep 5, 2019
6c76166
GitHub#454 removed unused import
Sep 6, 2019
d47c9d9
GitHub#454 fixes for PR comments:
Sep 6, 2019
63d6b47
GitHub#454 added documentation for the hideHeaders/hideCookies feature
Sep 6, 2019
1de5218
GitHub#454 made changes for PR comments:
Sep 16, 2019
d827a53
GitHub#454
Sep 16, 2019
e0df682
GitHub#454
Sep 16, 2019
6f97a05
GitHub#454 made formatter final
Sep 16, 2019
7315f50
GitHub#454 fixed checkstyle errors
Sep 17, 2019
4a842e5
Merge remote-tracking branch 'origin/sanitise-http-messages' into san…
Sep 17, 2019
58686b5
GitHub#454 fixed checkstyle errors
Sep 17, 2019
ae7d61a
GitHub#454 fixed checkstyle errors
Sep 17, 2019
ccc972a
GitHub#454 fixed checkstyle errors
Sep 17, 2019
9ad9f97
GitHub#454 fixed formatting in response to PR comments
Sep 18, 2019
e309d19
GitHub#454 Made fixes for PR comments: - added kotlin test - changed …
OwenLindsell Sep 26, 2019
3d11609
GitHub#454 fixed naming - nettyHeaders to httpHeaders and removed unu…
OwenLindsell Sep 27, 2019
6c20fd2
GitHub#454 fixed imports for checkstyle
OwenLindsell Sep 27, 2019
34a5329
GitHub#454 tidied test
OwenLindsell Sep 27, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -98,6 +98,6 @@ public boolean equals(Object o) {

@Override
public String toString() {
return name + "=" + HEADER_VALUES_JOINER.join(values);
return name + ":" + HEADER_VALUES_JOINER.join(values);
OwenLindsell marked this conversation as resolved.
Show resolved Hide resolved
}
}
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 @@ -349,8 +349,7 @@ public String toString() {
return toStringHelper(this)
.add("version", version)
.add("method", method)
.add("uri", url)
.add("headers", headers)
.add("url", url)
OwenLindsell marked this conversation as resolved.
Show resolved Hide resolved
.add("id", id)
.toString();
}
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 @@ -214,7 +214,6 @@ public String toString() {
return toStringHelper(this)
.add("version", version)
.add("status", status)
.add("headers", headers)
.toString();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -388,8 +388,7 @@ public String toString() {
return toStringHelper(this)
.add("version", version)
.add("method", method)
.add("uri", url)
.add("headers", headers)
.add("url", url)
OwenLindsell marked this conversation as resolved.
Show resolved Hide resolved
OwenLindsell marked this conversation as resolved.
Show resolved Hide resolved
.add("id", id)
.toString();
}
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 @@ -235,7 +235,6 @@ public String toString() {
return toStringHelper(this)
.add("version", version)
.add("status", status)
.add("headers", headers)
.toString();
}

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 @@ -31,15 +31,15 @@ public void rejectsEmptyValuesList() {
@Test
public void createsSingleValueHeader() {
HttpHeader header = header("name", "value");
assertThat(header.toString(), is("name=value"));
assertThat(header.toString(), is("name:value"));
assertThat(header.value(), is("value"));
assertThat(header.values(), contains("value"));
}

@Test
public void createsMultipleValueHeader() {
HttpHeader header = header("name", "value1", "value2");
assertThat(header.toString(), is("name=value1, value2"));
assertThat(header.toString(), is("name:value1, value2"));
OwenLindsell marked this conversation as resolved.
Show resolved Hide resolved
assertThat(header.value(), is("value1"));
assertThat(header.values(), contains("value1", "value2"));
}
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 @@ -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("HttpRequest{version=HTTP/1.1, method=PATCH, url=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("LiveHttpRequest{version=HTTP/1.0, method=PATCH, url=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 @@ -52,6 +52,7 @@
import static com.hotels.styx.api.extension.service.StickySessionConfig.stickySessionDisabled;
import static com.hotels.styx.client.StyxHeaderConfig.ORIGIN_ID_DEFAULT;
import static com.hotels.styx.client.stickysession.StickySessionCookie.newStickySessionCookie;
import static com.hotels.styx.common.format.HttpMessageFormatter.formatRequest;
import static io.netty.handler.codec.http.HttpMethod.HEAD;
import static java.util.Collections.emptyList;
import static java.util.Objects.nonNull;
Expand Down Expand Up @@ -255,9 +256,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[]{formatRequest(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 @@ -38,6 +38,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.hotels.styx.api.HttpHeaderNames.HOST;
import static com.hotels.styx.api.HttpHeaderNames.USER_AGENT;
import static com.hotels.styx.common.format.HttpMessageFormatter.formatRequest;
import static com.hotels.styx.api.extension.Origin.newOriginBuilder;
import static com.hotels.styx.client.HttpConfig.newHttpConfigBuilder;
import static java.util.Objects.requireNonNull;
Expand Down Expand Up @@ -158,7 +159,7 @@ private static Origin originFromRequest(LiveHttpRequest request, Boolean isHttps
.orElseGet(() -> {
checkArgument(request.url().isAbsolute(), "host header is not set for request=%s", request);
OwenLindsell marked this conversation as resolved.
Show resolved Hide resolved
return request.url().authority().map(Url.Authority::hostAndPort)
.orElseThrow(() -> new IllegalArgumentException("Cannot send request " + request + " as URL is not absolute and no HOST header is present"));
.orElseThrow(() -> new IllegalArgumentException("Cannot send request " + formatRequest(request) + " as URL is not absolute and no HOST header is present"));
OwenLindsell marked this conversation as resolved.
Show resolved Hide resolved
});

HostAndPort host = HostAndPort.fromString(hostAndPort);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import static com.google.common.base.Objects.toStringHelper;
import static com.hotels.styx.api.HttpHeaderNames.HOST;
import static com.hotels.styx.api.extension.service.BackendService.DEFAULT_RESPONSE_TIMEOUT_MILLIS;
import static com.hotels.styx.common.format.HttpMessageFormatter.formatRequest;
import static io.netty.handler.codec.http.LastHttpContent.EMPTY_LAST_CONTENT;
import static java.lang.String.format;
import static java.util.Objects.requireNonNull;
Expand Down Expand Up @@ -159,7 +160,7 @@ public Observable<LiveHttpResponse> execute(NettyConnection nettyConnection) {

if (requestIsOngoing(requestRequestBodyChunkSubscriber.get())) {
LOGGER.warn("Origin responded too quickly to an ongoing request, or it was cancelled. Connection={}, Request={}.",
new Object[]{nettyConnection.channel(), this.request});
new Object[]{nettyConnection.channel(), formatRequest(this.request)});
nettyConnection.close();
}
}
Expand Down Expand Up @@ -192,7 +193,7 @@ private void removeProxyBridgeHandlers(NettyConnection connection) {
} catch (NoSuchElementException cause) {
long elapsedTime = System.currentTimeMillis() - requestTime;
LOGGER.error("Failed to remove pipeline handlers from pooled connection. elapsedTime={}, request={}, terminationCount={}, executionCount={}, cause={}",
new Object[]{elapsedTime, request, terminationCount.get(), executeCount.get(), cause});
new Object[]{elapsedTime, formatRequest(request), terminationCount.get(), executeCount.get(), cause});
}
}

Expand Down Expand Up @@ -256,7 +257,7 @@ private ChannelFutureListener subscribeToRequestBody() {
} else {
String channelIdentifier = String.format("%s -> %s", nettyConnection.channel().localAddress(), nettyConnection.channel().remoteAddress());
LOGGER.error(format("Failed to send request headers. origin=%s connection=%s request=%s",
nettyConnection.getOrigin(), channelIdentifier, request), headersFuture.cause());
nettyConnection.getOrigin(), channelIdentifier, formatRequest(request)), headersFuture.cause());
responseFromOriginObserver.onError(new TransportLostException(nettyConnection.channel().remoteAddress(), nettyConnection.getOrigin()));
}
};
Expand Down Expand Up @@ -311,7 +312,7 @@ public void onNext(ByteBuf chunk) {
} else {
String channelIdentifier = String.format("%s -> %s", nettyConnection.channel().localAddress(), nettyConnection.channel().remoteAddress());
LOGGER.error(format("Failed to send request body data. origin=%s connection=%s request=%s",
nettyConnection.getOrigin(), channelIdentifier, request), future.cause());
nettyConnection.getOrigin(), channelIdentifier, formatRequest(request)), future.cause());
OwenLindsell marked this conversation as resolved.
Show resolved Hide resolved
this.onError(new TransportLostException(nettyConnection.channel().remoteAddress(), nettyConnection.getOrigin()));
}
});
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 @@ -68,8 +68,9 @@ public void logsClientSideRequestShortFormat() {
LiveHttpRequest styxRequest = get("http://www.hotels.com/foo/bar/request").build();
new HttpRequestMessageLogger("com.hotels.styx.http-messages.outbound", false).logRequest(styxRequest, origin, true);

assertThat(log.lastMessage(), is(loggingEvent(INFO, format("requestId=%s, secure=true, request=\\{method=GET, uri=%s, origin=\"%s\"\\}",
styxRequest.id(), styxRequest.url(), origin.hostAndPortString()))));
assertThat(log.lastMessage(), is(loggingEvent(INFO,
format("requestId=%s, request=LiveHttpRequest\\{version=HTTP/1.1, method=GET, url=%s, id=%s}, secure=true, origin=MyApp:h1:%s",
styxRequest.id(), styxRequest.url(), styxRequest.id(), origin.hostAndPortString()))));
}

@Test
Expand All @@ -78,8 +79,8 @@ public void logsClientSideRequestLongFormat() {
new HttpRequestMessageLogger("com.hotels.styx.http-messages.outbound", true).logRequest(styxRequest, origin, true);

assertThat(log.lastMessage(), is(loggingEvent(INFO,
format("requestId=%s, secure=true, request=\\{method=GET, uri=%s, origin=\"%s\", headers=\\[Host=www.hotels.com\\]\\}",
styxRequest.id(), styxRequest.url(), origin.hostAndPortString()))));
format("requestId=%s, request=LiveHttpRequest\\{version=HTTP/1.1, method=GET, url=%s, headers=\\[Host:www.hotels.com\\], id=%s}, secure=true, origin=MyApp:h1:%s",
styxRequest.id(), styxRequest.url(), styxRequest.id(), origin.hostAndPortString()))));
}

@Test
Expand All @@ -88,7 +89,7 @@ public void logsClientSideResponseDetailsShortFormat() {
LiveHttpResponse styxResponse = response(OK).build();
new HttpRequestMessageLogger("com.hotels.styx.http-messages.outbound", false).logResponse(styxRequest, styxResponse);

assertThat(log.lastMessage(), is(loggingEvent(INFO, format("requestId=%s, response=\\{status=200 OK\\}", styxRequest.id()))));
assertThat(log.lastMessage(), is(loggingEvent(INFO, format("requestId=%s, response=LiveHttpResponse\\{version=HTTP/1.1, status=200 OK\\}", styxRequest.id()))));
}

@Test
Expand All @@ -97,7 +98,7 @@ public void logsClientSideResponseDetailsLongFormat() {
LiveHttpResponse styxResponse = response(OK).build();
new HttpRequestMessageLogger("com.hotels.styx.http-messages.outbound", true).logResponse(styxRequest, styxResponse);

assertThat(log.lastMessage(), is(loggingEvent(INFO, format("requestId=%s, response=\\{status=200 OK\\, headers=\\[\\]}", styxRequest.id()))));
assertThat(log.lastMessage(), is(loggingEvent(INFO, format("requestId=%s, response=LiveHttpResponse\\{version=HTTP/1.1, status=200 OK, headers=\\[\\]\\}", styxRequest.id()))));
}

@Test
Expand All @@ -121,7 +122,7 @@ private Object[][] responseLogUnexpectedArguments() {

return new Object[][]{
{normalRequest, null, WARN, "requestId=.*, response=null"},
{null, normalResponse, INFO, "requestId=null, response=\\{status=200 OK\\}"},
{null, normalResponse, INFO, "requestId=null, response=LiveHttpResponse\\{version=HTTP/1.1, status=200 OK\\}"},
{null, null, WARN, "requestId=null, response=null"},
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
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.
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 com.hotels.styx.common.format;

import com.hotels.styx.api.HttpHeader;
import com.hotels.styx.api.HttpHeaders;
import com.hotels.styx.api.RequestCookie;

import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;

import static java.util.Collections.emptyList;
import static java.util.Objects.requireNonNull;

public class HttpHeaderFormatter {

private static final List<String> COOKIE_HEADER_NAMES = Arrays.asList("cookie", "set-cookie");
private static HttpHeaderFormatter instance;

public static void initialise(List<String> headerValuesToHide, List<String> cookieValuesToHide) {
headerValuesToHide.replaceAll(String::toLowerCase);
cookieValuesToHide.replaceAll(String::toLowerCase);
instance = new HttpHeaderFormatter(headerValuesToHide, cookieValuesToHide);
}

public static HttpHeaderFormatter instance() {
return instance == null
? new HttpHeaderFormatter(emptyList(), emptyList())
: instance;
}
OwenLindsell marked this conversation as resolved.
Show resolved Hide resolved

private List<String> headerValuesToHide;
private List<String> cookieValuesToHide;
OwenLindsell marked this conversation as resolved.
Show resolved Hide resolved

private HttpHeaderFormatter(List<String> headerValuesToHide, List<String> cookieValuesToHide) {
this.headerValuesToHide = requireNonNull(headerValuesToHide);
this.cookieValuesToHide = requireNonNull(cookieValuesToHide);
}

public String format(HttpHeaders headers) {
String sanitisedHeaders = StreamSupport.stream(headers.spliterator(), false)
.map(this::hideOrFormatHeader)
.collect(Collectors.joining(", "));

return "[" + sanitisedHeaders + "]";
}

private String hideOrFormatHeader(HttpHeader header) {
return headerValuesToHide.contains(header.name().toLowerCase())
? header.name() + ":****"
: formatHeaderAsCookieIfNecessary(header);
}

private String formatHeaderAsCookieIfNecessary(HttpHeader header) {
return COOKIE_HEADER_NAMES.contains(header.name().toLowerCase())
? formatCookieHeader(header)
: header.toString();
}

private String formatCookieHeader(HttpHeader header) {
String cookies = RequestCookie.decode(header.value()).stream()
.map(this::hideOrFormatCookie)
.collect(Collectors.joining(";"));

return header.name() + ":" + cookies;
}

private String hideOrFormatCookie(RequestCookie cookie) {
return cookieValuesToHide.contains(cookie.name().toLowerCase())
? cookie.name() + "=****"
: cookie.toString();
}

}
Loading