Back out 9fe5610363b3 (bug 959652) for bustage
authorPhil Ringnalda <philringnalda@gmail.com>
Tue, 11 Mar 2014 21:53:43 -0700
changeset 173121 31f9a148b33cb6f6eb509df29a339b5de4f65254
parent 173120 bd0463063293e9e2fdfcea2dce80bc849316af0f
child 173122 34941dd46be6611af8ba6c741afc7294afe65cd9
push id5609
push userphilringnalda@gmail.com
push dateWed, 12 Mar 2014 04:53:54 +0000
treeherderfx-team@31f9a148b33c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs959652
milestone30.0a1
backs out9fe5610363b3f8e35f248bb2ce726c8e2665a8a1
Back out 9fe5610363b3 (bug 959652) for bustage
mobile/android/base/background/healthreport/EnvironmentV1.java
mobile/android/tests/background/junit3/android-services-files.mk
mobile/android/tests/background/junit3/src/healthreport/TestEnvironmentV1HashAppender.java
--- a/mobile/android/base/background/healthreport/EnvironmentV1.java
+++ b/mobile/android/base/background/healthreport/EnvironmentV1.java
@@ -1,24 +1,24 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.background.healthreport;
 
 import java.io.UnsupportedEncodingException;
+import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.Iterator;
 import java.util.SortedSet;
 
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.mozilla.apache.commons.codec.binary.Base64;
 import org.mozilla.gecko.background.common.log.Logger;
