Repository: googleapis/google-http-java-client
Pull request #911 (2019-12-10 20:09:44)

fix: set mediaType to null if contentType cannot be parsed
Fixes issue #905 by setting mediaType to null if the contentType is invalid.

jmorrise: I force-pushed to resolve the conventionalcommits error, but I'm afraid things might be in a weird state now - let me know if I need to fix this.
chingor13: Note that this is potentially a behavior breaking change in that previously you could get an `InvalidArgumentException` (unchecked, runtime exception) when executing a request. Now, you will get the response with a null (unparsed) `MediaType` which would seem to be always preferred anyways.

Patch:
diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java b/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
index 5273300f..3341720a 100644
--- a/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
+++ b/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
@@ -150,7 +150,7 @@ public final class HttpResponse {
       contentType = request.getResponseHeaders().getContentType();
     }
     this.contentType = contentType;
-    mediaType = contentType == null ? null : new HttpMediaType(contentType);
+    this.mediaType = parseMediaType(contentType);
 
     // log from buffer
     if (loggable) {
@@ -158,6 +158,22 @@ public final class HttpResponse {
     }
   }
 
+  /**
+   * Returns an {@link HttpMediaType} object parsed from {@link #contentType}, or {@code null} if
+   * if {@link #contentType} cannot be parsed or {@link #contentType} is {@code null}.
+   */
+  private static HttpMediaType parseMediaType(String contentType) {
+    if (contentType == null) {
+      return null;
+    }
+    try {
+      return new HttpMediaType(contentType);
+    } catch (IllegalArgumentException e) {
+      // contentType is invalid and cannot be parsed.
+      return null;
+    }
+  }
+
   /**
    * Returns the limit to the content size that will be logged during {@link #getContent()}.
    *


Test patch:
diff --git a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
index a3efdb30..f7638d67 100644
--- a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
+++ b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
@@ -58,6 +58,10 @@ public class HttpResponseTest extends TestCase {
 
   private static final String SAMPLE = "123\u05D9\u05e0\u05D9\u05D1";
   private static final String SAMPLE2 = "123abc";
+  private static final String VALID_CONTENT_TYPE = "text/plain";
+  private static final String VALID_CONTENT_TYPE_WITH_PARAMS =
+      "application/vnd.com.google.datastore.entity+json; charset=utf-8; version=v1; q=0.9";
+  private static final String INVALID_CONTENT_TYPE = "!!!invalid!!!";
 
   public void testParseAsString_utf8() throws Exception {
     HttpTransport transport =
@@ -102,6 +106,81 @@ public class HttpResponseTest extends TestCase {
     assertEquals(SAMPLE2, response.parseAsString());
   }
 
+  public void testParseAsString_validContentType() throws Exception {
+    HttpTransport transport =
+        new MockHttpTransport() {
+          @Override
+          public LowLevelHttpRequest buildRequest(String method, String url) throws IOException {
+            return new MockLowLevelHttpRequest() {
+              @Override
+              public LowLevelHttpResponse execute() throws IOException {
+                MockLowLevelHttpResponse result = new MockLowLevelHttpResponse();
+                result.setContent(SAMPLE2);
+                result.setContentType(VALID_CONTENT_TYPE);
+                return result;
+              }
+            };
+          }
+        };
+    HttpRequest request =
+        transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL);
+
+    HttpResponse response = request.execute();
+    assertEquals(SAMPLE2, response.parseAsString());
+    assertEquals(VALID_CONTENT_TYPE, response.getContentType());
+    assertNotNull(response.getMediaType());
+  }
+
+  public void testParseAsString_validContentTypeWithParams() throws Exception {
+    HttpTransport transport =
+        new MockHttpTransport() {
+          @Override
+          public LowLevelHttpRequest buildRequest(String method, String url) throws IOException {
+            return new MockLowLevelHttpRequest() {
+              @Override
+              public LowLevelHttpResponse execute() throws IOException {
+                MockLowLevelHttpResponse result = new MockLowLevelHttpResponse();
+                result.setContent(SAMPLE2);
+                result.setContentType(VALID_CONTENT_TYPE_WITH_PARAMS);
+                return result;
+              }
+            };
+          }
+        };
+    HttpRequest request =
+        transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL);
+
+    HttpResponse response = request.execute();
+    assertEquals(SAMPLE2, response.parseAsString());
+    assertEquals(VALID_CONTENT_TYPE_WITH_PARAMS, response.getContentType());
+    assertNotNull(response.getMediaType());
+  }
+
+  public void testParseAsString_invalidContentType() throws Exception {
+    HttpTransport transport =
+        new MockHttpTransport() {
+          @Override
+          public LowLevelHttpRequest buildRequest(String method, String url) throws IOException {
+            return new MockLowLevelHttpRequest() {
+              @Override
+              public LowLevelHttpResponse execute() throws IOException {
+                MockLowLevelHttpResponse result = new MockLowLevelHttpResponse();
+                result.setContent(SAMPLE2);
+                result.setContentType(INVALID_CONTENT_TYPE);
+                return result;
+              }
+            };
+          }
+        };
+    HttpRequest request =
+        transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL);
+
+    HttpResponse response = request.execute();
+    assertEquals(SAMPLE2, response.parseAsString());
+    assertEquals(INVALID_CONTENT_TYPE, response.getContentType());
+    assertNull(response.getMediaType());
+  }
+
   public void testStatusCode_negative_dontThrowException() throws Exception {
     subtestStatusCode_negative(false);
   }