Commment signed webapks working and verified.

chrome/android/webapk/libs/client/junit/src/org/chromium/webapk/lib/client/WebApkValidatorTest.java

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.webapk.lib.client;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import static org.chromium.webapk.lib.common.WebApkMetaDataKeys.START_URL;
 
 import android.content.Intent;
 import android.content.pm.ActivityInfo;
+import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager.NameNotFoundException;
 import android.content.pm.ResolveInfo;
 import android.content.pm.Signature;
+import android.os.Bundle;
 
-import org.chromium.testing.local.LocalRobolectricTestRunner;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.annotation.Config;
 import org.robolectric.res.builder.RobolectricPackageManager;
 
+import org.chromium.testing.local.LocalRobolectricTestRunner;
+import org.chromium.testing.local.TestDir;
+
 import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.List;
 
-/**
- * Unit tests for {@link org.chromium.webapk.lib.client.WebApkValidator}.
- */
+/** Unit tests for {@link org.chromium.webapk.lib.client.WebApkValidator}. */
 @RunWith(LocalRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
 public class WebApkValidatorTest {
     private static final String WEBAPK_PACKAGE_NAME = "org.chromium.webapk.foo";
     private static final String INVALID_WEBAPK_PACKAGE_NAME = "invalid.org.chromium.webapk.foo";
     private static final String URL_OF_WEBAPK = "https://www.foo.com";
     private static final String URL_WITHOUT_WEBAPK = "https://www.other.com";
+    private static final String TEST_DATA_DIR = "webapks/";
 
-    private static final byte[] EXPECTED_SIGNATURE = new byte[] {
-        48, -126, 3, -121, 48, -126, 2, 111, -96, 3, 2, 1, 2, 2, 4, 20, -104, -66, -126, 48, 13,
-        6, 9, 42, -122, 72, -122, -9, 13, 1, 1, 11, 5, 0, 48, 116, 49, 11, 48, 9, 6, 3, 85, 4,
-        6, 19, 2, 67, 65, 49, 16, 48, 14, 6, 3, 85, 4, 8, 19, 7, 79, 110, 116, 97, 114, 105,
-        111, 49, 17, 48, 15, 6, 3, 85, 4, 7, 19, 8, 87, 97, 116, 101, 114, 108, 111, 111, 49,
-        17, 48, 15, 6, 3, 85, 4, 10, 19, 8, 67, 104, 114, 111, 109, 105, 117, 109, 49, 17, 48};
+    private static final byte[] EXPECTED_SIGNATURE = new byte[] {48, -126, 3, -121, 48, -126, 2,
+            111, -96, 3, 2, 1, 2, 2, 4, 20, -104, -66, -126, 48, 13, 6, 9, 42, -122, 72, -122, -9,
+            13, 1, 1, 11, 5, 0, 48, 116, 49, 11, 48, 9, 6, 3, 85, 4, 6, 19, 2, 67, 65, 49, 16, 48,
+            14, 6, 3, 85, 4, 8, 19, 7, 79, 110, 116, 97, 114, 105, 111, 49, 17, 48, 15, 6, 3, 85, 4,
+            7, 19, 8, 87, 97, 116, 101, 114, 108, 111, 111, 49, 17, 48, 15, 6, 3, 85, 4, 10, 19, 8,
+            67, 104, 114, 111, 109, 105, 117, 109, 49, 17, 48};
 
-    private static final byte[] SIGNATURE_1 = new byte[] {
-        13, 52, 51, 48, 51, 48, 51, 49, 53, 49, 54, 52, 52, 90, 48, 116, 49, 11, 48, 9, 6, 3,
-        85, 4, 6, 19, 2, 67, 65, 49, 16, 48, 14, 6, 3, 85, 4, 8, 19, 7, 79, 110, 116, 97, 114};
+    private static final byte[] SIGNATURE_1 = new byte[] {13, 52, 51, 48, 51, 48, 51, 49, 53, 49,
+            54, 52, 52, 90, 48, 116, 49, 11, 48, 9, 6, 3, 85, 4, 6, 19, 2, 67, 65, 49, 16, 48, 14,
+            6, 3, 85, 4, 8, 19, 7, 79, 110, 116, 97, 114};
 
-    private static final byte[] SIGNATURE_2 = new byte[] {
-        49, 17, 48, 15, 6, 3, 85, 4, 10, 19, 8, 67, 104, 114, 111, 109, 105, 117, 109, 49, 17,
-        48, 15, 6, 3, 85, 4, 11, 19, 8, 67, 104, 114, 111, 109, 105, 117, 109, 49, 26, 48, 24};
+    private static final byte[] SIGNATURE_2 = new byte[] {49, 17, 48, 15, 6, 3, 85, 4, 10, 19, 8,
+            67, 104, 114, 111, 109, 105, 117, 109, 49, 17, 48, 15, 6, 3, 85, 4, 11, 19, 8, 67, 104,
+            114, 111, 109, 105, 117, 109, 49, 26, 48, 24};
+
+    // This is the public key used for the test files (chrome/test/data/webapks/public.der)
+    private static final byte[] PUBLIC_KEY = new byte[] {48, 89, 48, 19, 6, 7, 42, -122, 72, -50,
+            61, 2, 1, 6, 8, 42, -122, 72, -50, 61, 3, 1, 7, 3, 66, 0, 4, 126, 126, 112, -21, 2, 44,
+            118, -29, 29, 86, -65, 118, -107, -82, 20, 118, 53, -103, 73, -58, -17, 24, 95, -11, 28,
+            60, -31, -7, -19, -125, -92, 120, -5, -128, 19, 19, -127, 13, -44, 25, 28, -14, 32, 55,
+            12, 68, -17, -43, 29, -125, 95, -121, 11, 81, -128, 25, 7, -87, -106, -119, -126, -42,
+            102, -84};
 
     private RobolectricPackageManager mPackageManager;
 
     @Before
     public void setUp() {
-        mPackageManager = (RobolectricPackageManager) RuntimeEnvironment
-                .application.getPackageManager();
-        WebApkValidator.initWithBrowserHostSignature(EXPECTED_SIGNATURE);
+        mPackageManager =
+                (RobolectricPackageManager) RuntimeEnvironment.application.getPackageManager();
+        WebApkValidator.initWithBrowserHostSignature(EXPECTED_SIGNATURE, PUBLIC_KEY);
     }
 
     /**
      * Tests {@link WebApkValidator.queryWebApkPackage()} returns a WebAPK's package name if the
      * WebAPK can handle the given URL and the WebAPK is valid.
      */
     @Test
     public void testQueryWebApkPackageReturnsPackageIfTheURLCanBeHandled() {
         try {
             Intent intent = Intent.parseUri(URL_OF_WEBAPK, Intent.URI_INTENT_SCHEME);
             intent.addCategory(Intent.CATEGORY_BROWSABLE);
 
             mPackageManager.addResolveInfoForIntent(intent, newResolveInfo(WEBAPK_PACKAGE_NAME));
             mPackageManager.addPackage(newPackageInfoWithBrowserSignature(
                     WEBAPK_PACKAGE_NAME, new Signature(EXPECTED_SIGNATURE)));
 
-            assertEquals(WEBAPK_PACKAGE_NAME, WebApkValidator.queryWebApkPackage(
-                    RuntimeEnvironment.application, URL_OF_WEBAPK));
+            assertEquals(WEBAPK_PACKAGE_NAME,
+                    WebApkValidator.queryWebApkPackage(
+                            RuntimeEnvironment.application, URL_OF_WEBAPK));
         } catch (URISyntaxException e) {
             Assert.fail("URI is invalid.");
         }
     }
 
-    /**

[pkotwicz, 2017/03/31 03:59:24]

Why this change?

[ScottK, 2017/04/03 17:44:17]

Google's vim auto formatter, apparently - reverted.

-     * Tests {@link WebApkValidator.queryWebApkPackage()} returns null for a non-browsable Intent.
+    /** Tests {@link WebApkValidator.queryWebApkPackage()} returns null for a non-browsable Intent.
      */
     @Test
     public void testQueryWebApkPackageReturnsNullForNonBrowsableIntent() {
         try {
             Intent intent = Intent.parseUri(URL_OF_WEBAPK, Intent.URI_INTENT_SCHEME);
 
             mPackageManager.addResolveInfoForIntent(intent, newResolveInfo(WEBAPK_PACKAGE_NAME));
             mPackageManager.addPackage(newPackageInfoWithBrowserSignature(
                     WEBAPK_PACKAGE_NAME, new Signature(EXPECTED_SIGNATURE)));
 
             assertNull(WebApkValidator.queryWebApkPackage(
                     RuntimeEnvironment.application, URL_OF_WEBAPK));
         } catch (URISyntaxException e) {
             Assert.fail("URI is invalid.");
         }
     }
 
     /**
-     * Tests {@link WebApkValidator.queryWebApkPackage()} returns null if no WebAPK handles
-     * the given URL.
+     * Tests {@link WebApkValidator.queryWebApkPackage()} returns null if no WebAPK handles the
+     * given URL.
      */
     @Test
     public void testQueryWebApkPackageReturnsNullWhenNoWebApkHandlesTheURL() {
         try {
             Intent intent = Intent.parseUri(URL_OF_WEBAPK, Intent.URI_INTENT_SCHEME);
             intent.addCategory(Intent.CATEGORY_BROWSABLE);
 
             mPackageManager.addResolveInfoForIntent(intent, newResolveInfo(WEBAPK_PACKAGE_NAME));
             mPackageManager.addPackage(newPackageInfoWithBrowserSignature(
                     WEBAPK_PACKAGE_NAME, new Signature(EXPECTED_SIGNATURE)));
@@ skipping 15 matching lines @@
         infos.add(newResolveInfo(WEBAPK_PACKAGE_NAME));
         mPackageManager.addPackage(newPackageInfoWithBrowserSignature(
                 WEBAPK_PACKAGE_NAME, new Signature(EXPECTED_SIGNATURE)));
 
         assertEquals(WEBAPK_PACKAGE_NAME,
                 WebApkValidator.findWebApkPackage(RuntimeEnvironment.application, infos));
     }
 
     /**
      * Tests {@link WebApkValidator.findWebApkPackage} returns null if none of the packages for the
      * ResolveInfos start with {@link WebApkConstants.WEBAPK_PACKAGE_PREFIX}.

[pkotwicz, 2017/03/31 03:59:24]

You probably need to update this comment

[ScottK, 2017/04/03 17:44:18]

Done.

      */
     @Test
     public void testFindWebApkPackageReturnsNullForInvalidPackageName() {
         List<ResolveInfo> infos = new ArrayList<ResolveInfo>();
         infos.add(newResolveInfo(INVALID_WEBAPK_PACKAGE_NAME));
         mPackageManager.addPackage(newPackageInfoWithBrowserSignature(
                 INVALID_WEBAPK_PACKAGE_NAME, new Signature(EXPECTED_SIGNATURE)));
 
         assertNull(WebApkValidator.findWebApkPackage(RuntimeEnvironment.application, infos));
     }
@@ skipping 23 matching lines @@
             throws NameNotFoundException {
         List<ResolveInfo> infos = new ArrayList<ResolveInfo>();
         infos.add(newResolveInfo(WEBAPK_PACKAGE_NAME));
         Signature signatures[] =
                 new Signature[] {new Signature(SIGNATURE_1), new Signature(SIGNATURE_2)};
         mPackageManager.addPackage(newPackageInfo(WEBAPK_PACKAGE_NAME, signatures));
 
         assertNull(WebApkValidator.findWebApkPackage(RuntimeEnvironment.application, infos));
     }
 
+    /** Tests {@link WebApkValidator.isValidWebApk} for comment signed webapks. */

[pkotwicz, 2017/03/31 03:59:24]

Can you make the comment more descriptive. The comment for
testIsValidWebApkCommentSigned() and testIsValidWebApkCommentSignedFailures()
should not be identical

[ScottK, 2017/04/03 17:44:17]

Done.

+    @Test
+    public void testIsValidWebApkCommentSigned() {
+        String[] filenames = {"example.apk", "java-example.apk"};
+        String packageName = "com.webapk.a9c419502bb98fcb7";
+        Signature[] signature = new Signature[] {new Signature(SIGNATURE_1)};
+
+        for (String filename : filenames) {

[pkotwicz, 2017/03/31 03:59:24]

Maybe you should call mPackageManager.removePackage() before adding a package
with the exact same name.

[ScottK, 2017/04/03 17:44:17]

Done.

+            mPackageManager.addPackage(
+                    newPackageInfo(packageName, signature, testFilename(filename)));
+            assertTrue(filename + " did not verify",
+                    WebApkValidator.isValidWebApk(RuntimeEnvironment.application, packageName));
+        }
+    }
+
+    /** Tests {@link WebApkValidator.isValidWebApk} for comment signed webapks. */
+    @Test
+    public void testIsValidWebApkCommentSignedFailures() {
+        String[] filenames = {
+                "bad-sig.apk", "bad-utf8-fname.apk", "empty.apk", "extra-len-too-large.apk",
+                "fcomment-too-large.apk", "no-cd.apk", "no-comment.apk", "no-eocd.apk",
+                "no-lfh.apk", "not-an.apk", "too-many-metainf.apk", "truncated.apk", "zeros.apk",
+                "zeros-at-end.apk",
+        };
+        String packageName = "com.webapk.a9c419502bb98fcb7";
+        Signature[] signature = new Signature[] {new Signature(SIGNATURE_1)};
+
+        for (String filename : filenames) {
+            mPackageManager.addPackage(
+                    newPackageInfo(packageName, signature, testFilename(filename)));
+            assertFalse(filename,
+                    WebApkValidator.isValidWebApk(RuntimeEnvironment.application, packageName));
+        }
+    }
+
+    // Get the full test filename.
+    private static String testFilename(String fname) {

[pkotwicz, 2017/03/31 03:59:24]

Nit: testFilename() -> testFilePath()

[ScottK, 2017/04/03 17:44:17]

Done.

+        return TestDir.getTestFilePath(TEST_DATA_DIR + fname);
+    }
+
     private static ResolveInfo newResolveInfo(String packageName) {
         ActivityInfo activityInfo = new ActivityInfo();
         activityInfo.packageName = packageName;
         ResolveInfo resolveInfo = new ResolveInfo();
         resolveInfo.activityInfo = activityInfo;
         return resolveInfo;
     }
 
     private static PackageInfo newPackageInfo(String packageName, Signature[] signatures) {

[pkotwicz, 2017/03/31 03:59:24]

Nit: I would pass the sourceDir to this function (and pass null in the
appropriate callers)

[ScottK, 2017/04/03 17:44:17]

Done.

         PackageInfo packageInfo = new PackageInfo();
         packageInfo.packageName = packageName;
         packageInfo.signatures = signatures;
+        packageInfo.applicationInfo = new ApplicationInfo();
+        packageInfo.applicationInfo.metaData = new Bundle();
+        packageInfo.applicationInfo.metaData.putString(START_URL, "https://non-empty.com/starturl");
         return packageInfo;
     }
 
+    private static PackageInfo newPackageInfo(
+            String packageName, Signature[] signatures, String sourceDir) {
+        PackageInfo packageInfo = newPackageInfo(packageName, signatures);
+        packageInfo.applicationInfo.sourceDir = sourceDir;
+        return packageInfo;
+    }
+
     // The browser signature is expected to always be the second signature - the first (and any
     // additional ones after the second) are ignored.
     private static PackageInfo newPackageInfoWithBrowserSignature(
             String packageName, Signature signature) {
         return newPackageInfo(packageName, new Signature[] {new Signature(""), signature});
     }
 }