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.

Sign up for GitHub

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

Jump to bottom

Prevent HttpLoggingPolicy Consuming Body #6638

Merged
merged 5 commits into from
Dec 5, 2019
Merged

Prevent HttpLoggingPolicy Consuming Body #6638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ad559e3
Prevent HttpLoggingPolicy from consuming request and response bodies
alzimmermsft Dec 4, 2019
8428dc8
Merge branch 'master' into AzCore_PreventLoggingFromEagerlyConsumingFlux
alzimmermsft Dec 4, 2019
5c77693
Updated tests to use named ClientLogger and updated tests to use a st…
alzimmermsft Dec 4, 2019
8009005
Merge branch 'master' into AzCore_PreventLoggingFromEagerlyConsumingFlux
alzimmermsft Dec 5, 2019
0fc10e8
Fixing linting issues
alzimmermsft Dec 5, 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
120 changes: 74 additions & 46 deletions sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
import com.azure.core.implementation.LogLevel;
import com.azure.core.implementation.LoggingUtil;
import com.azure.core.util.CoreUtils;
import com.azure.core.util.FluxUtil;
import com.azure.core.util.UrlBuilder;
import com.azure.core.util.logging.ClientLogger;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;
import reactor.core.publisher.Mono;

import java.io.ByteArrayOutputStream;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
Expand Down Expand Up @@ -100,7 +100,7 @@ public Mono<HttpResponse> process(HttpPipelineCallContext context, HttpPipelineN
* @param request HTTP request being sent to Azure.
* @return A Mono which will emit the string to log.
*/
private Mono<String> logRequest(final ClientLogger logger, final HttpRequest request) {
private Mono<Void> logRequest(final ClientLogger logger, final HttpRequest request) {
int numericLogLevel = LoggingUtil.getEnvironmentLoggingLevel().toNumeric();
if (shouldLoggingBeSkipped(numericLogLevel)) {
return Mono.empty();
Expand All @@ -117,44 +117,59 @@ private Mono<String> logRequest(final ClientLogger logger, final HttpRequest req

addHeadersToLogMessage(request.getHeaders(), requestLogMessage, numericLogLevel);

Mono<String> requestLoggingMono = Mono.defer(() -> Mono.just(requestLogMessage.toString()));
if (!httpLogDetailLevel.shouldLogBody()) {
return logAndReturn(logger, requestLogMessage, null);
}

if (httpLogDetailLevel.shouldLogBody()) {
if (request.getBody() == null) {
requestLogMessage.append("(empty body)")
.append(System.lineSeparator())
.append("--> END ")
.append(request.getHttpMethod())
.append(System.lineSeparator());
} else {
String contentType = request.getHeaders().getValue("Content-Type");
long contentLength = getContentLength(logger, request.getHeaders());
if (request.getBody() == null) {
requestLogMessage.append("(empty body)")
.append(System.lineSeparator())
.append("--> END ")
.append(request.getHttpMethod())
.append(System.lineSeparator());

if (shouldBodyBeLogged(contentType, contentLength)) {
requestLoggingMono = FluxUtil.collectBytesInByteBufferStream(request.getBody()).flatMap(bytes -> {
return logAndReturn(logger, requestLogMessage, null);
}

String contentType = request.getHeaders().getValue("Content-Type");
long contentLength = getContentLength(logger, request.getHeaders());

if (shouldBodyBeLogged(contentType, contentLength)) {
ByteArrayOutputStream outputStream = new ByteArrayOutputStream((int) contentLength);

// Add non-mutating operators to the data stream.
request.setBody(
request.getBody()
.doOnNext(byteBuffer -> {
for (int i = byteBuffer.position(); i < byteBuffer.limit(); i++) {
alzimmermsft marked this conversation as resolved.
Show resolved Hide resolved
outputStream.write(byteBuffer.get(i));
}
})
.doFinally(ignored -> {
requestLogMessage.append(contentLength)
.append("-byte body:")
.append(System.lineSeparator())
.append(prettyPrintIfNeeded(logger, contentType, new String(bytes, StandardCharsets.UTF_8)))
.append(prettyPrintIfNeeded(logger, contentType,
new String(outputStream.toByteArray(), StandardCharsets.UTF_8)))
.append(System.lineSeparator())
.append("--> END ")
.append(request.getHttpMethod())
.append(System.lineSeparator());

return Mono.just(requestLogMessage.toString());
});
} else {
requestLogMessage.append(contentLength)
.append("-byte body: (content not logged)")
.append(System.lineSeparator())
.append("--> END ")
.append(request.getHttpMethod())
.append(System.lineSeparator());
}
}
}
logger.info(requestLogMessage.toString());
}));

return requestLoggingMono.doOnNext(logger::info);
return Mono.empty();
} else {
requestLogMessage.append(contentLength)
.append("-byte body: (content not logged)")
.append(System.lineSeparator())
.append("--> END ")
.append(request.getHttpMethod())
.append(System.lineSeparator());

return logAndReturn(logger, requestLogMessage, null);
}
}

/*
Expand Down Expand Up @@ -194,32 +209,45 @@ private Mono<HttpResponse> logResponse(final ClientLogger logger, final HttpResp

addHeadersToLogMessage(response.getHeaders(), responseLogMessage, numericLogLevel);

Mono<String> responseLoggingMono = Mono.defer(() -> Mono.just(responseLogMessage.toString()));

if (httpLogDetailLevel.shouldLogBody()) {
final String contentTypeHeader = response.getHeaderValue("Content-Type");
if (!httpLogDetailLevel.shouldLogBody()) {
responseLogMessage.append("<-- END HTTP");
return logAndReturn(logger, responseLogMessage, response);
}

if (shouldBodyBeLogged(contentTypeHeader, getContentLength(logger, response.getHeaders()))) {
final HttpResponse bufferedResponse = response.buffer();
responseLoggingMono = bufferedResponse.getBodyAsString().flatMap(body -> {
String contentTypeHeader = response.getHeaderValue("Content-Type");
long contentLength = getContentLength(logger, response.getHeaders());

if (shouldBodyBeLogged(contentTypeHeader, contentLength)) {
HttpResponse bufferedResponse = response.buffer();
ByteArrayOutputStream outputStream = new ByteArrayOutputStream((int) contentLength);
return bufferedResponse.getBody()
.doOnNext(byteBuffer -> {
for (int i = byteBuffer.position(); i < byteBuffer.limit(); i++) {
alzimmermsft marked this conversation as resolved.
Show resolved Hide resolved
outputStream.write(byteBuffer.get(i));
}
})
.doFinally(ignored -> {
responseLogMessage.append("Response body:")
.append(System.lineSeparator())
.append(prettyPrintIfNeeded(logger, contentTypeHeader, body))
.append(prettyPrintIfNeeded(logger, contentTypeHeader,
new String(outputStream.toByteArray(), StandardCharsets.UTF_8)))
.append(System.lineSeparator())
.append("<-- END HTTP");

return Mono.just(responseLogMessage.toString());
}).switchIfEmpty(responseLoggingMono);
} else {
responseLogMessage.append("(body content not logged)")
.append(System.lineSeparator())
.append("<-- END HTTP");
}
logger.info(responseLogMessage.toString());
}).then(Mono.just(bufferedResponse));
} else {
responseLogMessage.append("<-- END HTTP");
responseLogMessage.append("(body content not logged)")
.append(System.lineSeparator())
.append("<-- END HTTP");

return logAndReturn(logger, responseLogMessage, response);
}
}

return responseLoggingMono.doOnNext(logger::info).thenReturn(response);
private <T> Mono<T> logAndReturn(ClientLogger logger, StringBuilder logMessageBuilder, T data) {
logger.info(logMessageBuilder.toString());
return Mono.justOrEmpty(data);
}

/*
Expand Down
11 changes: 6 additions & 5 deletions ...ore/azure-core/src/main/java/com/azure/core/implementation/http/BufferedHttpResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import com.azure.core.http.HttpHeaders;
import com.azure.core.http.HttpResponse;
import com.azure.core.util.FluxUtil;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;

Expand All @@ -17,7 +18,7 @@
*/
public final class BufferedHttpResponse extends HttpResponse {
private final HttpResponse innerHttpResponse;
private final Mono<byte[]> cachedBody;
private final Flux<ByteBuffer> cachedBody;

/**
* Creates a buffered HTTP response.
Expand All @@ -27,7 +28,7 @@ public final class BufferedHttpResponse extends HttpResponse {
public BufferedHttpResponse(HttpResponse innerHttpResponse) {
super(innerHttpResponse.getRequest());
this.innerHttpResponse = innerHttpResponse;
this.cachedBody = innerHttpResponse.getBodyAsByteArray().cache();
this.cachedBody = innerHttpResponse.getBody().cache();
}

@Override
Expand All @@ -46,13 +47,13 @@ public HttpHeaders getHeaders() {
}

@Override
public Mono<byte[]> getBodyAsByteArray() {
public Flux<ByteBuffer> getBody() {
return cachedBody;
}

@Override
public Flux<ByteBuffer> getBody() {
return getBodyAsByteArray().flatMapMany(bytes -> Flux.just(ByteBuffer.wrap(bytes)));
public Mono<byte[]> getBodyAsByteArray() {
return FluxUtil.collectBytesInByteBufferStream(cachedBody.map(ByteBuffer::duplicate));
}

@Override
Expand Down
Loading