Check for null error stream (#2249)

core/src/main/java/google/registry/request/UrlConnectionUtils.java

@@ -28,6 +28,7 @@ import com.google.common.net.MediaType;
 import java.io.DataOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.HttpURLConnection;
 import java.net.URLConnection;
 import java.util.Random;
 
@@ -36,26 +37,37 @@ public final class UrlConnectionUtils {
 
   private UrlConnectionUtils() {}
 
-  /** Retrieves the response from the given connection as a byte array. */
-  public static byte[] getResponseBytes(URLConnection connection) throws IOException {
-    try (InputStream is = connection.getInputStream()) {
+  /**
+   * Retrieves the response from the given connection as a byte array.
+   *
+   * <p>Note that in the event the response code is 4XX or 5XX, we use the error stream as any
+   * payload is included there.
+   *
+   * @see HttpURLConnection#getErrorStream()
+   */
+  public static byte[] getResponseBytes(HttpURLConnection connection) throws IOException {
+    int responseCode = connection.getResponseCode();
+    try (InputStream is =
+        responseCode < 400 ? connection.getInputStream() : connection.getErrorStream()) {
       return ByteStreams.toByteArray(is);
+    } catch (NullPointerException e) {
+      return new byte[] {};
     }
   }
 
   /** Sets auth on the given connection with the given username/password. */
-  public static void setBasicAuth(URLConnection connection, String username, String password) {
+  public static void setBasicAuth(HttpURLConnection connection, String username, String password) {
     setBasicAuth(connection, String.format("%s:%s", username, password));
   }
 
   /** Sets auth on the given connection with the given string, formatted "username:password". */
-  public static void setBasicAuth(URLConnection connection, String usernameAndPassword) {
+  public static void setBasicAuth(HttpURLConnection connection, String usernameAndPassword) {
     String token = base64().encode(usernameAndPassword.getBytes(UTF_8));
     connection.setRequestProperty(AUTHORIZATION, "Basic " + token);
   }
 
   /** Sets the given byte[] payload on the given connection with a particular content type. */
-  public static void setPayload(URLConnection connection, byte[] bytes, String contentType)
+  public static void setPayload(HttpURLConnection connection, byte[] bytes, String contentType)
       throws IOException {
     connection.setRequestProperty(CONTENT_TYPE, contentType);
     connection.setDoOutput(true);
@@ -72,7 +84,7 @@ public final class UrlConnectionUtils {
    * @see <a href="http://www.ietf.org/rfc/rfc2388.txt">RFC2388 - Returning Values from Forms</a>
    */
   public static void setPayloadMultipart(
-      URLConnection connection,
+      HttpURLConnection connection,
       String name,
       String filename,
       MediaType contentType,

core/src/test/java/google/registry/rde/RdeReportActionTest.java

@@ -263,7 +263,7 @@ public class RdeReportActionTest {
   @Test
   void testRunWithLock_badRequest_throws500WithErrorInfo() throws Exception {
     when(httpUrlConnection.getResponseCode()).thenReturn(STATUS_CODE_BAD_REQUEST);
-    when(httpUrlConnection.getInputStream()).thenReturn(IIRDEA_BAD_XML.openBufferedStream());
+    when(httpUrlConnection.getErrorStream()).thenReturn(IIRDEA_BAD_XML.openBufferedStream());
     InternalServerErrorException thrown =
         assertThrows(
             InternalServerErrorException.class,

core/src/test/java/google/registry/reporting/icann/IcannHttpReporterTest.java

@@ -99,10 +99,10 @@ class IcannHttpReporterTest {
 
   @Test
   void testFail_BadIirdeaResponse() throws Exception {
-    when(connection.getInputStream()).thenReturn(IIRDEA_BAD_XML.openBufferedStream());
     when(connection.getResponseCode()).thenReturn(STATUS_CODE_BAD_REQUEST);
+    when(connection.getErrorStream()).thenReturn(IIRDEA_BAD_XML.openBufferedStream());
     assertThat(reporter.send(FAKE_PAYLOAD, "test-transactions-201706.csv")).isFalse();
-    verify(connection).getInputStream();
+    verify(connection).getErrorStream();
   }
 
   @Test

core/src/test/java/google/registry/request/UrlConnectionUtilsTest.java

@@ -29,6 +29,7 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
+import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.util.Arrays;
 import java.util.List;
@@ -104,4 +105,21 @@ public class UrlConnectionUtilsTest {
             "Multipart data contains autogenerated boundary: "
                 + "------------------------------AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA");
   }
+
+  @Test
+  void testErrorStream() throws Exception {
+    HttpsURLConnection connection = mock(HttpsURLConnection.class);
+    when(connection.getResponseCode()).thenReturn(400);
+    when(connection.getErrorStream())
+        .thenReturn(new ByteArrayInputStream("Failure".getBytes(UTF_8)));
+    assertThat(UrlConnectionUtils.getResponseBytes(connection))
+        .isEqualTo("Failure".getBytes(UTF_8));
+  }
+
+  @Test
+  void testErrorStream_null() throws Exception {
+    HttpsURLConnection connection = mock(HttpsURLConnection.class);
+    when(connection.getResponseCode()).thenReturn(400);
+    assertThat(UrlConnectionUtils.getResponseBytes(connection)).isEmpty();
+  }
 }

core/src/test/java/google/registry/tmch/NordnUploadActionTest.java

@@ -108,6 +108,8 @@ class NordnUploadActionTest {
   void beforeEach() throws Exception {
     when(httpUrlConnection.getInputStream())
         .thenReturn(new ByteArrayInputStream("Success".getBytes(UTF_8)));
+    when(httpUrlConnection.getErrorStream())
+        .thenReturn(new ByteArrayInputStream("Failure".getBytes(UTF_8)));
     when(httpUrlConnection.getResponseCode()).thenReturn(SC_ACCEPTED);
     when(httpUrlConnection.getHeaderField(LOCATION)).thenReturn("http://trololol");
     when(httpUrlConnection.getOutputStream()).thenReturn(connectionOutputStream);

release/cloudbuild-deploy.yaml

@@ -78,8 +78,8 @@ steps:
       grep -e "^backend\|^default\|^bsa\|^pubapi\|^tools" |\
       while read line; do echo "${TAG_NAME},$line"; done | tee "$local_map"
     num_versions=$(cat "$local_map" | wc -l)
-    if [ "$num_versions" -ne 4 ]; then
-      echo "Expecting exactly four active services. Found $num_versions"
+    if [ "$num_versions" -ne 5 ]; then
+      echo "Expecting exactly five active services. Found $num_versions"
       exit 1
     fi
     gsutil cp "$local_map" gs://$PROJECT_ID-deployed-tags/nomulus.${_ENV}.tmp