From fff7b8079a1d3cc8340274aff7df8cd3dcb7e8d3 Mon Sep 17 00:00:00 2001 From: ibaker Date: Fri, 16 Apr 2021 11:45:36 +0100 Subject: [PATCH] Replace Util.toUpperInvariant() with Ascii.toUpperCase() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even when fixed to the US locale (and thus avoiding surprising behaviour in e.g. Turkish locale with "i" and "I") there are unexpected behaviours when upper and lower casing non-ASCII characters. For example it's sometimes not symmetric, e.g.: "ẞ".toLowerCase() -> "ß" "ß".toUpperCase() -> "SS" In all the ExoPlayer usages we are either dealing with known-ASCII strings (e.g. MIME types) or comparing against ASCII constant strings anyway, so it seems easier to just use Guava's ASCII-only class in these cases. Util.toUpperInvariant() is null-tolerant, while Ascii.toLowercase() is not. Most usages in this change are clearly non-null. The BandwidthMeter usages aren't annotated @Nullable, but the current code *would* work if countryCode was null in both cases. These methods will now throw NPE if they're passed null. PiperOrigin-RevId: 368816287 --- .../com/google/android/exoplayer2/util/Util.java | 14 ++------------ .../exoplayer2/upstream/DefaultBandwidthMeter.java | 3 ++- .../hls/playlist/HlsMediaPlaylistParserTest.java | 4 ++-- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java b/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java index 2d643673be..2e3e3d8567 100644 --- a/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java +++ b/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java @@ -760,16 +760,6 @@ public final class Util { return c == '\n' || c == '\r'; } - /** - * Converts text to upper case using {@link Locale#US}. - * - * @param text The text to convert. - * @return The upper case text, or null if {@code text} is null. - */ - public static @PolyNull String toUpperInvariant(@PolyNull String text) { - return text == null ? text : text.toUpperCase(Locale.US); - } - /** * Formats a string using {@link Locale#US}. * @@ -2151,11 +2141,11 @@ public final class Util { if (telephonyManager != null) { String countryCode = telephonyManager.getNetworkCountryIso(); if (!TextUtils.isEmpty(countryCode)) { - return toUpperInvariant(countryCode); + return Ascii.toUpperCase(countryCode); } } } - return toUpperInvariant(Locale.getDefault().getCountry()); + return Ascii.toUpperCase(Locale.getDefault().getCountry()); } /** diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeter.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeter.java index 8232d90668..1c0b3f2642 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeter.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeter.java @@ -25,6 +25,7 @@ import com.google.android.exoplayer2.util.Clock; import com.google.android.exoplayer2.util.NetworkTypeObserver; import com.google.android.exoplayer2.util.SlidingPercentile; import com.google.android.exoplayer2.util.Util; +import com.google.common.base.Ascii; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; @@ -170,7 +171,7 @@ public final class DefaultBandwidthMeter implements BandwidthMeter, TransferList */ public Builder setInitialBitrateEstimate(String countryCode) { initialBitrateEstimates = - getInitialBitrateEstimatesForCountry(Util.toUpperInvariant(countryCode)); + getInitialBitrateEstimatesForCountry(Ascii.toUpperCase(countryCode)); return this; } diff --git a/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/playlist/HlsMediaPlaylistParserTest.java b/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/playlist/HlsMediaPlaylistParserTest.java index 48f6a06b14..7cc9a75c1a 100644 --- a/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/playlist/HlsMediaPlaylistParserTest.java +++ b/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/playlist/HlsMediaPlaylistParserTest.java @@ -135,7 +135,7 @@ public class HlsMediaPlaylistParserTest { .isEqualTo("https://priv.example.com/key.php?r=2682"); // 0xA7A == 2682. assertThat(segment.encryptionIV).isNotNull(); - assertThat(Util.toUpperInvariant(segment.encryptionIV)).isEqualTo("A7A"); + assertThat(segment.encryptionIV).ignoringCase().isEqualTo("A7A"); assertThat(segment.byteRangeLength).isEqualTo(51740); assertThat(segment.byteRangeOffset).isEqualTo(2147586650L); assertThat(segment.url).isEqualTo("https://priv.example.com/fileSequence2682.ts"); @@ -148,7 +148,7 @@ public class HlsMediaPlaylistParserTest { .isEqualTo("https://priv.example.com/key.php?r=2682"); // 0xA7B == 2683. assertThat(segment.encryptionIV).isNotNull(); - assertThat(Util.toUpperInvariant(segment.encryptionIV)).isEqualTo("A7B"); + assertThat(segment.encryptionIV).ignoringCase().isEqualTo("A7B"); assertThat(segment.byteRangeLength).isEqualTo(C.LENGTH_UNSET); assertThat(segment.byteRangeOffset).isEqualTo(0); assertThat(segment.url).isEqualTo("https://priv.example.com/fileSequence2683.ts");