diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 03b3199e216..c90129e7ba1 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -2,6 +2,10 @@ ### dev-v2 (not yet released) ### +* Add `DataSpec.httpRequestHeaders` to set HTTP request headers when connecting + to an HTTP source. `DefaultHttpDataSource`, `CronetDataSource` and + `OkHttpDataSource` include headers set in the DataSpec when connecting to the + source. * Bypass sniffing in `ProgressiveMediaPeriod` in case a single extractor is provided ([#6325](https://github.com/google/ExoPlayer/issues/6325)). * Surface information provided by methods `isHardwareAccelerated`, diff --git a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java index 241b879b9a9..91bceb9aee6 100644 --- a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java +++ b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java @@ -37,6 +37,7 @@ import java.net.UnknownHostException; import java.nio.ByteBuffer; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -54,7 +55,9 @@ /** * DataSource without intermediate buffer based on Cronet API set using UrlRequest. * - *

This class's methods are organized in the sequence of expected calls. + *

Note: HTTP request headers will be set using all parameters passed via (in order of decreasing + * priority) the {@code dataSpec}, {@link #setRequestProperty} and the default parameters used to + * construct the instance. */ public class CronetDataSource extends BaseDataSource implements HttpDataSource { @@ -685,22 +688,22 @@ private UrlRequest.Builder buildRequestBuilder(DataSpec dataSpec) throws IOExcep cronetEngine .newUrlRequestBuilder(dataSpec.uri.toString(), urlRequestCallback, executor) .allowDirectExecutor(); + // Set the headers. - boolean isContentTypeHeaderSet = false; + Map requestHeaders = new HashMap<>(); if (defaultRequestProperties != null) { - for (Entry headerEntry : defaultRequestProperties.getSnapshot().entrySet()) { - String key = headerEntry.getKey(); - isContentTypeHeaderSet = isContentTypeHeaderSet || CONTENT_TYPE.equals(key); - requestBuilder.addHeader(key, headerEntry.getValue()); - } + requestHeaders.putAll(defaultRequestProperties.getSnapshot()); } - Map requestPropertiesSnapshot = requestProperties.getSnapshot(); - for (Entry headerEntry : requestPropertiesSnapshot.entrySet()) { + requestHeaders.putAll(requestProperties.getSnapshot()); + requestHeaders.putAll(dataSpec.httpRequestHeaders); + + for (Entry headerEntry : requestHeaders.entrySet()) { String key = headerEntry.getKey(); - isContentTypeHeaderSet = isContentTypeHeaderSet || CONTENT_TYPE.equals(key); - requestBuilder.addHeader(key, headerEntry.getValue()); + String value = headerEntry.getValue(); + requestBuilder.addHeader(key, value); } - if (dataSpec.httpBody != null && !isContentTypeHeaderSet) { + + if (dataSpec.httpBody != null && !requestHeaders.containsKey(CONTENT_TYPE)) { throw new IOException("HTTP request with non-empty body must set Content-Type"); } if (dataSpec.isFlagSet(DataSpec.FLAG_ALLOW_ICY_METADATA)) { diff --git a/extensions/cronet/src/test/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java b/extensions/cronet/src/test/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java index 2be369bad94..47f6fa7d2f6 100644 --- a/extensions/cronet/src/test/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java +++ b/extensions/cronet/src/test/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java @@ -60,6 +60,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentMatchers; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -95,6 +96,12 @@ public final class CronetDataSourceTest { @Before public void setUp() { MockitoAnnotations.initMocks(this); + + HttpDataSource.RequestProperties defaultRequestProperties = + new HttpDataSource.RequestProperties(); + defaultRequestProperties.set("defaultHeader1", "defaultValue1"); + defaultRequestProperties.set("defaultHeader2", "defaultValue2"); + dataSourceUnderTest = new CronetDataSource( mockCronetEngine, @@ -103,7 +110,7 @@ public void setUp() { TEST_READ_TIMEOUT_MS, /* resetTimeoutOnRedirects= */ true, Clock.DEFAULT, - /* defaultRequestProperties= */ null, + defaultRequestProperties, /* handleSetCookieRequests= */ false); dataSourceUnderTest.addTransferListener(mockTransferListener); when(mockCronetEngine.newUrlRequestBuilder( @@ -189,18 +196,59 @@ public void testRequestStartCalled() throws HttpDataSourceException { } @Test - public void testRequestHeadersSet() throws HttpDataSourceException { + public void testRequestSetsRangeHeader() throws HttpDataSourceException { testDataSpec = new DataSpec(Uri.parse(TEST_URL), 1000, 5000, null); mockResponseStartSuccess(); - dataSourceUnderTest.setRequestProperty("firstHeader", "firstValue"); - dataSourceUnderTest.setRequestProperty("secondHeader", "secondValue"); - dataSourceUnderTest.open(testDataSpec); // The header value to add is current position to current position + length - 1. verify(mockUrlRequestBuilder).addHeader("Range", "bytes=1000-5999"); - verify(mockUrlRequestBuilder).addHeader("firstHeader", "firstValue"); - verify(mockUrlRequestBuilder).addHeader("secondHeader", "secondValue"); + } + + @Test + public void testRequestHeadersSet() throws HttpDataSourceException { + + Map headersSet = new HashMap<>(); + doAnswer( + (invocation) -> { + String key = invocation.getArgument(0); + String value = invocation.getArgument(1); + headersSet.put(key, value); + return null; + }) + .when(mockUrlRequestBuilder) + .addHeader(ArgumentMatchers.anyString(), ArgumentMatchers.anyString()); + + dataSourceUnderTest.setRequestProperty("defaultHeader2", "dataSourceOverridesDefault"); + dataSourceUnderTest.setRequestProperty("dataSourceHeader1", "dataSourceValue1"); + dataSourceUnderTest.setRequestProperty("dataSourceHeader2", "dataSourceValue2"); + + Map dataSpecRequestProperties = new HashMap<>(); + dataSpecRequestProperties.put("defaultHeader3", "dataSpecOverridesAll"); + dataSpecRequestProperties.put("dataSourceHeader2", "dataSpecOverridesDataSource"); + dataSpecRequestProperties.put("dataSpecHeader1", "dataSpecValue1"); + testDataSpec = + new DataSpec( + /* uri= */ Uri.parse(TEST_URL), + /* httpMethod= */ DataSpec.HTTP_METHOD_GET, + /* httpBody= */ null, + /* absoluteStreamPosition= */ 1000, + /* position= */ 1000, + /* length= */ 5000, + /* key= */ null, + /* flags= */ 0, + dataSpecRequestProperties); + mockResponseStartSuccess(); + + dataSourceUnderTest.open(testDataSpec); + + assertThat(headersSet.get("defaultHeader1")).isEqualTo("defaultValue1"); + assertThat(headersSet.get("defaultHeader2")).isEqualTo("dataSourceOverridesDefault"); + assertThat(headersSet.get("defaultHeader3")).isEqualTo("dataSpecOverridesAll"); + assertThat(headersSet.get("dataSourceHeader1")).isEqualTo("dataSourceValue1"); + assertThat(headersSet.get("dataSourceHeader2")).isEqualTo("dataSpecOverridesDataSource"); + assertThat(headersSet.get("dataSpecHeader1")).isEqualTo("dataSpecValue1"); + verify(mockUrlRequest).start(); } @@ -241,6 +289,26 @@ public void testRequestOpenFail() { } } + @Test + public void open_ifBodyIsSetWithoutContentTypeHeader_fails() { + testDataSpec = + new DataSpec( + /* uri= */ Uri.parse(TEST_URL), + /* postBody= */ new byte[1024], + /* absoluteStreamPosition= */ 200, + /* position= */ 200, + /* length= */ 1024, + /* key= */ "key", + /* flags= */ 0); + + try { + dataSourceUnderTest.open(testDataSpec); + fail(); + } catch (IOException expected) { + // Expected + } + } + @Test public void testRequestOpenFailDueToDnsFailure() { mockResponseStartFailure(); diff --git a/extensions/okhttp/build.gradle b/extensions/okhttp/build.gradle index b0d9fb0187e..7f638e3c236 100644 --- a/extensions/okhttp/build.gradle +++ b/extensions/okhttp/build.gradle @@ -35,6 +35,8 @@ dependencies { implementation project(modulePrefix + 'library-core') implementation 'androidx.annotation:annotation:' + androidxAnnotationVersion compileOnly 'org.checkerframework:checker-qual:' + checkerframeworkVersion + testImplementation project(modulePrefix + 'testutils') + testImplementation 'org.robolectric:robolectric:' + robolectricVersion api 'com.squareup.okhttp3:okhttp:3.12.1' } diff --git a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java index 1a6a67b140e..2a8f6715770 100644 --- a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java +++ b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java @@ -34,6 +34,7 @@ import java.io.InputStream; import java.io.InterruptedIOException; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import okhttp3.CacheControl; @@ -45,7 +46,13 @@ import okhttp3.Response; import okhttp3.ResponseBody; -/** An {@link HttpDataSource} that delegates to Square's {@link Call.Factory}. */ +/** + * An {@link HttpDataSource} that delegates to Square's {@link Call.Factory}. + * + *

Note: HTTP request headers will be set using all parameters passed via (in order of decreasing + * priority) the {@code dataSpec}, {@link #setRequestProperty} and the default parameters used to + * construct the instance. + */ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { static { @@ -328,14 +335,19 @@ private Request makeRequest(DataSpec dataSpec) throws HttpDataSourceException { if (cacheControl != null) { builder.cacheControl(cacheControl); } + + Map headers = new HashMap<>(); if (defaultRequestProperties != null) { - for (Map.Entry property : defaultRequestProperties.getSnapshot().entrySet()) { - builder.header(property.getKey(), property.getValue()); - } + headers.putAll(defaultRequestProperties.getSnapshot()); } - for (Map.Entry property : requestProperties.getSnapshot().entrySet()) { - builder.header(property.getKey(), property.getValue()); + + headers.putAll(requestProperties.getSnapshot()); + headers.putAll(dataSpec.httpRequestHeaders); + + for (Map.Entry header : headers.entrySet()) { + builder.header(header.getKey(), header.getValue()); } + if (!(position == 0 && length == C.LENGTH_UNSET)) { String rangeRequest = "bytes=" + position + "-"; if (length != C.LENGTH_UNSET) { diff --git a/extensions/okhttp/src/test/AndroidManifest.xml b/extensions/okhttp/src/test/AndroidManifest.xml new file mode 100644 index 00000000000..3efff43f6b9 --- /dev/null +++ b/extensions/okhttp/src/test/AndroidManifest.xml @@ -0,0 +1,20 @@ + + + + + + diff --git a/extensions/okhttp/src/test/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSourceTest.java b/extensions/okhttp/src/test/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSourceTest.java new file mode 100644 index 00000000000..dab62b06e88 --- /dev/null +++ b/extensions/okhttp/src/test/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSourceTest.java @@ -0,0 +1,128 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.android.exoplayer2.ext.okhttp; + +import static com.google.common.truth.Truth.assertThat; + +import android.net.Uri; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.android.exoplayer2.upstream.DataSpec; +import com.google.android.exoplayer2.upstream.HttpDataSource; +import java.util.HashMap; +import java.util.Map; +import okhttp3.Call; +import okhttp3.MediaType; +import okhttp3.Protocol; +import okhttp3.Request; +import okhttp3.Response; +import okhttp3.ResponseBody; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentMatchers; +import org.mockito.Mockito; + +/** Unit tests for {@link OkHttpDataSource}. */ +@RunWith(AndroidJUnit4.class) +public class OkHttpDataSourceTest { + + @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"; + + // 1. Default properties on OkHttpDataSource + HttpDataSource.RequestProperties defaultRequestProperties = + new HttpDataSource.RequestProperties(); + defaultRequestProperties.set("0", defaultValue); + defaultRequestProperties.set("1", defaultValue); + defaultRequestProperties.set("2", defaultValue); + + Call.Factory mockCallFactory = Mockito.mock(Call.Factory.class); + OkHttpDataSource okHttpDataSource = + new OkHttpDataSource( + mockCallFactory, "testAgent", /* cacheControl= */ null, defaultRequestProperties); + + // 2. Additional properties set with setRequestProperty(). + okHttpDataSource.setRequestProperty("1", okHttpDataSourceValue); + okHttpDataSource.setRequestProperty("2", okHttpDataSourceValue); + okHttpDataSource.setRequestProperty("3", okHttpDataSourceValue); + okHttpDataSource.setRequestProperty("4", okHttpDataSourceValue); + + // 3. DataSpec properties + Map dataSpecRequestProperties = new HashMap<>(); + dataSpecRequestProperties.put("2", dataSpecValue); + dataSpecRequestProperties.put("3", dataSpecValue); + dataSpecRequestProperties.put("5", dataSpecValue); + + DataSpec dataSpec = + new DataSpec( + /* uri= */ Uri.parse("http://www.google.com"), + /* httpMethod= */ 1, + /* httpBody= */ null, + /* absoluteStreamPosition= */ 1000, + /* position= */ 1000, + /* length= */ 5000, + /* key= */ null, + /* flags= */ 0, + dataSpecRequestProperties); + + Mockito.doAnswer( + invocation -> { + Request request = invocation.getArgument(0); + assertThat(request.header("0")).isEqualTo(defaultValue); + assertThat(request.header("1")).isEqualTo(okHttpDataSourceValue); + assertThat(request.header("2")).isEqualTo(dataSpecValue); + assertThat(request.header("3")).isEqualTo(dataSpecValue); + assertThat(request.header("4")).isEqualTo(okHttpDataSourceValue); + assertThat(request.header("5")).isEqualTo(dataSpecValue); + + // return a Call whose .execute() will return a mock Response + Call returnValue = Mockito.mock(Call.class); + Mockito.doReturn( + new Response.Builder() + .request(request) + .protocol(Protocol.HTTP_1_1) + .code(200) + .message("OK") + .body(ResponseBody.create(MediaType.parse("text/plain"), "")) + .build()) + .when(returnValue) + .execute(); + return returnValue; + }) + .when(mockCallFactory) + .newCall(ArgumentMatchers.any()); + okHttpDataSource.open(dataSpec); + } +} diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java index 4d3aca570eb..63cad8786bb 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java @@ -326,6 +326,13 @@ public InvalidResponseCodeException( } + /** + * Opens the source to read the specified data. + * + *

Note: {@link HttpDataSource} implementations are advised to set request headers passed via + * (in order of decreasing priority) the {@code dataSpec}, {@link #setRequestProperty} and the + * default parameters set in the {@link Factory}. + */ @Override long open(DataSpec dataSpec) throws HttpDataSourceException; @@ -339,6 +346,10 @@ public InvalidResponseCodeException( * Sets the value of a request header. The value will be used for subsequent connections * established by the source. * + *

Note: If the same header is set as a default parameter in the {@link Factory}, then the + * header value set with this method should be preferred when connecting with the data source. See + * {@link #open}. + * * @param name The name of the header field. * @param value The value of the field. */ diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceTest.java index 7c2b63c941b..d724369d93f 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceTest.java @@ -39,9 +39,9 @@ public class DefaultHttpDataSourceTest { public void open_withSpecifiedRequestParameters_usesCorrectParameters() throws IOException { /* - * This test will set HTTP request parameters in the HttpDataSourceFactory (default), - * in the DefaultHttpDataSource instance and in the DataSpec instance according to the table - * below. Values wrapped in '*' are the ones that should be set in the connection request. + * 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 |