diff options
4 files changed, 92 insertions, 71 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 8cec03f4ca..c20cbb22a1 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 @@ -69,18 +69,19 @@ public class HttpDownloader { "Error downloading " + url + " to " + destination + ": " + e.getMessage()); } + String downloadedSha256; try { - String downloadedSha256 = getSha256(destination); - if (!downloadedSha256.equals(sha256)) { - throw new IOException( - "Downloaded file at " + destination + " has SHA-256 of " + downloadedSha256 - + ", does not match expected SHA-256 (" + sha256 + ")"); - } + downloadedSha256 = getHash(Hashing.sha256().newHasher(), destination); } catch (IOException e) { throw new IOException( "Could not hash file " + destination + ": " + e.getMessage() + ", expected SHA-256 of " + sha256 + ")"); } + if (!downloadedSha256.equals(sha256)) { + throw new IOException( + "Downloaded file at " + destination + " has SHA-256 of " + downloadedSha256 + + ", does not match expected SHA-256 (" + sha256 + ")"); + } return destination; } @@ -89,9 +90,7 @@ public class HttpDownloader { return Channels.newChannel(url.openStream()); } - private String getSha256(Path path) throws IOException { - Hasher hasher = Hashing.sha256().newHasher(); - + public static String getHash(Hasher hasher, Path path) throws IOException { byte byteBuffer[] = new byte[BUFFER_SIZE]; try (InputStream stream = path.getInputStream()) { int numBytesRead = stream.read(byteBuffer); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java index 9d5b429264..501937fba8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java @@ -18,6 +18,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Ascii; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; +import com.google.common.hash.Hasher; +import com.google.common.hash.Hashing; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.bazel.repository.DecompressorFactory.DecompressorException; import com.google.devtools.build.lib.bazel.repository.DecompressorFactory.JarDecompressor; @@ -56,7 +58,6 @@ import org.eclipse.aether.transport.file.FileTransporterFactory; import org.eclipse.aether.transport.http.HttpTransporterFactory; import java.io.IOException; -import java.util.Arrays; import java.util.List; /** @@ -80,15 +81,7 @@ public class MavenJarFunction extends HttpArchiveFunction { MavenDownloader createMavenDownloader(AttributeMap mapper) { String name = mapper.getName(); Path outputDirectory = getExternalRepositoryDirectory().getRelative(name); - MavenDownloader downloader = new MavenDownloader(name, mapper.get("group_id", Type.STRING), - mapper.get("artifact_id", Type.STRING), mapper.get("version", Type.STRING), - outputDirectory); - - List<String> repositories = mapper.get("repositories", Type.STRING_LIST); - if (repositories != null && !repositories.isEmpty()) { - downloader.setRepositories(repositories); - } - + MavenDownloader downloader = new MavenDownloader(name, mapper, outputDirectory); return downloader; } @@ -149,18 +142,32 @@ public class MavenJarFunction extends HttpArchiveFunction { private final String artifactId; private final String version; private final Path outputDirectory; - private List<RemoteRepository> repositories; + private final String sha1; + // TODO(kchodorow): change this to a single repository on 9/15. + private final List<RemoteRepository> repositories; - MavenDownloader(String name, String groupId, String artifactId, String version, - Path outputDirectory) { + public MavenDownloader(String name, AttributeMap mapper, Path outputDirectory) { this.name = name; - this.groupId = groupId; - this.artifactId = artifactId; - this.version = version; + this.groupId = mapper.get("group_id", Type.STRING); + this.artifactId = mapper.get("artifact_id", Type.STRING); + this.version = mapper.get("version", Type.STRING); this.outputDirectory = outputDirectory; - repositories = Arrays.asList( - new RemoteRepository.Builder("central", "default", MAVEN_CENTRAL_URL).build()); - this.repositories = ImmutableList.copyOf(repositories); + this.sha1 = mapper.get("sha1", Type.STRING); + if (mapper.has("repository", Type.STRING)) { + this.repositories = ImmutableList.of(new RemoteRepository.Builder( + "user-defined repository", "default", mapper.get("repository", Type.STRING)).build()); + } else if (mapper.has("repositories", Type.STRING_LIST)) { + // TODO(kchodorow): remove after 9/15, uses deprecated list of repositories attribute. + this.repositories = Lists.newArrayList(); + for (String repositoryUrl : mapper.get("repositories", Type.STRING_LIST)) { + this.repositories.add(new RemoteRepository.Builder( + "user-defined repository " + repositories.size(), "default", repositoryUrl).build()); + } + } else { + this.repositories = Lists.newArrayList(); + this.repositories.add(new RemoteRepository.Builder( + "central", "default", MAVEN_CENTRAL_URL).build()); + } } /** @@ -178,17 +185,6 @@ public class MavenJarFunction extends HttpArchiveFunction { } /** - * Customizes the set of Maven repositories to check. Takes a list of repository URLs. - */ - public void setRepositories(List<String> repositories) { - this.repositories = Lists.newArrayList(); - for (String repositoryUrl : repositories) { - this.repositories.add(new RemoteRepository.Builder( - "user-defined repository " + repositories.size(), "default", repositoryUrl).build()); - } - } - - /** * Download the Maven artifact to the output directory. Returns the path to the jar. */ public Path download() throws IOException { @@ -206,7 +202,16 @@ public class MavenJarFunction extends HttpArchiveFunction { } catch (ArtifactResolutionException e) { throw new IOException("Failed to fetch Maven dependency: " + e.getMessage()); } - return outputDirectory.getRelative(artifact.getFile().getAbsolutePath()); + + // Verify checksum. + Path downloadPath = outputDirectory.getRelative(artifact.getFile().getAbsolutePath()); + Hasher hasher = Hashing.sha1().newHasher(); + String downloadSha1 = HttpDownloader.getHash(hasher, downloadPath); + if (!sha1.equals(downloadSha1)) { + throw new IOException("Downloaded file at " + downloadPath + " has SHA-1 of " + + downloadSha1 + ", does not match expected SHA-1 (" + sha1 + ")"); + } + return downloadPath; } private RepositorySystemSession newRepositorySystemSession(RepositorySystem system) { 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 056e946329..fb59402656 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 @@ -48,31 +48,24 @@ public class MavenJarRule implements RuleDefinition { ${SYNOPSIS} <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ .add(attr("version", Type.STRING).mandatory()) - /* <!-- #BLAZE_RULE(maven_jar).ATTRIBUTE(repositories) --> - A list of repositories to use to attempt to fetch the jar. + /* <!-- #BLAZE_RULE(maven_jar).ATTRIBUTE(repository) --> + A URL for a Maven repository to fetch the jar from. ${SYNOPSIS} - <p>Defaults to Maven Central ("repo1.maven.org"). If repositories are specified, they will - be checked in the order listed here (Maven Central will not be checked in this case, - unless it is on the list).</p> + <p>Defaults to Maven Central ("central.maven.org").</p> - <p><b>To be implemented: add a maven_repositories rule that allows a list of repositories - to be labeled.</b></p> + <p><b>To be implemented: add a maven_repository rule that allows a default repository + to be specified once.</b></p> <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ - .add(attr("repositories", Type.STRING_LIST)) - /* <!-- #BLAZE_RULE(maven_jar).ATTRIBUTE(exclusions) --> - Transitive dependencies of this dependency that should not be downloaded. - ${SYNOPSIS} - - <p>Defaults to None: Bazel will download all of the dependencies requested by the Maven - dependency. If exclusions are specified, they will not be downloaded.</p> - - <p>Exclusions are specified in the format "<group_id>:<artifact_id>", for example, - "com.google.guava:guava".</p> - - <p><b>Not yet implemented.</b></p> - <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ - .add(attr("exclusions", Type.STRING_LIST)) + .add(attr("repository", Type.STRING)) + .add(attr("repositories", Type.STRING_LIST)).setUndocumented() + /* <!-- #BLAZE_RULE(maven_jar).ATTRIBUTE(sha1) --> + A SHA-1 hash of the desired jar. + ${SYNOPSIS} + + <p>If the downloaded jar does not match this hash, Bazel will error out.</p> + <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ + .add(attr("sha1", Type.STRING).mandatory()) .setWorkspaceOnly() .build(); } diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh index db2b174edf..1f8eec689f 100755 --- a/src/test/shell/bazel/external_integration_test.sh +++ b/src/test/shell/bazel/external_integration_test.sh @@ -45,15 +45,15 @@ EOF } case "${PLATFORM}" in - darwin) + darwin) function nc_l() { - nc -l $1 - } + nc -l $1 + } ;; - *) + *) function nc_l() { - nc -l -p $1 -q 1 - } + nc -l -p $1 -q 1 + } ;; esac @@ -98,6 +98,8 @@ EOF ${bazel_javabase}/bin/jar cf $test_jar carnivore/Mongoose.class sha256=$(sha256sum $test_jar | cut -f 1 -d ' ') + # OS X doesn't have sha1sum, so use openssl. + sha1=$(openssl sha1 $test_jar | cut -f 2 -d ' ') serve_file $test_jar cd ${WORKSPACE_DIR} } @@ -324,7 +326,8 @@ maven_jar( group_id = "com.example.carnivore", artifact_id = "carnivore", version = "1.23", - repositories = ['http://localhost:$nc_port/'] + repository = 'http://localhost:$nc_port/', + sha1 = '$sha1', ) bind(name = 'mongoose', actual = '@endangered//jar') EOF @@ -345,14 +348,14 @@ EOF nc_port=$(pick_random_unused_tcp_port) || exit 1 nc_l $nc_port < $http_response & nc_pid=$! - cat > WORKSPACE <<EOF maven_jar( name = 'endangered', group_id = "carnivore", artifact_id = "carnivore", version = "1.23", - repositories = ['http://localhost:$nc_port/'] + repository = 'http://localhost:$nc_port/', + sha1 = '0000000000000000000000000000000000000000', ) bind(name = 'mongoose', actual = '@endangered//jar') EOF @@ -362,6 +365,26 @@ EOF expect_log "Failed to fetch Maven dependency: Could not find artifact" } +function test_maven_jar_mismatched_sha1() { + serve_jar + + cat > WORKSPACE <<EOF +maven_jar( + name = 'endangered', + group_id = "com.example.carnivore", + artifact_id = "carnivore", + version = "1.23", + repository = 'http://localhost:$nc_port/', + sha1 = '$sha256', +) +bind(name = 'mongoose', actual = '@endangered//jar') +EOF + + bazel fetch //zoo:ball-pit >& $TEST_log && echo "Expected fetch to fail" + kill_nc + expect_log "has SHA-1 of $sha1, does not match expected SHA-1 ($sha256)" +} + function test_new_remote_repo() { # Create a zipped-up repository HTTP response. local repo2=$TEST_TMPDIR/repo2 @@ -427,7 +450,8 @@ maven_jar( group_id = "com.example.carnivore", artifact_id = "carnivore", version = "1.23", - repositories = ['http://localhost:$nc_port/'] + repository = 'http://localhost:$nc_port/', + sha1 = '$sha1', ) bind(name = 'mongoose', actual = '@endangered//jar') EOF |