feat: HttpLoggingInterceptor logs only plain text response bodies

Signed-off-by: Marc Nuri <marc@marcnuri.com>

Author: manusa

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/BufferUtil.java

@@ -16,6 +16,9 @@
 package io.fabric8.kubernetes.client.http;
 
 import java.nio.ByteBuffer;
+import java.nio.charset.CharacterCodingException;
+import java.nio.charset.CharsetDecoder;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Collection;
 
@@ -70,4 +73,24 @@ public static ByteBuffer copy(ByteBuffer buffer) {
  buffer.position(position);
  return clone;
  }
+
+ /**
+ * Very rudimentary method to check if the provided ByteBuffer contains text.
+ * 
+ * @return true if the buffer contains text, false otherwise.
+ */
+ public static boolean isPlainText(ByteBuffer originalBuffer) {
+ if (originalBuffer == null) {
+ return false;
+ }
+ final ByteBuffer buffer = copy(originalBuffer);
+ final CharsetDecoder decoder = StandardCharsets.UTF_8.newDecoder();
+ try {
+ decoder.decode(buffer);
+ return true;
+ } catch (CharacterCodingException ex) {
+ return false;
+ }
+ }
+
 }

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/http/HttpLoggingInterceptor.java

@@ -113,8 +113,9 @@ public DeferredLoggingConsumer(HttpLogger httpLogger, HttpRequest originalReques
 
  @Override
  public void consume(List<ByteBuffer> value, AsyncBody asyncBody) throws Exception {
- // TODO: Should try to detect if the body is text or not? (HttpLoggingInterceptor.isPlainText)
- value.stream().map(BufferUtil::copy).forEach(responseBody::add); // Potential leak
+ if (!value.isEmpty() && BufferUtil.isPlainText(value.iterator().next())) {
+ value.stream().map(BufferUtil::copy).forEach(responseBody::add); // Potential leak
+ }
  originalConsumer.consume(value, asyncBody);
  }
 
@@ -133,7 +134,7 @@ public void accept(Void v, Throwable throwable) {
 
  /**
  * Registers the asyncBody.done() callback.
- * 
+ *
  * @param asyncBody the AsyncBody instance to register the callback on the done() future.
  */
  private void processAsyncBody(AsyncBody asyncBody) {
@@ -168,7 +169,7 @@ void logResponse(HttpResponse<?> response) {
  }
 
  void logResponseBody(Queue<ByteBuffer> responseBody) {
- if (logger.isTraceEnabled() && responseBody != null) {
+ if (logger.isTraceEnabled() && responseBody != null && !responseBody.isEmpty()) {
  final StringBuilder bodyString = new StringBuilder();
  while (!responseBody.isEmpty()) {
  bodyString.append(StandardCharsets.UTF_8.decode(responseBody.poll()));

kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractHttpLoggingInterceptorTest.java

@@ -15,8 +15,16 @@
  */
 package io.fabric8.kubernetes.client.http;
 
+import io.fabric8.mockwebserver.Context;
 import io.fabric8.mockwebserver.DefaultMockServer;
+import io.fabric8.mockwebserver.ServerRequest;
+import io.fabric8.mockwebserver.ServerResponse;
+import io.fabric8.mockwebserver.internal.SimpleRequest;
 import io.fabric8.mockwebserver.utils.ResponseProviders;
+import okhttp3.mockwebserver.MockResponse;
+import okhttp3.mockwebserver.MockWebServer;
+import okhttp3.mockwebserver.RecordedRequest;
+import okio.Buffer;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
@@ -27,7 +35,11 @@
 import org.slf4j.Logger;
 
 import java.net.URI;
+import java.util.ArrayDeque;
 import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Queue;
 import java.util.concurrent.TimeUnit;
 
 import static org.mockito.ArgumentMatchers.anyInt;
@@ -36,18 +48,21 @@
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.timeout;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.when;
 
 public abstract class AbstractHttpLoggingInterceptorTest {
 
  private static DefaultMockServer server;
+ private static Map<ServerRequest, Queue<ServerResponse>> responses;
  private Logger logger;
  private InOrder inOrder;
  private HttpClient httpClient;
 
  @BeforeAll
  static void beforeAll() {
- server = new DefaultMockServer(false);
+ responses = new HashMap<>();
+ server = new DefaultMockServer(new Context(), new MockWebServer(), responses, false);
  server.start();
  }
 
@@ -157,6 +172,33 @@ public void httpResponseBodyLogged() throws Exception {
  inOrder.verify(logger).trace("-HTTP END-");
  }
 
+ @Test
+ @DisplayName("HTTP binary response body is skipped")
+ public void httpResponseBodySkipped() throws Exception {
+ final MockResponse binaryResponse = new MockResponse()
+ .setResponseCode(200)
+ .setBody(new Buffer().write(new byte[] { (byte) 0xFF, (byte) 0xD8, (byte) 0x00, (byte) 0x12, (byte) 0x34 }));
+ responses.computeIfAbsent(new SimpleRequest("/binary-response-body"), k -> new ArrayDeque<>()).add(
+ new ServerResponse() {
+ @Override
+ public boolean isRepeatable() {
+ return true;
+ }
+
+ @Override
+ public MockResponse toMockResponse(RecordedRequest recordedRequest) {
+ return binaryResponse;
+ }
+ });
+ httpClient.sendAsync(httpClient.newHttpRequestBuilder()
+ .uri(server.url("/binary-response-body"))
+ .build(), String.class).get(10, TimeUnit.SECONDS);
+ inOrder.verify(logger, timeout(1000L)).trace("-HTTP START-");
+ inOrder.verify(logger).trace(eq("< {} {}"), anyInt(), anyString());
+ inOrder.verify(logger, times(1)).trace(anyString()); // only -HTTP END- was logged
+ inOrder.verifyNoMoreInteractions();
+ }
+
  @Test
  @DisplayName("WS request URI is logged")
  public void wsRequestUriLogged() throws Exception {