Merge pull request #6861 from…

Merge pull request #6861 from chrisfillmore:feature/responseBodyForInvalidResponseCodeException_6853

PiperOrigin-RevId: 314105612

RELEASENOTES.md

@@ -88,6 +88,8 @@
     *   Extend `EventTime` with more details about the current player state for
         easier access
         ([#7332](https://github.com/google/ExoPlayer/issues/7332)).
+    *   Add `HttpDataSource.InvalidResponseCodeException#responseBody` field
+        ([#6853](https://github.com/google/ExoPlayer/issues/6853)).
 *   Video: Pass frame rate hint to `Surface.setFrameRate` on Android R devices.
 *   Text:
     *   Parse `<ruby>` and `<rt>` tags in WebVTT subtitles (rendering is coming

extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java

@@ -31,11 +31,13 @@ import com.google.android.exoplayer2.util.Clock;
 import com.google.android.exoplayer2.util.ConditionVariable;
 import com.google.android.exoplayer2.util.Log;
 import com.google.android.exoplayer2.util.Predicate;
+import com.google.android.exoplayer2.util.Util;
 import java.io.IOException;
 import java.io.InterruptedIOException;
 import java.net.SocketTimeoutException;
 import java.net.UnknownHostException;
 import java.nio.ByteBuffer;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -440,12 +442,28 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
     UrlResponseInfo responseInfo = Assertions.checkNotNull(this.responseInfo);
     int responseCode = responseInfo.getHttpStatusCode();
     if (responseCode < 200 || responseCode > 299) {
+      byte[] responseBody = Util.EMPTY_BYTE_ARRAY;
+      ByteBuffer readBuffer = getOrCreateReadBuffer();
+      while (!readBuffer.hasRemaining()) {
+        operation.close();
+        readBuffer.clear();
+        readInternal(readBuffer);
+        if (finished) {
+          break;
+        }
+        readBuffer.flip();
+        int existingResponseBodyEnd = responseBody.length;
+        responseBody = Arrays.copyOf(responseBody, responseBody.length + readBuffer.remaining());
+        readBuffer.get(responseBody, existingResponseBodyEnd, readBuffer.remaining());
+      }
+
       InvalidResponseCodeException exception =
           new InvalidResponseCodeException(
               responseCode,
               responseInfo.getHttpStatusText(),
               responseInfo.getAllHeaders(),
-              dataSpec);
+              dataSpec,
+              responseBody);
       if (responseCode == 416) {
         exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE));
       }
@@ -496,17 +514,12 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
       return C.RESULT_END_OF_INPUT;
     }
 
-    ByteBuffer readBuffer = this.readBuffer;
-    if (readBuffer == null) {
-      readBuffer = ByteBuffer.allocateDirect(READ_BUFFER_SIZE_BYTES);
-      readBuffer.limit(0);
-      this.readBuffer = readBuffer;
-    }
+    ByteBuffer readBuffer = getOrCreateReadBuffer();
     while (!readBuffer.hasRemaining()) {
       // Fill readBuffer with more data from Cronet.
       operation.close();
       readBuffer.clear();
-      readInternal(castNonNull(readBuffer));
+      readInternal(readBuffer);
 
       if (finished) {
         bytesRemaining = 0;
@@ -603,11 +616,8 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
       operation.close();
 
       if (!useCallerBuffer) {
-        if (readBuffer == null) {
-          readBuffer = ByteBuffer.allocateDirect(READ_BUFFER_SIZE_BYTES);
-        } else {
-          readBuffer.clear();
-        }
+        ByteBuffer readBuffer = getOrCreateReadBuffer();
+        readBuffer.clear();
         if (bytesToSkip < READ_BUFFER_SIZE_BYTES) {
           readBuffer.limit((int) bytesToSkip);
         }
@@ -781,6 +791,14 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
     }
   }
 
+  private ByteBuffer getOrCreateReadBuffer() {
+    if (readBuffer == null) {
+      readBuffer = ByteBuffer.allocateDirect(READ_BUFFER_SIZE_BYTES);
+      readBuffer.limit(0);
+    }
+    return readBuffer;
+  }
+
   private static boolean isCompressed(UrlResponseInfo info) {
     for (Map.Entry<String, String> entry : info.getAllHeadersAsList()) {
       if (entry.getKey().equalsIgnoreCase("Content-Encoding")) {
@@ -893,7 +911,11 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
         if (responseCode == 307 || responseCode == 308) {
           exception =
               new InvalidResponseCodeException(
-                  responseCode, info.getHttpStatusText(), info.getAllHeaders(), dataSpec);
+                  responseCode,
+                  info.getHttpStatusText(),
+                  info.getAllHeaders(),
+                  dataSpec,
+                  /* responseBody= */ Util.EMPTY_BYTE_ARRAY);
           operation.open();
           return;
         }

extensions/cronet/src/test/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java

@@ -378,15 +378,18 @@ public final class CronetDataSourceTest {
   }
 
   @Test
-  public void requestOpenValidatesStatusCode() {
+  public void requestOpenPropagatesFailureResponseBody() throws Exception {
     mockResponseStartSuccess();
-    testUrlResponseInfo = createUrlResponseInfo(500); // statusCode
+    // Use a size larger than CronetDataSource.READ_BUFFER_SIZE_BYTES
+    int responseLength = 40 * 1024;
+    mockReadSuccess(/* position= */ 0, /* length= */ responseLength);
+    testUrlResponseInfo = createUrlResponseInfo(/* statusCode= */ 500);
 
     try {
       dataSourceUnderTest.open(testDataSpec);
-      fail("HttpDataSource.HttpDataSourceException expected");
-    } catch (HttpDataSourceException e) {
-      assertThat(e).isInstanceOf(HttpDataSource.InvalidResponseCodeException.class);
+      fail("HttpDataSource.InvalidResponseCodeException expected");
+    } catch (HttpDataSource.InvalidResponseCodeException e) {
+      assertThat(e.responseBody).isEqualTo(buildTestDataArray(0, responseLength));
       // Check for connection not automatically closed.
       verify(mockUrlRequest, never()).cancel();
       verify(mockTransferListener, never())

extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java

@@ -230,10 +230,18 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource {
 
     // Check for a valid response code.
     if (!response.isSuccessful()) {
+      byte[] errorResponseBody;
+      try {
+        errorResponseBody = Util.toByteArray(Assertions.checkNotNull(responseByteStream));
+      } catch (IOException e) {
+        throw new HttpDataSourceException(
+            "Error reading non-2xx response body", e, dataSpec, HttpDataSourceException.TYPE_OPEN);
+      }
       Map<String, List<String>> headers = response.headers().toMultimap();
       closeConnectionQuietly();
       InvalidResponseCodeException exception =
-          new InvalidResponseCodeException(responseCode, response.message(), headers, dataSpec);
+          new InvalidResponseCodeException(
+              responseCode, response.message(), headers, dataSpec, errorResponseBody);
       if (responseCode == 416) {
         exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE));
       }

extensions/okhttp/src/test/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSourceTest.java

@@ -17,10 +17,16 @@
 package com.google.android.exoplayer2.ext.okhttp;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import androidx.test.ext.junit.runners.AndroidJUnit4;
+import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.upstream.DataSpec;
 import com.google.android.exoplayer2.upstream.HttpDataSource;
+import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 import okhttp3.Call;
@@ -38,24 +44,25 @@ import org.mockito.Mockito;
 @RunWith(AndroidJUnit4.class)
 public class OkHttpDataSourceTest {
 
+  /**
+   * This test will set HTTP default request parameters (1) in the OkHttpDataSource, (2) via
+   * OkHttpDataSource.setRequestProperty() and (3) in the DataSpec instance according to the table
+   * below. Values wrapped in '*' are the ones that should be set in the connection request.
+   *
+   * <pre>{@code
+   * +-----------------------+---+-----+-----+-----+-----+-----+
+   * |                       |            Header Key           |
+   * +-----------------------+---+-----+-----+-----+-----+-----+
+   * |       Location        | 0 |  1  |  2  |  3  |  4  |  5  |
+   * +-----------------------+---+-----+-----+-----+-----+-----+
+   * | Default               |*Y*|  Y  |  Y  |     |     |     |
+   * | OkHttpDataSource      |   | *Y* |  Y  |  Y  | *Y* |     |
+   * | DataSpec              |   |     | *Y* | *Y* |     | *Y* |
+   * +-----------------------+---+-----+-----+-----+-----+-----+
+   * }</pre>
+   */
   @Test
   public void open_setsCorrectHeaders() throws HttpDataSource.HttpDataSourceException {
-    /*
-     * This test will set HTTP default request parameters (1) in the OkHttpDataSource, (2) via
-     * OkHttpDataSource.setRequestProperty() and (3) in the DataSpec instance according to the table
-     * below. Values wrapped in '*' are the ones that should be set in the connection request.
-     *
-     * +-----------------------+---+-----+-----+-----+-----+-----+
-     * |                       |            Header Key           |
-     * +-----------------------+---+-----+-----+-----+-----+-----+
-     * |       Location        | 0 |  1  |  2  |  3  |  4  |  5  |
-     * +-----------------------+---+-----+-----+-----+-----+-----+
-     * | Default               |*Y*|  Y  |  Y  |     |     |     |
-     * | OkHttpDataSource      |   | *Y* |  Y  |  Y  | *Y* |     |
-     * | DataSpec              |   |     | *Y* | *Y* |     | *Y* |
-     * +-----------------------+---+-----+-----+-----+-----+-----+
-     */
-
     String defaultValue = "Default";
     String okHttpDataSourceValue = "OkHttpDataSource";
     String dataSpecValue = "DataSpec";
@@ -67,7 +74,7 @@ public class OkHttpDataSourceTest {
     defaultRequestProperties.set("1", defaultValue);
     defaultRequestProperties.set("2", defaultValue);
 
-    Call.Factory mockCallFactory = Mockito.mock(Call.Factory.class);
+    Call.Factory mockCallFactory = mock(Call.Factory.class);
     OkHttpDataSource okHttpDataSource =
         new OkHttpDataSource(
             mockCallFactory, "testAgent", /* cacheControl= */ null, defaultRequestProperties);
@@ -103,8 +110,8 @@ public class OkHttpDataSourceTest {
               assertThat(request.header("5")).isEqualTo(dataSpecValue);
 
               // return a Call whose .execute() will return a mock Response
-              Call returnValue = Mockito.mock(Call.class);
-              Mockito.doReturn(
+              Call returnValue = mock(Call.class);
+              doReturn(
                       new Response.Builder()
                           .request(request)
                           .protocol(Protocol.HTTP_1_1)
@@ -120,4 +127,44 @@ public class OkHttpDataSourceTest {
         .newCall(ArgumentMatchers.any());
     okHttpDataSource.open(dataSpec);
   }
+
+  @Test
+  public void open_invalidResponseCode() throws Exception {
+    Call.Factory callFactory =
+        request -> {
+          Call mockCall = mock(Call.class);
+
+          try {
+            when(mockCall.execute())
+                .thenReturn(
+                    new Response.Builder()
+                        .request(request)
+                        .protocol(Protocol.HTTP_1_1)
+                        .code(404)
+                        .message("NOT FOUND")
+                        .body(ResponseBody.create(MediaType.parse("text/plain"), "failure msg"))
+                        .build());
+          } catch (IOException e) {
+            throw new AssertionError(e);
+          }
+          return mockCall;
+        };
+
+    OkHttpDataSource okHttpDataSource =
+        new OkHttpDataSource(
+            callFactory,
+            "testAgent",
+            /* cacheControl= */ null,
+            /* defaultRequestProperties= */ null);
+
+    DataSpec dataSpec = new DataSpec.Builder().setUri("http://www.google.com").build();
+
+    HttpDataSource.InvalidResponseCodeException exception =
+        assertThrows(
+            HttpDataSource.InvalidResponseCodeException.class,
+            () -> okHttpDataSource.open(dataSpec));
+
+    assertThat(exception.responseCode).isEqualTo(404);
+    assertThat(exception.responseBody).isEqualTo("failure msg".getBytes(C.UTF8_NAME));
+  }
 }

library/common/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java

@@ -306,22 +306,51 @@ public interface HttpDataSource extends DataSource {
      */
     public final Map<String, List<String>> headerFields;
 
-    /** @deprecated Use {@link #InvalidResponseCodeException(int, String, Map, DataSpec)}. */
+    /** The response body. */
+    public final byte[] responseBody;
+
+    /**
+     * @deprecated Use {@link #InvalidResponseCodeException(int, String, Map, DataSpec, byte[])}.
+     */
     @Deprecated
     public InvalidResponseCodeException(
         int responseCode, Map<String, List<String>> headerFields, DataSpec dataSpec) {
-      this(responseCode, /* responseMessage= */ null, headerFields, dataSpec);
+      this(
+          responseCode,
+          /* responseMessage= */ null,
+          headerFields,
+          dataSpec,
+          /* responseBody= */ Util.EMPTY_BYTE_ARRAY);
     }
 
+    /**
+     * @deprecated Use {@link #InvalidResponseCodeException(int, String, Map, DataSpec, byte[])}.
+     */
+    @Deprecated
     public InvalidResponseCodeException(
         int responseCode,
         @Nullable String responseMessage,
         Map<String, List<String>> headerFields,
         DataSpec dataSpec) {
+      this(
+          responseCode,
+          responseMessage,
+          headerFields,
+          dataSpec,
+          /* responseBody= */ Util.EMPTY_BYTE_ARRAY);
+    }
+
+    public InvalidResponseCodeException(
+        int responseCode,
+        @Nullable String responseMessage,
+        Map<String, List<String>> headerFields,
+        DataSpec dataSpec,
+        byte[] responseBody) {
       super("Response code: " + responseCode, dataSpec, TYPE_OPEN);
       this.responseCode = responseCode;
       this.responseMessage = responseMessage;
       this.headerFields = headerFields;
+      this.responseBody = responseBody;
     }
 
   }

library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java

@@ -296,9 +296,20 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou
     // Check for a valid response code.
     if (responseCode < 200 || responseCode > 299) {
       Map<String, List<String>> headers = connection.getHeaderFields();
+      @Nullable InputStream errorStream = connection.getErrorStream();
+      byte[] errorResponseBody;
+      try {
+        errorResponseBody =
+            errorStream != null ? Util.toByteArray(errorStream) : Util.EMPTY_BYTE_ARRAY;
+      } catch (IOException e) {
+        throw new HttpDataSourceException(
+            "Error reading non-2xx response body", e, dataSpec, HttpDataSourceException.TYPE_OPEN);
+      }
       closeConnectionQuietly();
       InvalidResponseCodeException exception =
-          new InvalidResponseCodeException(responseCode, responseMessage, headers, dataSpec);
+          new InvalidResponseCodeException(
+              responseCode, responseMessage, headers, dataSpec, errorResponseBody);
+
       if (responseCode == 416) {
         exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE));
       }
@@ -540,7 +551,7 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou
     connection.setInstanceFollowRedirects(followRedirects);
     connection.setDoOutput(httpBody != null);
     connection.setRequestMethod(DataSpec.getStringForHttpMethod(httpMethod));
-    
+
     if (httpBody != null) {
       connection.setFixedLengthStreamingMode(httpBody.length);
       connection.connect();

library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceTest.java

@@ -17,8 +17,13 @@
 package com.google.android.exoplayer2.upstream;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.when;
 
 import androidx.test.ext.junit.runners.AndroidJUnit4;
+import com.google.android.exoplayer2.testutil.TestUtil;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -34,25 +39,25 @@ import org.mockito.Mockito;
 @RunWith(AndroidJUnit4.class)
 public class DefaultHttpDataSourceTest {
 
+  /**
+   * This test will set HTTP default request parameters (1) in the DefaultHttpDataSource, (2) via
+   * DefaultHttpDataSource.setRequestProperty() and (3) in the DataSpec instance according to the
+   * table below. Values wrapped in '*' are the ones that should be set in the connection request.
+   *
+   * <pre>{@code
+   * +-----------------------+---+-----+-----+-----+-----+-----+
+   * |                       |            Header Key           |
+   * +-----------------------+---+-----+-----+-----+-----+-----+
+   * |       Location        | 0 |  1  |  2  |  3  |  4  |  5  |
+   * +-----------------------+---+-----+-----+-----+-----+-----+
+   * | Default               |*Y*|  Y  |  Y  |     |     |     |
+   * | DefaultHttpDataSource |   | *Y* |  Y  |  Y  | *Y* |     |
+   * | DataSpec              |   |     | *Y* | *Y* |     | *Y* |
+   * +-----------------------+---+-----+-----+-----+-----+-----+
+   * }</pre>
+   */
   @Test
   public void open_withSpecifiedRequestParameters_usesCorrectParameters() throws IOException {
-
-    /*
-     * This test will set HTTP default request parameters (1) in the DefaultHttpDataSource, (2) via
-     * DefaultHttpDataSource.setRequestProperty() and (3) in the DataSpec instance according to the
-     * table below. Values wrapped in '*' are the ones that should be set in the connection request.
-     *
-     * +-----------------------+---+-----+-----+-----+-----+-----+
-     * |                       |            Header Key           |
-     * +-----------------------+---+-----+-----+-----+-----+-----+
-     * |       Location        | 0 |  1  |  2  |  3  |  4  |  5  |
-     * +-----------------------+---+-----+-----+-----+-----+-----+
-     * | Default               |*Y*|  Y  |  Y  |     |     |     |
-     * | DefaultHttpDataSource |   | *Y* |  Y  |  Y  | *Y* |     |
-     * | DataSpec              |   |     | *Y* | *Y* |     | *Y* |
-     * +-----------------------+---+-----+-----+-----+-----+-----+
-     */
-
     String defaultParameter = "Default";
     String dataSourceInstanceParameter = "DefaultHttpDataSource";
     String dataSpecParameter = "Dataspec";
@@ -72,10 +77,8 @@ public class DefaultHttpDataSourceTest {
                 defaultParameters));
 
     Map<String, String> sentRequestProperties = new HashMap<>();
-    HttpURLConnection mockHttpUrlConnection = makeMockHttpUrlConnection(sentRequestProperties);
-    Mockito.doReturn(mockHttpUrlConnection)
-        .when(defaultHttpDataSource)
-        .openConnection(ArgumentMatchers.any());
+    HttpURLConnection mockHttpUrlConnection = make200MockHttpUrlConnection(sentRequestProperties);
+    doReturn(mockHttpUrlConnection).when(defaultHttpDataSource).openConnection(any());
 
     defaultHttpDataSource.setRequestProperty("1", dataSourceInstanceParameter);
     defaultHttpDataSource.setRequestProperty("2", dataSourceInstanceParameter);
@@ -106,22 +109,48 @@ public class DefaultHttpDataSourceTest {
     assertThat(sentRequestProperties.get("5")).isEqualTo(dataSpecParameter);
   }
 
+  @Test
+  public void open_invalidResponseCode() throws Exception {
+    DefaultHttpDataSource defaultHttpDataSource =
+        Mockito.spy(
+            new DefaultHttpDataSource(
+                /* userAgent= */ "testAgent",
+                /* connectTimeoutMillis= */ 1000,
+                /* readTimeoutMillis= */ 1000,
+                /* allowCrossProtocolRedirects= */ false,
+                /* defaultRequestProperties= */ null));
+
+    HttpURLConnection mockHttpUrlConnection =
+        make404MockHttpUrlConnection(/* responseData= */ TestUtil.createByteArray(1, 2, 3));
+    doReturn(mockHttpUrlConnection).when(defaultHttpDataSource).openConnection(any());
+
+    DataSpec dataSpec = new DataSpec.Builder().setUri("http://www.google.com").build();
+
+    HttpDataSource.InvalidResponseCodeException exception =
+        assertThrows(
+            HttpDataSource.InvalidResponseCodeException.class,
+            () -> defaultHttpDataSource.open(dataSpec));
+
+    assertThat(exception.responseCode).isEqualTo(404);
+    assertThat(exception.responseBody).isEqualTo(TestUtil.createByteArray(1, 2, 3));
+  }
+
   /**
    * Creates a mock {@link HttpURLConnection} that stores all request parameters inside {@code
    * requestProperties}.
    */
-  private static HttpURLConnection makeMockHttpUrlConnection(Map<String, String> requestProperties)
-      throws IOException {
+  private static HttpURLConnection make200MockHttpUrlConnection(
+      Map<String, String> requestProperties) throws IOException {
     HttpURLConnection mockHttpUrlConnection = Mockito.mock(HttpURLConnection.class);
-    Mockito.when(mockHttpUrlConnection.usingProxy()).thenReturn(false);
+    when(mockHttpUrlConnection.usingProxy()).thenReturn(false);
 
-    Mockito.when(mockHttpUrlConnection.getInputStream())
+    when(mockHttpUrlConnection.getInputStream())
         .thenReturn(new ByteArrayInputStream(new byte[128]));
 
-    Mockito.when(mockHttpUrlConnection.getOutputStream()).thenReturn(new ByteArrayOutputStream());
+    when(mockHttpUrlConnection.getOutputStream()).thenReturn(new ByteArrayOutputStream());
 
-    Mockito.when(mockHttpUrlConnection.getResponseCode()).thenReturn(200);
-    Mockito.when(mockHttpUrlConnection.getResponseMessage()).thenReturn("OK");
+    when(mockHttpUrlConnection.getResponseCode()).thenReturn(200);
+    when(mockHttpUrlConnection.getResponseMessage()).thenReturn("OK");
 
     Mockito.doAnswer(
             (invocation) -> {
@@ -135,4 +164,15 @@ public class DefaultHttpDataSourceTest {
 
     return mockHttpUrlConnection;
   }
+
+  /** Creates a mock {@link HttpURLConnection} that returns a 404 response code. */
+  private static HttpURLConnection make404MockHttpUrlConnection(byte[] responseData)
+      throws IOException {
+    HttpURLConnection mockHttpUrlConnection = Mockito.mock(HttpURLConnection.class);
+    when(mockHttpUrlConnection.getOutputStream()).thenReturn(new ByteArrayOutputStream());
+    when(mockHttpUrlConnection.getErrorStream()).thenReturn(new ByteArrayInputStream(responseData));
+    when(mockHttpUrlConnection.getResponseCode()).thenReturn(404);
+    when(mockHttpUrlConnection.getResponseMessage()).thenReturn("NOT FOUND");
+    return mockHttpUrlConnection;
+  }
 }