From 06755d4247543fa4e661940bb13b2da5fe47d2c6 Mon Sep 17 00:00:00 2001 From: ibaker Date: Wed, 17 Nov 2021 14:12:37 +0000 Subject: [PATCH] Remove null keys from DefaultHttpDataSource#getResponseHeaders PiperOrigin-RevId: 410507648 --- RELEASENOTES.md | 1 + .../DefaultHttpDataSourceContractTest.java | 5 -- .../upstream/DefaultHttpDataSource.java | 78 ++++++++++++++++++- .../testutil/DataSourceContractTest.java | 5 -- 4 files changed, 77 insertions(+), 12 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 7b20dd6377..99378b7dc1 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -26,6 +26,7 @@ `buildVideoRenderers()` or `buildAudioRenderers()` can access the codec adapter factory and pass it to `MediaCodecRenderer` instances they create. + * Remove null keys from `DefaultHttpDataSource#getResponseHeaders`. * Extractors: * WAV: Add support for RF64 streams ([#9543](https://github.com/google/ExoPlayer/issues/9543). diff --git a/library/datasource/src/androidTest/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceContractTest.java b/library/datasource/src/androidTest/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceContractTest.java index 0976234a63..0fb570f8b9 100644 --- a/library/datasource/src/androidTest/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceContractTest.java +++ b/library/datasource/src/androidTest/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceContractTest.java @@ -20,7 +20,6 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.testutil.DataSourceContractTest; import com.google.android.exoplayer2.testutil.HttpDataSourceTestEnv; import com.google.common.collect.ImmutableList; -import org.junit.Ignore; import org.junit.Rule; import org.junit.runner.RunWith; @@ -44,8 +43,4 @@ public class DefaultHttpDataSourceContractTest extends DataSourceContractTest { protected Uri getNotFoundUri() { return Uri.parse(httpDataSourceTestEnv.getNonexistentUrl()); } - - @Override - @Ignore("internal b/205811776") - public void getResponseHeaders_noNullKeysOrValues() {} } diff --git a/library/datasource/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java b/library/datasource/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java index bcd878fcbc..dd9a64e389 100644 --- a/library/datasource/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java +++ b/library/datasource/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java @@ -29,6 +29,9 @@ import com.google.android.exoplayer2.upstream.DataSpec.HttpMethod; import com.google.android.exoplayer2.util.Log; import com.google.android.exoplayer2.util.Util; import com.google.common.base.Predicate; +import com.google.common.collect.ForwardingMap; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Sets; import com.google.common.net.HttpHeaders; import java.io.IOException; import java.io.InputStream; @@ -39,10 +42,10 @@ import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.NoRouteToHostException; import java.net.URL; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.zip.GZIPInputStream; /** @@ -310,7 +313,18 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou @Override public Map> getResponseHeaders() { - return connection == null ? Collections.emptyMap() : connection.getHeaderFields(); + if (connection == null) { + return ImmutableMap.of(); + } + // connection.getHeaderFields() always contains a null key with a value like + // ["HTTP/1.1 200 OK"]. The response code is available from HttpURLConnection#getResponseCode() + // and the HTTP version is fixed when establishing the connection. + // DataSource#getResponseHeaders() doesn't allow null keys in the returned map, so we need to + // remove it. + // connection.getHeaderFields() returns a special unmodifiable case-insensitive Map + // so we can't just remove the null key or make a copy without the null key. Instead we wrap it + // in a ForwardingMap subclass that ignores and filters out null keys in the read methods. + return new NullFilteringHeadersMap(connection.getHeaderFields()); } @Override @@ -815,4 +829,64 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou String contentEncoding = connection.getHeaderField("Content-Encoding"); return "gzip".equalsIgnoreCase(contentEncoding); } + + private static class NullFilteringHeadersMap extends ForwardingMap> { + + private final Map> headers; + + public NullFilteringHeadersMap(Map> headers) { + this.headers = headers; + } + + @Override + protected Map> delegate() { + return headers; + } + + @Override + public boolean containsKey(@Nullable Object key) { + return key != null && super.containsKey(key); + } + + @Nullable + @Override + public List get(@Nullable Object key) { + return key == null ? null : super.get(key); + } + + @Override + public Set keySet() { + return Sets.filter(super.keySet(), key -> key != null); + } + + @Override + public Set>> entrySet() { + return Sets.filter(super.entrySet(), entry -> entry.getKey() != null); + } + + @Override + public int size() { + return super.size() - (super.containsKey(null) ? 1 : 0); + } + + @Override + public boolean isEmpty() { + return super.isEmpty() || (super.size() == 1 && super.containsKey(null)); + } + + @Override + public boolean containsValue(@Nullable Object value) { + return super.standardContainsValue(value); + } + + @Override + public boolean equals(@Nullable Object object) { + return object != null && super.standardEquals(object); + } + + @Override + public int hashCode() { + return super.standardHashCode(); + } + } } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/DataSourceContractTest.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/DataSourceContractTest.java index 07faf15a6d..0202a4e533 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/DataSourceContractTest.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/DataSourceContractTest.java @@ -545,11 +545,6 @@ public abstract class DataSourceContractTest { Map> responseHeaders = dataSource.getResponseHeaders(); for (String key : responseHeaders.keySet()) { - // TODO(internal b/205811776): Remove this when DefaultHttpDataSource is fixed to not - // return a null key. - if (key == null) { - continue; - } String caseFlippedKey = invertAsciiCaseOfEveryOtherCharacter(key); assertWithMessage("key='%s', caseFlippedKey='%s'", key, caseFlippedKey) .that(responseHeaders.get(caseFlippedKey))