Add http headers values to serialization exception message (#880)

Motivation: When users attempt to deserialize a response that has the wrong content-type, they receive an exception that lists all the headers, but with `<filtered>` as the value, making it hard to diagnose. Modifications: - Add a new debug filter `DEFAULT_DEBUG_HEADER_FILTER` to the class `HeaderUtils`. - Modify the following classes [`DefaultClassHttpDeserializer`, `DefaultTypeHttpDeserializer`, `FormUrlEncodedHttpDeserializer`, `HttpStringDeserializer`] and change the method `checkContentType` to call `headers.toString(DEFAULT_DEBUG_HEADER_FILTER)`. - Add tests to cover the new SerializationException messages. Result: Better debugging info in `SerializationException` when attempting to deserialize invalid `content-type`.
apple · Dec 2, 2019 · d182820 · d182820
1 parent 707d8be
commit d182820
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 5 deletions.
diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/DefaultClassHttpDeserializer.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/DefaultClassHttpDeserializer.java
@@ -23,6 +23,8 @@
 
 import java.util.function.Predicate;
 
+import static io.servicetalk.http.api.HeaderUtils.DEFAULT_DEBUG_HEADER_FILTER;
+
 /**
  * A {@link HttpDeserializer} that can deserialize to a {@link Class} of type {@link T}.
  *
@@ -64,7 +66,7 @@ public Publisher<T> deserialize(final HttpHeaders headers, final Publisher<Buffe
     private void checkContentType(final HttpHeaders headers) {
         if (!checkContentType.test(headers)) {
             throw new SerializationException("Unexpected headers, can not deserialize. Headers: "
-                    + headers.toString());
+                    + headers.toString(DEFAULT_DEBUG_HEADER_FILTER));
         }
     }
 }
diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/DefaultTypeHttpDeserializer.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/DefaultTypeHttpDeserializer.java
@@ -24,6 +24,8 @@
 
 import java.util.function.Predicate;
 
+import static io.servicetalk.http.api.HeaderUtils.DEFAULT_DEBUG_HEADER_FILTER;
+
 /**
  * A {@link HttpDeserializer} that can deserialize a {@link TypeHolder} of type {@link T}.
  *
@@ -65,7 +67,7 @@ public Publisher<T> deserialize(final HttpHeaders headers, final Publisher<Buffe
     private void checkContentType(final HttpHeaders headers) {
         if (!checkContentType.test(headers)) {
             throw new SerializationException("Unexpected headers, can not deserialize. Headers: "
-                    + headers.toString());
+                    + headers.toString(DEFAULT_DEBUG_HEADER_FILTER));
         }
     }
 }
diff --git a/...cetalk-http-api/src/main/java/io/servicetalk/http/api/FormUrlEncodedHttpDeserializer.java b/...cetalk-http-api/src/main/java/io/servicetalk/http/api/FormUrlEncodedHttpDeserializer.java
@@ -29,6 +29,7 @@
 import java.util.function.Predicate;
 import javax.annotation.Nullable;
 
+import static io.servicetalk.http.api.HeaderUtils.DEFAULT_DEBUG_HEADER_FILTER;
 import static io.servicetalk.http.api.HeaderUtils.hasContentType;
 import static io.servicetalk.http.api.HttpHeaderValues.APPLICATION_X_WWW_FORM_URLENCODED;
 import static io.servicetalk.http.api.QueryStringDecoder.decodeParams;
@@ -101,7 +102,7 @@ public Publisher<Map<String, List<String>>> deserialize(final HttpHeaders header
     private void checkContentType(final HttpHeaders headers) {
         if (!checkContentType.test(headers)) {
             throw new SerializationException("Unexpected headers, can not deserialize. Headers: "
-                    + headers.toString());
+                    + headers.toString(DEFAULT_DEBUG_HEADER_FILTER));
         }
     }
 

diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HeaderUtils.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HeaderUtils.java
@@ -19,6 +19,7 @@
 
 import java.nio.charset.Charset;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.function.BiFunction;
@@ -44,6 +45,7 @@
 import static java.lang.System.lineSeparator;
 import static java.nio.charset.Charset.availableCharsets;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.Arrays.asList;
 import static java.util.Collections.unmodifiableMap;
 import static java.util.regex.Pattern.CASE_INSENSITIVE;
 import static java.util.regex.Pattern.compile;
@@ -60,6 +62,17 @@ public final class HeaderUtils {
     static final int HASH_CODE_SEED = 0xc2b2ae35;
     public static final BiFunction<? super CharSequence, ? super CharSequence, CharSequence> DEFAULT_HEADER_FILTER =
             (k, v) -> "<filtered>";
+    private static final List<CharSequence> DEFAULT_DEBUG_HEADER_NAMES = asList(CONTENT_TYPE, CONTENT_LENGTH,
+            TRANSFER_ENCODING);
+    static final BiFunction<? super CharSequence, ? super CharSequence, CharSequence> DEFAULT_DEBUG_HEADER_FILTER =
+            (key, value) -> {
+                for (CharSequence headerName : DEFAULT_DEBUG_HEADER_NAMES) {
+                    if (contentEqualsIgnoreCase(key, headerName)) {
+                        return value;
+                    }
+                }
+                return "<filtered>";
+            };
     private static final ByteProcessor HEADER_NAME_VALIDATOR = value -> {
         validateHeaderNameToken(value);
         return true;

diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpStringDeserializer.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpStringDeserializer.java
@@ -27,6 +27,7 @@
 import java.util.function.Predicate;
 import javax.annotation.Nullable;
 
+import static io.servicetalk.http.api.HeaderUtils.DEFAULT_DEBUG_HEADER_FILTER;
 import static io.servicetalk.http.api.HeaderUtils.hasContentType;
 import static io.servicetalk.http.api.HttpHeaderValues.TEXT_PLAIN;
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -100,7 +101,7 @@ private String toString(@Nullable Buffer buffer) {
     private void checkContentType(final HttpHeaders headers) {
         if (!checkContentType.test(headers)) {
             throw new SerializationException("Unexpected headers, can not deserialize. Headers: "
-                    + headers.toString());
+                    + headers.toString(DEFAULT_DEBUG_HEADER_FILTER));
         }
     }
 }
diff --git a/...lk-http-api/src/test/java/io/servicetalk/http/api/FormUrlEncodedHttpDeserializerTest.java b/...lk-http-api/src/test/java/io/servicetalk/http/api/FormUrlEncodedHttpDeserializerTest.java
@@ -33,7 +33,9 @@
 import static io.servicetalk.buffer.netty.BufferAllocators.DEFAULT_ALLOCATOR;
 import static io.servicetalk.http.api.HttpHeaderNames.CONTENT_TYPE;
 import static java.util.Collections.singletonList;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.rules.ExpectedException.none;
@@ -82,7 +84,38 @@ public void invalidContentTypeThrows() {
         final FormUrlEncodedHttpDeserializer deserializer = FormUrlEncodedHttpDeserializer.UTF8;
 
         final HttpHeaders headers = DefaultHttpHeadersFactory.INSTANCE.newHeaders();
-        headers.set(CONTENT_TYPE, "invalid/content/type");
+        final String invalidContentType = "invalid/content/type";
+        headers.set(CONTENT_TYPE, invalidContentType);
+
+        expectedException.expect(instanceOf(SerializationException.class));
+        expectedException.expectMessage(containsString(invalidContentType));
+
+        deserializer.deserialize(headers, EMPTY_BUFFER);
+    }
+
+    @Test
+    public void invalidContentTypeThrowsAndMasksAdditionalHeadersValues() {
+        final FormUrlEncodedHttpDeserializer deserializer = FormUrlEncodedHttpDeserializer.UTF8;
+
+        final HttpHeaders headers = DefaultHttpHeadersFactory.INSTANCE.newHeaders();
+        final String invalidContentType = "invalid/content/type";
+        final String someHost = "some/host";
+        headers.set(CONTENT_TYPE, invalidContentType);
+        headers.set(HttpHeaderNames.HOST, someHost);
+
+        expectedException.expect(instanceOf(SerializationException.class));
+        expectedException.expectMessage(containsString(invalidContentType));
+        expectedException.expectMessage(containsString("<filtered>"));
+        expectedException.expectMessage(not(containsString(someHost)));
+
+        deserializer.deserialize(headers, EMPTY_BUFFER);
+    }
+
+    @Test
+    public void missingContentTypeThrows() {
+        final FormUrlEncodedHttpDeserializer deserializer = FormUrlEncodedHttpDeserializer.UTF8;
+
+        final HttpHeaders headers = DefaultHttpHeadersFactory.INSTANCE.newHeaders();
 
         expectedException.expect(instanceOf(SerializationException.class));
 

diff --git a/servicetalk-http-api/src/test/java/io/servicetalk/http/api/HeaderUtilsTest.java b/servicetalk-http-api/src/test/java/io/servicetalk/http/api/HeaderUtilsTest.java
@@ -23,7 +23,9 @@
 
 import static io.netty.util.AsciiString.of;
 import static io.servicetalk.http.api.HeaderUtils.isTransferEncodingChunked;
+import static io.servicetalk.http.api.HttpHeaderNames.CONTENT_LENGTH;
 import static io.servicetalk.http.api.HttpHeaderNames.CONTENT_TYPE;
+import static io.servicetalk.http.api.HttpHeaderNames.ORIGIN;
 import static io.servicetalk.http.api.HttpHeaderNames.TRANSFER_ENCODING;
 import static io.servicetalk.http.api.HttpHeaderValues.APPLICATION_JSON;
 import static io.servicetalk.http.api.HttpHeaderValues.APPLICATION_X_WWW_FORM_URLENCODED;
@@ -39,6 +41,19 @@
 import static org.junit.Assert.assertTrue;
 
 public class HeaderUtilsTest {
+    @Test
+    public void defaultDebugHeaderFilter() {
+        assertEquals(APPLICATION_JSON, HeaderUtils.DEFAULT_DEBUG_HEADER_FILTER.apply(CONTENT_TYPE, APPLICATION_JSON));
+
+        assertEquals("3495", HeaderUtils.DEFAULT_DEBUG_HEADER_FILTER.apply(CONTENT_LENGTH, "3495"));
+
+        assertEquals(CHUNKED, HeaderUtils.DEFAULT_DEBUG_HEADER_FILTER.apply(TRANSFER_ENCODING, CHUNKED));
+
+        assertEquals(CHUNKED, HeaderUtils.DEFAULT_DEBUG_HEADER_FILTER.apply("TrAnsFeR-eNcOdiNg", CHUNKED));
+
+        assertEquals("<filtered>", HeaderUtils.DEFAULT_DEBUG_HEADER_FILTER.apply(ORIGIN, "some/origin"));
+    }
+
     @Test
     public void hasContentType() {
         assertFalse(HeaderUtils.hasContentType(