From 7ebfed505c5ac42cb6e614a3dd9b7755af3e92b2 Mon Sep 17 00:00:00 2001 From: ibaker Date: Wed, 7 Feb 2024 08:41:45 -0800 Subject: [PATCH] Stop double-encoding CMCD query parameters `Uri.appendQueryParameter` is documented to encode its arguments, so calling `Uri.encode` beforehand results in double-encoding. Issue: androidx/media#1075 #minor-release PiperOrigin-RevId: 604995441 --- RELEASENOTES.md | 2 ++ .../androidx/media3/exoplayer/upstream/CmcdData.java | 3 +-- .../media3/exoplayer/upstream/CmcdDataTest.java | 10 ++++++---- .../exoplayer/dash/DefaultDashChunkSourceTest.java | 4 +--- .../media3/exoplayer/hls/HlsChunkSourceTest.java | 4 +--- .../smoothstreaming/DefaultSsChunkSourceTest.java | 4 +--- 6 files changed, 12 insertions(+), 15 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 7950c9e542b..9183d5126be 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -10,6 +10,8 @@ * Fix the regex used for validating custom Common Media Client Data (CMCD) key names by modifying it to only check for hyphen ([#1028](https://github.com/androidx/media/issues/1028)). + * Stop double-encoding CMCD query parameters + ([#1075](https://github.com/androidx/media/issues/1075)). * Transformer: * Add `audioConversionProcess` and `videoConversionProcess` to `ExportResult` indicating how the respective track in the output file diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/upstream/CmcdData.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/upstream/CmcdData.java index 1ebd6727f95..dc1a5b58fbd 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/upstream/CmcdData.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/upstream/CmcdData.java @@ -417,8 +417,7 @@ public DataSpec addToDataSpec(DataSpec dataSpec) { .uri .buildUpon() .appendQueryParameter( - CmcdConfiguration.CMCD_QUERY_PARAMETER_KEY, - Uri.encode(COMMA_JOINER.join(keyValuePairs))); + CmcdConfiguration.CMCD_QUERY_PARAMETER_KEY, COMMA_JOINER.join(keyValuePairs)); return dataSpec.buildUpon().setUri(uriBuilder.build()).build(); } } diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/upstream/CmcdDataTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/upstream/CmcdDataTest.java index efd07f8ca3b..bafe3e352b9 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/upstream/CmcdDataTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/upstream/CmcdDataTest.java @@ -108,7 +108,7 @@ public void createInstance_populatesCmcdHttpQueryParameters() { getCustomData() { return new ImmutableListMultimap.Builder() .put("CMCD-Object", "key-1=1") - .put("CMCD-Request", "key-2=\"stringValue1,stringValue2\"") + .put("CMCD-Request", "key-2=\"stringVälue1,stringVälue2\"") .build(); } @@ -143,11 +143,13 @@ public int getRequestedMaximumThroughputKbps(int throughputKbps) { dataSpec = cmcdData.addToDataSpec(dataSpec); - assertThat( - Uri.decode(dataSpec.uri.getQueryParameter(CmcdConfiguration.CMCD_QUERY_PARAMETER_KEY))) + // Confirm that the values above are URL-encoded + assertThat(dataSpec.uri.toString()).doesNotContain("ä"); + assertThat(dataSpec.uri.toString()).contains(Uri.encode("ä")); + assertThat(dataSpec.uri.getQueryParameter(CmcdConfiguration.CMCD_QUERY_PARAMETER_KEY)) .isEqualTo( "bl=1800,br=840,bs,cid=\"mediaId\",d=3000,dl=900,key-1=1," - + "key-2=\"stringValue1,stringValue2\",mtp=500,pr=2.00,rtp=1700,sf=d," + + "key-2=\"stringVälue1,stringVälue2\",mtp=500,pr=2.00,rtp=1700,sf=d," + "sid=\"sessionId\",st=l,su,tb=1000"); } diff --git a/libraries/exoplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultDashChunkSourceTest.java b/libraries/exoplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultDashChunkSourceTest.java index 37d89a3e78d..fa5eaf8738a 100644 --- a/libraries/exoplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultDashChunkSourceTest.java +++ b/libraries/exoplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultDashChunkSourceTest.java @@ -532,9 +532,7 @@ public int getRequestedMaximumThroughputKbps(int throughputKbps) { output); assertThat( - Uri.decode( - output.chunk.dataSpec.uri.getQueryParameter( - CmcdConfiguration.CMCD_QUERY_PARAMETER_KEY))) + output.chunk.dataSpec.uri.getQueryParameter(CmcdConfiguration.CMCD_QUERY_PARAMETER_KEY)) .isEqualTo( "bl=0,br=700,cid=\"mediaId\",com.example.test-key-1=1,d=4000,dl=0," + "key-2=\"stringValue\",mtp=1000,nor=\"..%2Fvideo_4000_700000.m4s\",nrr=\"0-\"," diff --git a/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/HlsChunkSourceTest.java b/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/HlsChunkSourceTest.java index 303e4c1bb6e..524aebb83ba 100644 --- a/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/HlsChunkSourceTest.java +++ b/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/HlsChunkSourceTest.java @@ -442,9 +442,7 @@ public int getRequestedMaximumThroughputKbps(int throughputKbps) { output); assertThat( - Uri.decode( - output.chunk.dataSpec.uri.getQueryParameter( - CmcdConfiguration.CMCD_QUERY_PARAMETER_KEY))) + output.chunk.dataSpec.uri.getQueryParameter(CmcdConfiguration.CMCD_QUERY_PARAMETER_KEY)) .isEqualTo( "bl=0,br=800,cid=\"mediaId\",com.example.test-key-1=1,d=4000,dl=0," + "key-2=\"stringValue\",nor=\"..%2F3.mp4\",nrr=\"0-\",ot=v,sf=h," diff --git a/libraries/exoplayer_smoothstreaming/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSourceTest.java b/libraries/exoplayer_smoothstreaming/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSourceTest.java index 89a7307ba03..bfffe289c95 100644 --- a/libraries/exoplayer_smoothstreaming/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSourceTest.java +++ b/libraries/exoplayer_smoothstreaming/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSourceTest.java @@ -283,9 +283,7 @@ public int getRequestedMaximumThroughputKbps(int throughputKbps) { output); assertThat( - Uri.decode( - output.chunk.dataSpec.uri.getQueryParameter( - CmcdConfiguration.CMCD_QUERY_PARAMETER_KEY))) + output.chunk.dataSpec.uri.getQueryParameter(CmcdConfiguration.CMCD_QUERY_PARAMETER_KEY)) .isEqualTo( "bl=0,br=308,cid=\"mediaId\",com.example.test-key-1=1,d=1968,dl=0," + "key-2=\"stringValue\",mtp=1000,nor=\"..%2FFragments(video%3D19680000)\",ot=v,"