Fix ClearKey response conversion pre O-MR1

Issue: #4075

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=191872512

RELEASENOTES.md

@@ -53,6 +53,8 @@
   `BaseRenderer.onStreamChanged`.
   `BaseRenderer.onStreamChanged`.
 * HLS: Fix playlist loading error propagation when the current selection does
 * HLS: Fix playlist loading error propagation when the current selection does
   not include all of the playlist's variants.
   not include all of the playlist's variants.
+* Fix ClearKey decryption error if the key contains a forward slash
+  ([#4075](https://github.com/google/ExoPlayer/issues/4075)).
 * Fix IllegalStateException when switching surface on Huawei P9 Lite
 * Fix IllegalStateException when switching surface on Huawei P9 Lite
   ([#4084](https://github.com/google/ExoPlayer/issues/4084)).
   ([#4084](https://github.com/google/ExoPlayer/issues/4084)).
 
 

library/core/src/main/java/com/google/android/exoplayer2/drm/ClearKeyUtil.java

@@ -17,8 +17,6 @@ package com.google.android.exoplayer2.drm;
 
 
 import android.util.Log;
 import android.util.Log;
 import com.google.android.exoplayer2.util.Util;
 import com.google.android.exoplayer2.util.Util;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 import org.json.JSONArray;
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.json.JSONObject;
@@ -29,7 +27,6 @@ import org.json.JSONObject;
 /* package */ final class ClearKeyUtil {
 /* package */ final class ClearKeyUtil {
 
 
   private static final String TAG = "ClearKeyUtil";
   private static final String TAG = "ClearKeyUtil";
-  private static final Pattern REQUEST_KIDS_PATTERN = Pattern.compile("\"kids\":\\[\"(.*?)\"]");
 
 
   private ClearKeyUtil() {}
   private ClearKeyUtil() {}
 
 
@@ -43,21 +40,12 @@ import org.json.JSONObject;
     if (Util.SDK_INT >= 27) {
     if (Util.SDK_INT >= 27) {
       return request;
       return request;
     }
     }
-    // Prior to O-MR1 the ClearKey CDM encoded the values in the "kids" array using Base64 rather
-    // than Base64Url. See [Internal: b/64388098]. Any "/" characters that ended up in the request
-    // as a result were not escaped as "\/". We know the exact request format from the platform's
-    // InitDataParser.cpp, so we can use a regexp rather than parsing the JSON.
+    // Prior to O-MR1 the ClearKey CDM encoded the values in the "kids" array using Base64 encoding
+    // rather than Base64Url encoding. See [Internal: b/64388098]. We know the exact request format
+    // from the platform's InitDataParser.cpp. Since there aren't any "+" or "/" symbols elsewhere
+    // in the request, it's safe to fix the encoding by replacement through the whole request.
     String requestString = Util.fromUtf8Bytes(request);
     String requestString = Util.fromUtf8Bytes(request);
-    Matcher requestKidsMatcher = REQUEST_KIDS_PATTERN.matcher(requestString);
-    if (!requestKidsMatcher.find()) {
-      Log.e(TAG, "Failed to adjust request data: " + requestString);
-      return request;
-    }
-    int kidsStartIndex = requestKidsMatcher.start(1);
-    int kidsEndIndex = requestKidsMatcher.end(1);
-    StringBuilder adjustedRequestBuilder = new StringBuilder(requestString);
-    base64ToBase64Url(adjustedRequestBuilder, kidsStartIndex, kidsEndIndex);
-    return Util.getUtf8Bytes(adjustedRequestBuilder.toString());
+    return Util.getUtf8Bytes(base64ToBase64Url(requestString));
   }
   }
 
 
   /**
   /**
@@ -71,39 +59,39 @@ import org.json.JSONObject;
       return response;
       return response;
     }
     }
     // Prior to O-MR1 the ClearKey CDM expected Base64 encoding rather than Base64Url encoding for
     // Prior to O-MR1 the ClearKey CDM expected Base64 encoding rather than Base64Url encoding for
-    // the "k" and "kid" strings. See [Internal: b/64388098].
+    // the "k" and "kid" strings. See [Internal: b/64388098]. We know that the ClearKey CDM only
+    // looks at the k, kid and kty parameters in each key, so can ignore the rest of the response.
     try {
     try {
       JSONObject responseJson = new JSONObject(Util.fromUtf8Bytes(response));
       JSONObject responseJson = new JSONObject(Util.fromUtf8Bytes(response));
+      StringBuilder adjustedResponseBuilder = new StringBuilder("{\"keys\":[");
       JSONArray keysArray = responseJson.getJSONArray("keys");
       JSONArray keysArray = responseJson.getJSONArray("keys");
       for (int i = 0; i < keysArray.length(); i++) {
       for (int i = 0; i < keysArray.length(); i++) {
+        if (i != 0) {
+          adjustedResponseBuilder.append(",");
+        }
         JSONObject key = keysArray.getJSONObject(i);
         JSONObject key = keysArray.getJSONObject(i);
-        key.put("k", base64UrlToBase64(key.getString("k")));
-        key.put("kid", base64UrlToBase64(key.getString("kid")));
+        adjustedResponseBuilder.append("{\"k\":\"");
+        adjustedResponseBuilder.append(base64UrlToBase64(key.getString("k")));
+        adjustedResponseBuilder.append("\",\"kid\":\"");
+        adjustedResponseBuilder.append(base64UrlToBase64(key.getString("kid")));
+        adjustedResponseBuilder.append("\",\"kty\":\"");
+        adjustedResponseBuilder.append(key.getString("kty"));
+        adjustedResponseBuilder.append("\"}");
       }
       }
-      return Util.getUtf8Bytes(responseJson.toString());
+      adjustedResponseBuilder.append("]}");
+      return Util.getUtf8Bytes(adjustedResponseBuilder.toString());
     } catch (JSONException e) {
     } catch (JSONException e) {
       Log.e(TAG, "Failed to adjust response data: " + Util.fromUtf8Bytes(response), e);
       Log.e(TAG, "Failed to adjust response data: " + Util.fromUtf8Bytes(response), e);
       return response;
       return response;
     }
     }
   }
   }
 
 
-  private static void base64ToBase64Url(StringBuilder base64, int startIndex, int endIndex) {
-    for (int i = startIndex; i < endIndex; i++) {
-      switch (base64.charAt(i)) {
-        case '+':
-          base64.setCharAt(i, '-');
-          break;
-        case '/':
-          base64.setCharAt(i, '_');
-          break;
-        default:
-          break;
-      }
-    }
+  private static String base64ToBase64Url(String base64) {
+    return base64.replace('+', '-').replace('/', '_');
   }
   }
 
 
-  private static String base64UrlToBase64(String base64) {
-    return base64.replace('-', '+').replace('_', '/');
+  private static String base64UrlToBase64(String base64Url) {
+    return base64Url.replace('-', '+').replace('_', '/');
   }
   }
 
 
 }
 }

library/core/src/test/java/com/google/android/exoplayer2/drm/ClearKeyUtilTest.java

@@ -17,8 +17,7 @@ package com.google.android.exoplayer2.drm;
 
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertThat;
 
 
-import com.google.android.exoplayer2.C;
-import java.nio.charset.Charset;
+import com.google.android.exoplayer2.util.Util;
 import org.junit.Test;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runner.RunWith;
 import org.robolectric.RobolectricTestRunner;
 import org.robolectric.RobolectricTestRunner;
@@ -27,37 +26,115 @@ import org.robolectric.annotation.Config;
 /**
 /**
  * Unit test for {@link ClearKeyUtil}.
  * Unit test for {@link ClearKeyUtil}.
  */
  */
-// TODO: When API level 27 is supported, add tests that check the adjust methods are no-ops.
 @RunWith(RobolectricTestRunner.class)
 @RunWith(RobolectricTestRunner.class)
 public final class ClearKeyUtilTest {
 public final class ClearKeyUtilTest {
 
 
+  private static final byte[] SINGLE_KEY_RESPONSE =
+      Util.getUtf8Bytes(
+          "{"
+              + "\"keys\":["
+              + "{"
+              + "\"k\":\"abc_def-\","
+              + "\"kid\":\"ab_cde-f\","
+              + "\"kty\":\"o_c-t\","
+              + "\"ignored\":\"ignored\""
+              + "}"
+              + "],"
+              + "\"ignored\":\"ignored\""
+              + "}");
+  private static final byte[] MULTI_KEY_RESPONSE =
+      Util.getUtf8Bytes(
+          "{"
+              + "\"keys\":["
+              + "{"
+              + "\"k\":\"abc_def-\","
+              + "\"kid\":\"ab_cde-f\","
+              + "\"kty\":\"oct\","
+              + "\"ignored\":\"ignored\""
+              + "},{"
+              + "\"k\":\"ghi_jkl-\","
+              + "\"kid\":\"gh_ijk-l\","
+              + "\"kty\":\"oct\""
+              + "}"
+              + "],"
+              + "\"ignored\":\"ignored\""
+              + "}");
+  private static final byte[] KEY_REQUEST =
+      Util.getUtf8Bytes(
+          "{"
+              + "\"kids\":["
+              + "\"abc+def/\","
+              + "\"ab+cde/f\""
+              + "],"
+              + "\"type\":\"temporary\""
+              + "}");
+
   @Config(sdk = 26)
   @Config(sdk = 26)
   @Test
   @Test
-  public void testAdjustResponseDataV26() {
-    byte[] data = ("{\"keys\":[{"
-        + "\"k\":\"abc_def-\","
-        + "\"kid\":\"ab_cde-f\"}],"
-        + "\"type\":\"abc_def-"
-        + "\"}").getBytes(Charset.forName(C.UTF8_NAME));
-    // We expect "-" and "_" to be replaced with "+" and "\/" (forward slashes need to be escaped in
-    // JSON respectively, for "k" and "kid" only.
-    byte[] expected = ("{\"keys\":[{"
-        + "\"k\":\"abc\\/def+\","
-        + "\"kid\":\"ab\\/cde+f\"}],"
-        + "\"type\":\"abc_def-"
-        + "\"}").getBytes(Charset.forName(C.UTF8_NAME));
-    assertThat(ClearKeyUtil.adjustResponseData(data)).isEqualTo(expected);
+  public void testAdjustSingleKeyResponseDataV26() {
+    // Everything but the keys should be removed. Within each key only the k, kid and kty parameters
+    // should remain. Any "-" and "_" characters in the k and kid values should be replaced with "+"
+    // and "/".
+    byte[] expected =
+        Util.getUtf8Bytes(
+            "{"
+                + "\"keys\":["
+                + "{"
+                + "\"k\":\"abc/def+\",\"kid\":\"ab/cde+f\",\"kty\":\"o_c-t\""
+                + "}"
+                + "]"
+                + "}");
+    assertThat(ClearKeyUtil.adjustResponseData(SINGLE_KEY_RESPONSE)).isEqualTo(expected);
+  }
+
+  @Config(sdk = 26)
+  @Test
+  public void testAdjustMultiKeyResponseDataV26() {
+    // Everything but the keys should be removed. Within each key only the k, kid and kty parameters
+    // should remain. Any "-" and "_" characters in the k and kid values should be replaced with "+"
+    // and "/".
+    byte[] expected =
+        Util.getUtf8Bytes(
+            "{"
+                + "\"keys\":["
+                + "{"
+                + "\"k\":\"abc/def+\",\"kid\":\"ab/cde+f\",\"kty\":\"oct\""
+                + "},{"
+                + "\"k\":\"ghi/jkl+\",\"kid\":\"gh/ijk+l\",\"kty\":\"oct\""
+                + "}"
+                + "]"
+                + "}");
+    assertThat(ClearKeyUtil.adjustResponseData(MULTI_KEY_RESPONSE)).isEqualTo(expected);
+  }
+
+  @Config(sdk = 27)
+  @Test
+  public void testAdjustResponseDataV27() {
+    // Response should be unchanged.
+    assertThat(ClearKeyUtil.adjustResponseData(SINGLE_KEY_RESPONSE)).isEqualTo(SINGLE_KEY_RESPONSE);
   }
   }
 
 
   @Config(sdk = 26)
   @Config(sdk = 26)
   @Test
   @Test
   public void testAdjustRequestDataV26() {
   public void testAdjustRequestDataV26() {
-    byte[] data = "{\"kids\":[\"abc+def/\",\"ab+cde/f\"],\"type\":\"abc+def/\"}"
-        .getBytes(Charset.forName(C.UTF8_NAME));
     // We expect "+" and "/" to be replaced with "-" and "_" respectively, for "kids".
     // We expect "+" and "/" to be replaced with "-" and "_" respectively, for "kids".
-    byte[] expected = "{\"kids\":[\"abc-def_\",\"ab-cde_f\"],\"type\":\"abc+def/\"}"
-        .getBytes(Charset.forName(C.UTF8_NAME));
-    assertThat(ClearKeyUtil.adjustRequestData(data)).isEqualTo(expected);
+    byte[] expected =
+        Util.getUtf8Bytes(
+            "{"
+                + "\"kids\":["
+                + "\"abc-def_\","
+                + "\"ab-cde_f\""
+                + "],"
+                + "\"type\":\"temporary\""
+                + "}");
+    assertThat(ClearKeyUtil.adjustRequestData(KEY_REQUEST)).isEqualTo(expected);
+  }
+
+  @Config(sdk = 27)
+  @Test
+  public void testAdjustRequestDataV27() {
+    // Request should be unchanged.
+    assertThat(ClearKeyUtil.adjustRequestData(KEY_REQUEST)).isEqualTo(KEY_REQUEST);
   }
   }
 
 
 }
 }