-import org.mozilla.gecko.background.nativecode.NativeCrypto;
 
 public abstract class EnvironmentV1 {
   private static final String LOG_TAG = "GeckoEnvironment";
   private static final int VERSION = 1;
 
   protected final Class<? extends EnvironmentAppender> appenderClass;
 
   protected volatile String hash = null;
@@ -65,51 +65,46 @@ public abstract class EnvironmentV1 {
    * own appender that just records strings, for example.
    */
   public static abstract class EnvironmentAppender {
     public abstract void append(String s);
     public abstract void append(int v);
   }
 
   public static class HashAppender extends EnvironmentAppender {
-    private final StringBuilder builder;
+    final MessageDigest hasher;
 
     public HashAppender() throws NoSuchAlgorithmException {
-      builder = new StringBuilder();
+      // Note to the security-minded reader: we deliberately use SHA-1 here, not
+      // a stronger hash. These identifiers don't strictly need a cryptographic
+      // hash function, because there is negligible value in attacking the hash.
+      // We use SHA-1 because it's *shorter* -- the exact same reason that Git
+      // chose SHA-1.
+      hasher = MessageDigest.getInstance("SHA-1");
     }
 
     @Override
     public void append(String s) {
-      builder.append((s == null) ? "null" : s);
+      try {
+        hasher.update(((s == null) ? "null" : s).getBytes("UTF-8"));
+      } catch (UnsupportedEncodingException e) {
+        // This can never occur. Thanks, Java.
+      }
     }
 
     @Override
     public void append(int profileCreation) {
       append(Integer.toString(profileCreation, 10));
     }
 
     @Override
     public String toString() {
       // We *could* use ASCII85… but the savings would be negated by the
       // inclusion of JSON-unsafe characters like double-quote.
-      final byte[] inputBytes;
-      try {
-        inputBytes = builder.toString().getBytes("UTF-8");
-      } catch (UnsupportedEncodingException e) {
-        Logger.warn(LOG_TAG, "Invalid charset String passed to getBytes", e);
-        return null;
-      }
-
-      // Note to the security-minded reader: we deliberately use SHA-1 here, not
-      // a stronger hash. These identifiers don't strictly need a cryptographic
-      // hash function, because there is negligible value in attacking the hash.
-      // We use SHA-1 because it's *shorter* -- the exact same reason that Git
-      // chose SHA-1.
-      final byte[] hash = NativeCrypto.sha1(inputBytes);
-      return new Base64(-1, null, false).encodeAsString(hash);
+      return new Base64(-1, null, false).encodeAsString(hasher.digest());
     }
   }
 
   /**
    * Ensure that the {@link Environment} has been registered with its
    * storage layer, and can be used to annotate events.
    *
    * It's safe to call this method more than once, and each time you'll
--- a/mobile/android/tests/background/junit3/android-services-files.mk
+++ b/mobile/android/tests/background/junit3/android-services-files.mk
@@ -25,17 +25,16 @@ BACKGROUND_TESTS_JAVA_FILES := \
   src/fxa/TestBrowserIDKeyPairGeneration.java \
   src/healthreport/MockDatabaseEnvironment.java \
   src/healthreport/MockHealthReportDatabaseStorage.java \
   src/healthreport/MockHealthReportSQLiteOpenHelper.java \
   src/healthreport/MockProfileInformationCache.java \
   src/healthreport/prune/TestHealthReportPruneService.java \
   src/healthreport/prune/TestPrunePolicyDatabaseStorage.java \
   src/healthreport/TestEnvironmentBuilder.java \
-  src/healthreport/TestEnvironmentV1HashAppender.java \
   src/healthreport/TestHealthReportBroadcastService.java \
   src/healthreport/TestHealthReportDatabaseStorage.java \
   src/healthreport/TestHealthReportGenerator.java \
   src/healthreport/TestHealthReportProvider.java \
   src/healthreport/TestHealthReportSQLiteOpenHelper.java \
   src/healthreport/TestProfileInformationCache.java \
   src/healthreport/upload/TestAndroidSubmissionClient.java \
   src/healthreport/upload/TestHealthReportUploadService.java \
deleted file mode 100644
--- a/mobile/android/tests/background/junit3/src/healthreport/TestEnvironmentV1HashAppender.java
+++ /dev/null
@@ -1,146 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-package org.mozilla.gecko.background.healthreport;
-
-import java.io.UnsupportedEncodingException;
-import java.security.MessageDigest;
-import java.security.NoSuchAlgorithmException;
-import java.util.Arrays;
-import java.util.LinkedList;
-
-import org.mozilla.apache.commons.codec.binary.Base64;
-import org.mozilla.gecko.background.healthreport.EnvironmentV1.EnvironmentAppender;
-import org.mozilla.gecko.background.healthreport.EnvironmentV1.HashAppender;
-import org.mozilla.gecko.background.helpers.FakeProfileTestCase;
-import org.mozilla.gecko.sync.Utils;
-
-/**
- * Tests the HashAppender functionality. Note that these tests must be run on an Android
- * device because the SHA-1 native library needs to be loaded.
- */
-public class TestEnvironmentV1HashAppender extends FakeProfileTestCase {
-  // input and expected values via: http://oauth.googlecode.com/svn/code/c/liboauth/src/sha1.c
-  private final static String[] INPUTS = new String[] {
-    "abc",
-    "abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq",
-    "" // To be filled in below.
-  };
-  static {
-    final String baseStr = "01234567";
-    final int repetitions = 80;
-    final StringBuilder builder = new StringBuilder(baseStr.length() * repetitions);
-    for (int i = 0; i < 80; ++i) {
-      builder.append(baseStr);
-    }
-    INPUTS[2] = builder.toString();
-  }
-
-  private final static String[] EXPECTEDS = new String[] {
-    "a9993e364706816aba3e25717850c26c9cd0d89d",
-    "84983e441c3bd26ebaae4aa1f95129e5e54670f1",
-    "dea356a2cddd90c7a7ecedc5ebb563934f460452"
-  };
-  static {
-    for (int i = 0; i < EXPECTEDS.length; ++i) {
-      EXPECTEDS[i] = new Base64(-1, null, false).encodeAsString(Utils.hex2Byte(EXPECTEDS[i]));
-    }
-  }
-
-  public void testSHA1Hashing() throws Exception {
-    for (int i = 0; i < INPUTS.length; ++i) {
-      final String input = INPUTS[i];
-      final String expected = EXPECTEDS[i];
-
-      final HashAppender appender = new HashAppender();
-      addStringToAppenderInParts(appender, input);
-      final String result = appender.toString();
-
-      assertEquals(expected, result);
-    }
-  }
-
-  /**
-   * Tests to ensure output is the same as the former MessageDigest implementation (bug 959652).
-   */
-  public void testAgainstMessageDigestImpl() throws Exception {
-    // List.add doesn't allow add(null) so we make a LinkedList here.
-    final LinkedList<String> inputs = new LinkedList<String>(Arrays.asList(INPUTS));
-    inputs.add(null);
-
-    for (final String input : inputs) {
-      final HashAppender hAppender = new HashAppender();
-      final MessageDigestHashAppender mdAppender = new MessageDigestHashAppender();
-
-      hAppender.append(input);
-      mdAppender.append(input);
-
-      final String hResult = hAppender.toString();
-      final String mdResult = mdAppender.toString();
-      assertEquals(mdResult, hResult);
-    }
-  }
-
-  public void testIntegersAgainstMessageDigestImpl() throws Exception {
-    final int[] INPUTS = {Integer.MIN_VALUE, -1337, -42, 0, 42, 1337, Integer.MAX_VALUE};
-    for (final int input : INPUTS) {
-      final HashAppender hAppender = new HashAppender();
-      final MessageDigestHashAppender mdAppender = new MessageDigestHashAppender();
-
-      hAppender.append(input);
-      mdAppender.append(input);
-
-      final String hResult = hAppender.toString();
-      final String mdResult = mdAppender.toString();
-      assertEquals(mdResult, hResult);
-    }
-  }
-
-  private void addStringToAppenderInParts(final EnvironmentAppender appender, final String input) {
-    int substrInd = 0;
-    int substrLength = 1;
-    while (substrInd < input.length()) {
-      final int endInd = Math.min(substrInd + substrLength, input.length());
-
-      appender.append(input.substring(substrInd, endInd));
-
-      substrInd = endInd;
-      ++substrLength;
-    }
-  }
-
-  // --- COPY-PASTA'D CODE, FOR TESTING PURPOSES. ---
-  public static class MessageDigestHashAppender extends EnvironmentAppender {
-    final MessageDigest hasher;
-
-    public MessageDigestHashAppender() throws NoSuchAlgorithmException {
-      // Note to the security-minded reader: we deliberately use SHA-1 here, not
-      // a stronger hash. These identifiers don't strictly need a cryptographic
-      // hash function, because there is negligible value in attacking the hash.
-      // We use SHA-1 because it's *shorter* -- the exact same reason that Git
-      // chose SHA-1.
-      hasher = MessageDigest.getInstance("SHA-1");
-    }
-
-    @Override
-    public void append(String s) {
-      try {
-        hasher.update(((s == null) ? "null" : s).getBytes("UTF-8"));
-      } catch (UnsupportedEncodingException e) {
-        // This can never occur. Thanks, Java.
-      }
-    }
-
-    @Override
-    public void append(int profileCreation) {
-      append(Integer.toString(profileCreation, 10));
-    }
-
-    @Override
-    public String toString() {
-      // We *could* use ASCII85… but the savings would be negated by the
-      // inclusion of JSON-unsafe characters like double-quote.
-      return new Base64(-1, null, false).encodeAsString(hasher.digest());
-    }
-  }
-}