Make sha256 optional

RELNOTES: The sha256 attribute is now optional (although recommended!) for
remote repository rules.

--
MOS_MIGRATED_REVID=114139613

7 files changed, 42 insertions, 19 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpDownloader.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpDownloader.java
index f6bf028cde..dd19f8305e 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpDownloader.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpDownloader.java
@@ -107,14 +107,16 @@ public class HttpDownloader {
     }
     Path destination = outputDirectory.getRelative(filename);
 
-    try {
-      String currentSha256 = getHash(Hashing.sha256().newHasher(), destination);
-      if (currentSha256.equals(sha256)) {
-        // No need to download.
-        return destination;
+    if (!sha256.isEmpty()) {
+      try {
+        String currentSha256 = getHash(Hashing.sha256().newHasher(), destination);
+        if (currentSha256.equals(sha256)) {
+          // No need to download.
+          return destination;
+        }
+      } catch (IOException e) {
+        // Ignore error trying to hash. We'll just download again.
       }
-    } catch (IOException e) {
-      // Ignore error trying to hash. We'll just download again.
     }
 
     AtomicInteger totalBytes = new AtomicInteger(0);
@@ -146,6 +148,14 @@ public class HttpDownloader {
       }, 0, TimeUnit.SECONDS);
     }
 
+    compareHashes(destination);
+    return destination;
+  }
+
+  private void compareHashes(Path destination) throws IOException {
+    if (sha256.isEmpty()) {
+      return;
+    }
     String downloadedSha256;
     try {
       downloadedSha256 = getHash(Hashing.sha256().newHasher(), destination);
@@ -159,7 +169,6 @@ public class HttpDownloader {
           "Downloaded file at " + destination + " has SHA-256 of " + downloadedSha256
               + ", does not match expected SHA-256 (" + sha256 + ")");
     }
-    return destination;
   }
 
   private ScheduledFuture<?> getLoggerHandle(final AtomicInteger totalBytes) {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpArchiveRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpArchiveRule.java
index cdcb866299..884217f017 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpArchiveRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpArchiveRule.java
@@ -45,9 +45,12 @@ public class HttpArchiveRule implements RuleDefinition {
         /* <!-- #BLAZE_RULE(http_archive).ATTRIBUTE(sha256) -->
          The expected SHA-256 hash of the file downloaded.
 
-         <p>This must match the SHA-256 hash of the file downloaded.</p>
+         <p>This must match the SHA-256 hash of the file downloaded. <em>It is a security risk to
+         omit the SHA-256 as remote files can change.</em> At best omitting this field will make
+         your build non-hermetic. It is optional to make development easier but should be set
+         before shipping.</p>
          <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("sha256", STRING).mandatory())
+        .add(attr("sha256", STRING))
         /* <!-- #BLAZE_RULE(http_archive).ATTRIBUTE(type) -->
          The archive type of the downloaded file.
 
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpFileRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpFileRule.java
index d85ac4b0bd..e0d20a8518 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpFileRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpFileRule.java
@@ -46,9 +46,12 @@ public class HttpFileRule implements RuleDefinition {
         /* <!-- #BLAZE_RULE(http_file).ATTRIBUTE(sha256) -->
          The expected SHA-256 of the file downloaded.
 
-         <p>This must match the SHA-256 of the file downloaded.</p>
+         <p>This must match the SHA-256 of the file downloaded. <em>It is a security risk to
+         omit the SHA-256 as remote files can change.</em> At best omitting this field will make
+         your build non-hermetic. It is optional to make development easier but should be set
+         before shipping.</p>
          <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("sha256", STRING).mandatory())
+        .add(attr("sha256", STRING))
         /* <!-- #BLAZE_RULE(http_file).ATTRIBUTE(executable) -->
          If the downloaded file should be made executable. Defaults to False.
 
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpJarRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpJarRule.java
index 51d189a34c..d249e9d2ea 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpJarRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpJarRule.java
@@ -44,9 +44,12 @@ public class HttpJarRule implements RuleDefinition {
         /* <!-- #BLAZE_RULE(http_jar).ATTRIBUTE(sha256) -->
         The expected SHA-256 of the file downloaded.
 
-        <p>This must match the SHA-256 of the file downloaded.</p>
+        <p>This must match the SHA-256 of the file downloaded. <em>It is a security risk to
+        omit the SHA-256 as remote files can change.</em> At best omitting this field will make
+        your build non-hermetic. It is optional to make development easier but should be set
+        before shipping.</p>
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("sha256", STRING).mandatory())
+        .add(attr("sha256", STRING))
         .setWorkspaceOnly()
         .build();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/MavenJarRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/MavenJarRule.java
index 84ed2375f6..da8639481c 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/MavenJarRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/MavenJarRule.java
@@ -62,7 +62,10 @@ public class MavenJarRule implements RuleDefinition {
         /* <!-- #BLAZE_RULE(maven_jar).ATTRIBUTE(sha1) -->
          A SHA-1 hash of the desired jar.
 
-         <p>If the downloaded jar does not match this hash, Bazel will error out.</p>
+         <p>If the downloaded jar does not match this hash, Bazel will error out. <em>It is a
+         security risk to omit the SHA-1 as remote files can change.</em> At best omitting this
+         field will make your build non-hermetic. It is optional to make development easier but
+         should be set before shipping.</p>
          <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
         .add(attr("sha1", Type.STRING))
         .setWorkspaceOnly()
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/NewHttpArchiveRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/NewHttpArchiveRule.java
index 8f610ce449..ab6224eb05 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/NewHttpArchiveRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/NewHttpArchiveRule.java
@@ -42,9 +42,12 @@ public class NewHttpArchiveRule implements RuleDefinition {
         /* <!-- #BLAZE_RULE(new_http_archive).ATTRIBUTE(sha256) -->
          The expected SHA-256 hash of the file downloaded.
 
-         <p>This must match the SHA-256 hash of the file downloaded.</p>
+         <p>This must match the SHA-256 hash of the file downloaded. <em>It is a security risk to
+         omit the SHA-256 as remote files can change.</em> At best omitting this field will make
+         your build non-hermetic. It is optional to make development easier but should be set
+         before shipping.</p>
          <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("sha256", STRING).mandatory())
+        .add(attr("sha256", STRING))
         /* <!-- #BLAZE_RULE(new_http_archive).ATTRIBUTE(build_file) -->
          The file to use as the BUILD file for this repository.
 
diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh
index a2c23760ff..640cf3723f 100755
--- a/src/test/shell/bazel/external_integration_test.sh
+++ b/src/test/shell/bazel/external_integration_test.sh
@@ -317,8 +317,7 @@ function test_jar_download() {
   serve_jar
 
   cat > WORKSPACE <<EOF
-http_jar(name = 'endangered', url = 'http://localhost:$nc_port/lib.jar',
-    sha256 = '$sha256')
+http_jar(name = 'endangered', url = 'http://localhost:$nc_port/lib.jar')
 EOF
 
   mkdir -p zoo