Skip to content

Commit

Permalink
Use DataSpec request params in HttpDataSource impls
Browse files Browse the repository at this point in the history
Include Dataspec.httpRequestHeaders in CronetDataSource,
and OkHttpDataSource. Updated documentation of
HttpDataSource.open() to suggest that it should set request
headers (in decreasing priority) from (1) the passed DataSpec,
(2) parameters set with setRequestProperty() and (3) default
parameters set in the HttpDataSource.Factory. No mechanism
has been put in place to enforce this.

PiperOrigin-RevId: 266895574
  • Loading branch information
christosts authored and tonihei committed Sep 5, 2019
1 parent a12c664 commit 260db03
Show file tree
Hide file tree
Showing 9 changed files with 276 additions and 28 deletions.
4 changes: 4 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -54,7 +55,9 @@
/**
* DataSource without intermediate buffer based on Cronet API set using UrlRequest.
*
* <p>This class's methods are organized in the sequence of expected calls.
* <p>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 {

Expand Down Expand Up @@ -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<String, String> requestHeaders = new HashMap<>();
if (defaultRequestProperties != null) {
for (Entry<String, String> headerEntry : defaultRequestProperties.getSnapshot().entrySet()) {
String key = headerEntry.getKey();
isContentTypeHeaderSet = isContentTypeHeaderSet || CONTENT_TYPE.equals(key);
requestBuilder.addHeader(key, headerEntry.getValue());
}
requestHeaders.putAll(defaultRequestProperties.getSnapshot());
}
Map<String, String> requestPropertiesSnapshot = requestProperties.getSnapshot();
for (Entry<String, String> headerEntry : requestPropertiesSnapshot.entrySet()) {
requestHeaders.putAll(requestProperties.getSnapshot());
requestHeaders.putAll(dataSpec.httpRequestHeaders);

for (Entry<String, String> 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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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<String, String> 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<String, String> 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();
}

Expand Down Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions extensions/okhttp/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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}.
*
* <p>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 {
Expand Down Expand Up @@ -328,14 +335,19 @@ private Request makeRequest(DataSpec dataSpec) throws HttpDataSourceException {
if (cacheControl != null) {
builder.cacheControl(cacheControl);
}

Map<String, String> headers = new HashMap<>();
if (defaultRequestProperties != null) {
for (Map.Entry<String, String> property : defaultRequestProperties.getSnapshot().entrySet()) {
builder.header(property.getKey(), property.getValue());
}
headers.putAll(defaultRequestProperties.getSnapshot());
}
for (Map.Entry<String, String> property : requestProperties.getSnapshot().entrySet()) {
builder.header(property.getKey(), property.getValue());

headers.putAll(requestProperties.getSnapshot());
headers.putAll(dataSpec.httpRequestHeaders);

for (Map.Entry<String, String> 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) {
Expand Down
20 changes: 20 additions & 0 deletions extensions/okhttp/src/test/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?xml version="1.0" encoding="utf-8"?>
<!--
~ 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.
-->

<manifest package="com.google.android.exoplayer2.ext.okhttp.test">
<uses-sdk/>
</manifest>
Original file line number Diff line number Diff line change
@@ -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<String, String> 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);
}
}
Loading

0 comments on commit 260db03

Please sign in to comment.