Stop double-encoding CMCD query parameters

`Uri.appendQueryParameter` is documented to encode its arguments, so calling `Uri.encode` beforehand results in double-encoding. Issue: #1075 #minor-release PiperOrigin-RevId: 604995441
androidx · Feb 7, 2024 · 7ebfed5 · 7ebfed5
1 parent 5f9c96a
commit 7ebfed5
Show file tree

Hide file tree

Showing 6 changed files with 12 additions and 15 deletions.
diff --git 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
@@ -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
@@ -108,7 +108,7 @@ public void createInstance_populatesCmcdHttpQueryParameters() {
  getCustomData() {
  return new ImmutableListMultimap.Builder<String, String>()
  .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/...oplayer_dash/src/test/java/androidx/media3/exoplayer/dash/DefaultDashChunkSourceTest.java b/...oplayer_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
@@ -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/...ing/src/test/java/androidx/media3/exoplayer/smoothstreaming/DefaultSsChunkSourceTest.java b/...ing/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,"