diff options
Diffstat (limited to 'src')
13 files changed, 199 insertions, 94 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java index e4db5a4ebe..a760fd9aa4 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java @@ -189,7 +189,7 @@ public final class BuildOptions implements Cloneable, Serializable { } public String computeChecksum() { - return Fingerprint.md5Digest(computeCacheKey()); + return Fingerprint.getHexDigest(computeCacheKey()); } /** String representation of build options. */ diff --git a/src/main/java/com/google/devtools/build/lib/exec/BinTools.java b/src/main/java/com/google/devtools/build/lib/exec/BinTools.java index 46991b3f85..a24ba60812 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/BinTools.java +++ b/src/main/java/com/google/devtools/build/lib/exec/BinTools.java @@ -262,7 +262,7 @@ public final class BinTools { private static FileArtifactValue hash(Path path) throws IOException { DigestHashFunction hashFn = path.getFileSystem().getDigestFunction(); - Hasher hasher = hashFn.getHash().newHasher(); + Hasher hasher = hashFn.getHashFunction().newHasher(); int bytesCopied = 0; try (InputStream in = path.getInputStream()) { byte[] buffer = new byte[1024]; diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java index 1b5e82096e..4fe3c31711 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java @@ -221,7 +221,7 @@ public class SpawnLogContext implements ActionContext { ((VirtualActionInput) input).writeTo(buffer); byte[] blob = buffer.toByteArray(); return digest - .setHash(hashFunction.getHash().hashBytes(blob).toString()) + .setHash(hashFunction.getHashFunction().hashBytes(blob).toString()) .setSizeBytes(blob.length) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java b/src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java index 5759568e79..bbe1da1d2d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java +++ b/src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java @@ -59,7 +59,7 @@ public class DigestUtil { } public Digest compute(byte[] blob) { - return buildDigest(hashFn.getHash().hashBytes(blob).toString(), blob.length); + return buildDigest(hashFn.getHashFunction().hashBytes(blob).toString(), blob.length); } public Digest compute(Path file) throws IOException { @@ -114,7 +114,7 @@ public class DigestUtil { } public HashingOutputStream newHashingOutputStream(OutputStream out) { - return new HashingOutputStream(hashFn.getHash(), out); + return new HashingOutputStream(hashFn.getHashFunction(), out); } public String toString(Digest digest) { diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java index a177598036..0e0ba2d8bf 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java @@ -86,8 +86,11 @@ public final class SandboxModule extends BlazeModule { if (options.sandboxBase.isEmpty()) { return env.getOutputBase().getRelative("sandbox"); } else { - String dirName = String.format("%s-sandbox.%s", env.getRuntime().getProductName(), - Fingerprint.md5Digest(env.getOutputBase().toString())); + String dirName = + String.format( + "%s-sandbox.%s", + env.getRuntime().getProductName(), + Fingerprint.getHexDigest(env.getOutputBase().toString())); FileSystem fileSystem = env.getRuntime().getFileSystem(); Path resolvedSandboxBase = fileSystem.getPath(options.sandboxBase).resolveSymbolicLinks(); return resolvedSandboxBase.getRelative(dirName); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentValue.java index 648cd67d0c..e6e15616a5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentValue.java @@ -72,7 +72,7 @@ public class ConfigurationFragmentValue implements SkyValue { private ConfigurationFragmentKey( BuildOptions buildOptions, Class<? extends Fragment> fragmentType) { this.buildOptions = Preconditions.checkNotNull(buildOptions); - this.checksum = Fingerprint.md5Digest(buildOptions.computeCacheKey()); + this.checksum = Fingerprint.getHexDigest(buildOptions.computeCacheKey()); this.fragmentType = Preconditions.checkNotNull(fragmentType); } diff --git a/src/main/java/com/google/devtools/build/lib/util/Fingerprint.java b/src/main/java/com/google/devtools/build/lib/util/Fingerprint.java index a2bafa6f6a..556bafdb0a 100644 --- a/src/main/java/com/google/devtools/build/lib/util/Fingerprint.java +++ b/src/main/java/com/google/devtools/build/lib/util/Fingerprint.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.util; import com.google.common.io.ByteStreams; +import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.protobuf.CodedOutputStream; @@ -23,47 +24,45 @@ import java.nio.charset.StandardCharsets; import java.security.DigestException; import java.security.DigestOutputStream; import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; import java.util.Map; import java.util.UUID; import java.util.function.Consumer; import javax.annotation.Nullable; /** - * Simplified wrapper for MD5 message digests. + * Simplified wrapper for computing message digests. * * @see java.security.MessageDigest */ public final class Fingerprint implements Consumer<String> { - private static final MessageDigest MD5_PROTOTYPE; - private static final boolean MD5_PROTOTYPE_SUPPORTS_CLONE; - - static { - MD5_PROTOTYPE = getMd5Instance(); - MD5_PROTOTYPE_SUPPORTS_CLONE = supportsClone(MD5_PROTOTYPE); - } - // Make novel use of a CodedOutputStream, which is good at efficiently serializing data. By // flushing at the end of each digest we can continue to use the stream. private final CodedOutputStream codedOut; - private final MessageDigest md5; + private final MessageDigest messageDigest; /** Creates and initializes a new instance. */ - public Fingerprint() { - md5 = cloneOrCreateMd5(); + public Fingerprint(DigestHashFunction digestFunction) { + messageDigest = digestFunction.cloneOrCreateMessageDigest(); // This is a lot of indirection, but CodedOutputStream does a reasonable job of converting // strings to bytes without creating a whole bunch of garbage, which pays off. - codedOut = CodedOutputStream.newInstance( - new DigestOutputStream(ByteStreams.nullOutputStream(), md5), - /*bufferSize=*/ 1024); + codedOut = + CodedOutputStream.newInstance( + new DigestOutputStream(ByteStreams.nullOutputStream(), messageDigest), + /*bufferSize=*/ 1024); + } + + public Fingerprint() { + // TODO(b/112460990): Use the value from DigestHashFunction.getDefault(), but check for + // contention. + this(DigestHashFunction.MD5); } /** * Completes the hash computation by doing final operations and resets the underlying state, * allowing this instance to be used again. * - * @return the MD5 digest as a 16-byte array + * @return the digest as a 16-byte array * @see java.security.MessageDigest#digest() */ public byte[] digestAndReset() { @@ -72,7 +71,7 @@ public final class Fingerprint implements Consumer<String> { } catch (IOException e) { throw new IllegalStateException("failed to flush", e); } - return md5.digest(); + return messageDigest.digest(); } /** @@ -87,7 +86,7 @@ public final class Fingerprint implements Consumer<String> { public void digestAndReset(byte[] buf, int offset, int len) { try { codedOut.flush(); - md5.digest(buf, offset, len); + messageDigest.digest(buf, offset, len); } catch (IOException e) { throw new IllegalStateException("failed to flush", e); } catch (DigestException e) { @@ -250,18 +249,6 @@ public final class Fingerprint implements Consumer<String> { return this; } - private static MessageDigest cloneOrCreateMd5() { - if (MD5_PROTOTYPE_SUPPORTS_CLONE) { - try { - return (MessageDigest) MD5_PROTOTYPE.clone(); - } catch (CloneNotSupportedException e) { - throw new IllegalStateException("Could not clone md5", e); - } - } else { - return getMd5Instance(); - } - } - private static String hexDigest(byte[] digest) { StringBuilder b = new StringBuilder(32); for (int i = 0; i < digest.length; i++) { @@ -272,33 +259,22 @@ public final class Fingerprint implements Consumer<String> { return b.toString(); } - private static MessageDigest getMd5Instance() { - try { - return MessageDigest.getInstance("md5"); - } catch (NoSuchAlgorithmException e) { - throw new IllegalStateException("md5 not available", e); - } - } - - private static boolean supportsClone(MessageDigest toCheck) { - try { - toCheck.clone(); - return true; - } catch (CloneNotSupportedException e) { - return false; - } - } - // -------- Convenience methods ---------------------------- /** - * Computes the hex digest from a String using UTF8 encoding and returning - * the hexDigest(). + * Computes the hex digest from a String using UTF8 encoding and returning the hexDigest(). * * @param input the String from which to compute the digest */ - public static String md5Digest(String input) { - return hexDigest(cloneOrCreateMd5().digest(input.getBytes(StandardCharsets.UTF_8))); + public static String getHexDigest(String input) { + // TODO(b/112460990): This convenience method, if kept should not use MD5 by default, but should + // use the value from DigestHashFunction.getDefault(). However, this gets called during class + // loading in a few places, before setDefault() has been called, so these call-sites should be + // removed before this can be done safely. + return hexDigest( + DigestHashFunction.MD5 + .cloneOrCreateMessageDigest() + .digest(input.getBytes(StandardCharsets.UTF_8))); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/vfs/DigestHashFunction.java b/src/main/java/com/google/devtools/build/lib/vfs/DigestHashFunction.java index 12aee0757b..123e4b5a9c 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/DigestHashFunction.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/DigestHashFunction.java @@ -14,16 +14,26 @@ package com.google.devtools.build.lib.vfs; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.hash.HashFunction; import com.google.common.hash.Hashing; import com.google.devtools.common.options.Converter; import com.google.devtools.common.options.OptionsParsingException; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map.Entry; -/** Type of hash function to use for digesting files. */ +/** + * Type of hash function to use for digesting files. + * + * <p>This tracks parallel {@link java.security.MessageDigest} and {@link HashFunction} interfaces + * for each provided hash, as Bazel uses both - MessageDigest where performance is critical and + * HashFunctions where ease-of-use wins over. + */ // The underlying HashFunctions are immutable and thread safe. public class DigestHashFunction { // This map must be declared first to make sure that calls to register() have it ready. @@ -36,33 +46,24 @@ public class DigestHashFunction { private static DigestHashFunction defaultHash; private static boolean defaultHasBeenSet = false; - private final HashFunction hash; + private final HashFunction hashFunction; private final String name; + private final MessageDigest messageDigestPrototype; + private final boolean messageDigestPrototypeSupportsClone; - private DigestHashFunction(HashFunction hash, String name) { - this.hash = hash; + private DigestHashFunction(HashFunction hashFunction, String name) { + this.hashFunction = hashFunction; this.name = name; - } - - public HashFunction getHash() { - return hash; - } - - public boolean isValidDigest(byte[] digest) { - // TODO(b/109764197): Remove this check to accept variable-length hashes. - return digest != null && digest.length * 8 == hash.bits(); - } - - @Override - public String toString() { - return name; + this.messageDigestPrototype = getMessageDigestInstance(); + this.messageDigestPrototypeSupportsClone = supportsClone(messageDigestPrototype); } /** * Creates a new DigestHashFunction that is registered to be recognized by its name in {@link * DigestFunctionConverter}. * - * @param hashName the canonical name for this hash function. + * @param hashName the canonical name for this hash function - and the name that can be used to + * uncover the MessageDigest. * @param altNames alternative names that will be mapped to this function by the converter but * will not serve as the canonical name for the DigestHashFunction. * @param hash The {@link HashFunction} to register. @@ -70,6 +71,15 @@ public class DigestHashFunction { */ public static DigestHashFunction register( HashFunction hash, String hashName, String... altNames) { + try { + MessageDigest.getInstance(hashName); + } catch (NoSuchAlgorithmException e) { + throw new IllegalArgumentException( + "The hash function name provided does not correspond to a valid MessageDigest: " + + hashName, + e); + } + DigestHashFunction hashFunction = new DigestHashFunction(hash, hashName); List<String> names = ImmutableList.<String>builder().add(hashName).add(altNames).build(); synchronized (hashFunctionRegistry) { @@ -149,4 +159,55 @@ public class DigestHashFunction { return "hash function"; } } + + public HashFunction getHashFunction() { + return hashFunction; + } + + public MessageDigest cloneOrCreateMessageDigest() { + if (messageDigestPrototypeSupportsClone) { + try { + return (MessageDigest) messageDigestPrototype.clone(); + } catch (CloneNotSupportedException e) { + // We checked at initialization that this could be cloned, so this should never happen. + throw new IllegalStateException("Could not clone message digest", e); + } + } else { + return getMessageDigestInstance(); + } + } + + public boolean isValidDigest(byte[] digest) { + // TODO(b/109764197): Remove this check to accept variable-length hashes. + return digest != null && digest.length * 8 == hashFunction.bits(); + } + + @Override + public String toString() { + return name; + } + + private MessageDigest getMessageDigestInstance() { + try { + return MessageDigest.getInstance(name); + } catch (NoSuchAlgorithmException e) { + // We check when we register() this digest function that the message digest exists. This + // should never happen. + throw new IllegalStateException("message digest " + name + " not available", e); + } + } + + private static boolean supportsClone(MessageDigest toCheck) { + try { + toCheck.clone(); + return true; + } catch (CloneNotSupportedException e) { + return false; + } + } + + @VisibleForTesting + static Collection<DigestHashFunction> getPossibleHashFunctions() { + return hashFunctionRegistry.values(); + } } diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java index 1c3a163f62..2305aa0dc0 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java @@ -275,7 +275,7 @@ public abstract class FileSystem { public InputStream openStream() throws IOException { return getInputStream(path); } - }.hash(digestFunction.getHash()).asBytes(); + }.hash(digestFunction.getHashFunction()).asBytes(); } /** diff --git a/src/main/java/com/google/devtools/build/lib/vfs/Path.java b/src/main/java/com/google/devtools/build/lib/vfs/Path.java index 497b0bbd5d..f20c25a8b9 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/Path.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/Path.java @@ -777,7 +777,7 @@ public class Path public String getDirectoryDigest() throws IOException { List<String> entries = new ArrayList<String>(fileSystem.getDirectoryEntries(this)); Collections.sort(entries); - Hasher hasher = fileSystem.getDigestFunction().getHash().newHasher(); + Hasher hasher = fileSystem.getDigestFunction().getHashFunction().newHasher(); for (String entry : entries) { Path path = this.getChild(entry); FileStatus stat = path.stat(Symlinks.NOFOLLOW); diff --git a/src/test/java/com/google/devtools/build/lib/util/FingerprintTest.java b/src/test/java/com/google/devtools/build/lib/util/FingerprintTest.java index 372b48d80c..b3fd3c4ef2 100644 --- a/src/test/java/com/google/devtools/build/lib/util/FingerprintTest.java +++ b/src/test/java/com/google/devtools/build/lib/util/FingerprintTest.java @@ -58,7 +58,7 @@ public class FingerprintTest { public void bytesFingerprint() { assertThat(new Fingerprint().addBytes("Hello World!".getBytes(UTF_8)).hexDigestAndReset()) .isEqualTo("ed076287532e86365e841e92bfc50d8c"); - assertThat(Fingerprint.md5Digest("Hello World!")) + assertThat(Fingerprint.getHexDigest("Hello World!")) .isEqualTo("ed076287532e86365e841e92bfc50d8c"); } diff --git a/src/test/java/com/google/devtools/build/lib/vfs/DigestHashFunctionTest.java b/src/test/java/com/google/devtools/build/lib/vfs/DigestHashFunctionGlobalsTest.java index 1e4044bd31..77f7b62b21 100644 --- a/src/test/java/com/google/devtools/build/lib/vfs/DigestHashFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/vfs/DigestHashFunctionGlobalsTest.java @@ -29,7 +29,7 @@ import org.junit.runners.JUnit4; * equality. */ @RunWith(JUnit4.class) -public class DigestHashFunctionTest { +public class DigestHashFunctionGlobalsTest { private final DigestFunctionConverter converter = new DigestFunctionConverter(); @Before @@ -64,22 +64,26 @@ public class DigestHashFunctionTest { @Test public void lateRegistrationGetsPickedUpByConverter() throws Exception { - DigestHashFunction.register(Hashing.goodFastHash(32), "goodFastHash32"); + DigestHashFunction.register(Hashing.goodFastHash(32), "SHA-512"); - assertThat(converter.convert("goodFastHash32")).isSameAs(converter.convert("GOODFASTHASH32")); + assertThat(converter.convert("SHA-512")).isSameAs(converter.convert("sha-512")); } @Test public void lateRegistrationWithAlternativeNamesGetsPickedUpByConverter() throws Exception { - DigestHashFunction.register( - Hashing.goodFastHash(64), "goodFastHash64", "goodFastHash-64", "good-fast-hash-64"); - - assertThat(converter.convert("goodFastHash64")).isSameAs(converter.convert("GOODFASTHASH64")); - assertThat(converter.convert("goodFastHash64")).isSameAs(converter.convert("goodFastHash-64")); - assertThat(converter.convert("goodFastHash64")) - .isSameAs(converter.convert("good-fast-hash-64")); - assertThat(converter.convert("goodFastHash64")) - .isSameAs(converter.convert("GOOD-fast-HASH-64")); + DigestHashFunction.register(Hashing.goodFastHash(64), "SHA-384", "SHA384", "SHA_384"); + + assertThat(converter.convert("SHA-384")).isSameAs(converter.convert("SHA-384")); + assertThat(converter.convert("Sha-384")).isSameAs(converter.convert("SHA-384")); + assertThat(converter.convert("sha-384")).isSameAs(converter.convert("SHA-384")); + + assertThat(converter.convert("SHA384")).isSameAs(converter.convert("SHA-384")); + assertThat(converter.convert("Sha384")).isSameAs(converter.convert("SHA-384")); + assertThat(converter.convert("sha384")).isSameAs(converter.convert("SHA-384")); + + assertThat(converter.convert("SHA_384")).isSameAs(converter.convert("SHA-384")); + assertThat(converter.convert("Sha_384")).isSameAs(converter.convert("SHA-384")); + assertThat(converter.convert("sha_384")).isSameAs(converter.convert("SHA-384")); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/vfs/DigestHashFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/vfs/DigestHashFunctionsTest.java new file mode 100644 index 0000000000..6b3b19923d --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/vfs/DigestHashFunctionsTest.java @@ -0,0 +1,61 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.vfs; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import java.nio.charset.StandardCharsets; +import java.util.Collection; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +/** + * Tests different {@link DigestHashFunction} for consistency between the MessageDigests and the + * HashFunctions that it exposes. + */ +@RunWith(Parameterized.class) +public class DigestHashFunctionsTest { + @Parameters(name = "{index}: digestHashFunction={0}") + public static Collection<DigestHashFunction[]> hashFunctions() { + // TODO(b/112537387): Remove the array-ification and return Collection<DigestHashFunction>. This + // is possible in Junit4.12, but 4.11 requires the array. Bazel 0.18 will have Junit4.12, so + // this can change then. + return DigestHashFunction.getPossibleHashFunctions() + .stream() + .map(dhf -> new DigestHashFunction[] {dhf}) + .collect(ImmutableList.toImmutableList()); + } + + @Parameter public DigestHashFunction digestHashFunction; + + private void assertHashFunctionAndMessageDigestEquivalentForInput(byte[] input) { + byte[] hashFunctionOutput = digestHashFunction.getHashFunction().hashBytes(input).asBytes(); + byte[] messageDigestOutput = digestHashFunction.cloneOrCreateMessageDigest().digest(input); + assertThat(hashFunctionOutput).isEqualTo(messageDigestOutput); + } + + @Test + public void emptyDigestIsConsistent() { + assertHashFunctionAndMessageDigestEquivalentForInput(new byte[] {}); + } + + @Test + public void shortDigestIsConsistent() { + assertHashFunctionAndMessageDigestEquivalentForInput("Bazel".getBytes(StandardCharsets.UTF_8)); + } +} |