From a6476862fdcce53c6a57b0642599174aa14420ab Mon Sep 17 00:00:00 2001 From: mcilwain Date: Fri, 11 Jan 2019 07:11:34 -0800 Subject: [PATCH] Use Guava instead of Apache Commons for file ops in Ghostryde This backs out most of [] fixes the external build (which wasn't finding Apache Commons correctly), and makes miscellaneous tweaks and fixes, including better handling representing the default case of decrypting to stdout. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=228877090 --- gradle/core/build.gradle | 1 - gradle/dependencies.gradle | 1 - java/google/registry/repositories.bzl | 14 ------ java/google/registry/tools/BUILD | 2 - .../registry/tools/GhostrydeCommand.java | 49 ++++++++++--------- .../registry/tools/GhostrydeCommandTest.java | 45 +++++++++++++---- 6 files changed, 61 insertions(+), 51 deletions(-) diff --git a/gradle/core/build.gradle b/gradle/core/build.gradle index 70472fd70..27eba6897 100644 --- a/gradle/core/build.gradle +++ b/gradle/core/build.gradle @@ -150,7 +150,6 @@ dependencies { maybeRuntime deps['com.sun.activation:javax.activation'] maybeRuntime deps['com.thoughtworks.paranamer:paranamer'] maybeRuntime deps['commons-codec:commons-codec'] - compile deps['commons-io:commons-io'] maybeRuntime deps['commons-logging:commons-logging'] compile deps['dnsjava:dnsjava'] maybeRuntime deps['io.netty:netty-buffer'] diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 7ba6767b6..fb1fc9a00 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -74,7 +74,6 @@ ext { 'com.jcraft:jsch:0.1.53', 'com.jcraft:jzlib:1.1.3', 'commons-codec:commons-codec:1.10', - 'commons-io:commons-io:2.6', 'commons-logging:commons-logging:1.2', 'com.squareup:javapoet:1.8.0', 'com.squareup:javawriter:2.5.1', diff --git a/java/google/registry/repositories.bzl b/java/google/registry/repositories.bzl index cd5a23c68..30e8d930f 100644 --- a/java/google/registry/repositories.bzl +++ b/java/google/registry/repositories.bzl @@ -106,7 +106,6 @@ def domain_registry_repositories( omit_com_sun_xml_bind_jaxb_xjc = False, omit_com_thoughtworks_paranamer = False, omit_commons_codec = False, - omit_commons_io = False, omit_commons_logging = False, omit_dnsjava = False, omit_io_netty_buffer = False, @@ -338,8 +337,6 @@ def domain_registry_repositories( com_thoughtworks_paranamer() if not omit_commons_codec: commons_codec() - if not omit_commons_io: - commons_io() if not omit_commons_logging: commons_logging() if not omit_dnsjava: @@ -1802,17 +1799,6 @@ def commons_codec(): ], ) -def commons_io(): - java_import_external( - name = "commons_io", - licenses = ["notice"], # Apache License, Version 2.0 - jar_sha256 = "a10418348d234968600ccb1d988efcbbd08716e1d96936ccc1880e7d22513474", - jar_urls = [ - "http://maven.ibiblio.org/maven2/commons-io/commons-io/2.5/commons-io-2.5.jar", - "http://repo1.maven.org/maven2/commons-io/commons-io/2.5/commons-io-2.5.jar", - ], - ) - def commons_logging(): java_import_external( name = "commons_logging", diff --git a/java/google/registry/tools/BUILD b/java/google/registry/tools/BUILD index 4f91842ba..8236e2d77 100644 --- a/java/google/registry/tools/BUILD +++ b/java/google/registry/tools/BUILD @@ -66,7 +66,6 @@ java_library( "//java/google/registry/whois", "//java/google/registry/xjc", "//java/google/registry/xml", - "//third_party/java/jakarta_commons_io", "//third_party/jaxb", "//third_party/objectify:objectify-v4_1", "@com_beust_jcommander", @@ -93,7 +92,6 @@ java_library( "@com_google_oauth_client_jetty", "@com_google_re2j", "@com_googlecode_json_simple", - "@commons_io", "@io_bazel_rules_closure//closure/templates", "@jline", "@joda_time", diff --git a/java/google/registry/tools/GhostrydeCommand.java b/java/google/registry/tools/GhostrydeCommand.java index ee8807592..ba219f3d7 100644 --- a/java/google/registry/tools/GhostrydeCommand.java +++ b/java/google/registry/tools/GhostrydeCommand.java @@ -15,24 +15,23 @@ package google.registry.tools; import static com.google.common.base.Preconditions.checkArgument; -import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; +import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import com.google.common.io.ByteStreams; +import com.google.common.io.Files; import google.registry.keyring.api.KeyModule.Key; import google.registry.rde.Ghostryde; import google.registry.tools.params.PathParameter; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import javax.annotation.Nullable; import javax.inject.Inject; import javax.inject.Provider; -import org.apache.commons.io.IOUtils; -import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPPrivateKey; import org.bouncycastle.openpgp.PGPPublicKey; @@ -62,9 +61,11 @@ final class GhostrydeCommand implements CommandWithRemoteApi { "Output file. If this is a directory: (a) in --encrypt mode, the output " + "filename will be the input filename with '.ghostryde' appended, and will have an " + "extra '.length' file with the original file's length; (b) In --decrypt " - + "mode, the output filename will be the input filename with '.decrypt' appended.", + + "mode, the output filename will be the input filename with '.decrypt' appended. " + + "Defaults to stdout in --decrypt mode.", validateWith = PathParameter.class) - private Path output = Paths.get("/dev/stdout"); + @Nullable + private Path output; @Inject @Key("rdeStagingEncryptionKey") @@ -78,38 +79,40 @@ final class GhostrydeCommand implements CommandWithRemoteApi { public final void run() throws Exception { checkArgument(encrypt ^ decrypt, "Please specify either --encrypt or --decrypt"); if (encrypt) { + checkArgumentNotNull(output, "--output path is required in --encrypt mode"); runEncrypt(); } else { runDecrypt(); } } - private void runEncrypt() throws IOException, PGPException { - Path outFile = Files.isDirectory(output) - ? output.resolve(input.getFileName() + ".ghostryde") - : output; - Path lenOutFile = - Files.isDirectory(output) ? output.resolve(input.getFileName() + ".length") : null; - try (OutputStream out = Files.newOutputStream(outFile); - OutputStream lenOut = lenOutFile == null ? null : Files.newOutputStream(lenOutFile); + private void runEncrypt() throws IOException { + boolean isOutputToDir = output.toFile().isDirectory(); + Path outFile = isOutputToDir ? output.resolve(input.getFileName() + ".ghostryde") : output; + Path lenOutFile = isOutputToDir ? output.resolve(input.getFileName() + ".length") : null; + try (OutputStream out = Files.asByteSink(outFile.toFile()).openBufferedStream(); + OutputStream lenOut = + (lenOutFile == null) + ? null + : Files.asByteSink(lenOutFile.toFile()).openBufferedStream(); OutputStream ghostrydeEncoder = Ghostryde.encoder(out, rdeStagingEncryptionKey.get(), lenOut); - InputStream in = Files.newInputStream(input)) { + InputStream in = Files.asByteSource(input.toFile()).openBufferedStream()) { ByteStreams.copy(in, ghostrydeEncoder); } } - private void runDecrypt() throws IOException, PGPException { - try (InputStream in = Files.newInputStream(input); + private void runDecrypt() throws IOException { + try (InputStream in = Files.asByteSource(input.toFile()).openBufferedStream(); InputStream ghostDecoder = Ghostryde.decoder(in, rdeStagingDecryptionKey.get())) { - System.err.println("output = " + output); - if (output.toString().equals("/dev/stdout")) { - System.err.println("doing copy"); - IOUtils.copy(ghostDecoder, System.out); + if (output == null) { + ByteStreams.copy(ghostDecoder, System.out); } else { Path outFile = - Files.isDirectory(output) ? output.resolve(input.getFileName() + ".decrypt") : output; - Files.copy(ghostDecoder, outFile, REPLACE_EXISTING); + output.toFile().isDirectory() + ? output.resolve(input.getFileName() + ".decrypt") + : output; + Files.asByteSink(outFile.toFile()).writeFrom(ghostDecoder); } } } diff --git a/javatests/google/registry/tools/GhostrydeCommandTest.java b/javatests/google/registry/tools/GhostrydeCommandTest.java index 72464a082..65c54a501 100644 --- a/javatests/google/registry/tools/GhostrydeCommandTest.java +++ b/javatests/google/registry/tools/GhostrydeCommandTest.java @@ -15,6 +15,7 @@ package google.registry.tools; import static com.google.common.truth.Truth.assertThat; +import static google.registry.testing.JUnitBackports.assertThrows; import static java.nio.charset.StandardCharsets.UTF_8; import google.registry.keyring.api.Keyring; @@ -26,7 +27,6 @@ import java.io.ByteArrayOutputStream; import java.io.PrintStream; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -76,10 +76,35 @@ public class GhostrydeCommandTest extends CommandTestCase { System.setOut(orgStdout); } + @Test + public void testParameters_cantSpecifyBothEncryptAndDecrypt() { + IllegalArgumentException thrown = + assertThrows(IllegalArgumentException.class, () -> runCommand("--encrypt", "--decrypt")); + assertThat(thrown).hasMessageThat().isEqualTo("Please specify either --encrypt or --decrypt"); + } + + @Test + public void testParameters_mustSpecifyOneOfEncryptOrDecrypt() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> runCommand("--input=" + tmpDir.newFile(), "--output=" + tmpDir.newFile())); + assertThat(thrown).hasMessageThat().isEqualTo("Please specify either --encrypt or --decrypt"); + } + + @Test + public void testEncrypt_outputPathIsRequired() { + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> runCommand("--encrypt", "--input=" + tmpDir.newFile())); + assertThat(thrown).hasMessageThat().isEqualTo("--output path is required in --encrypt mode"); + } + @Test public void testEncrypt_outputIsAFile_writesToFile() throws Exception { - Path inFile = Paths.get(tmpDir.newFile("atrain.txt").toString()); - Path outFile = Paths.get(tmpDir.newFile().toString()); + Path inFile = tmpDir.newFile("atrain.txt").toPath(); + Path outFile = tmpDir.newFile().toPath(); Files.write(inFile, SONG_BY_CHRISTINA_ROSSETTI); runCommand("--encrypt", "--input=" + inFile, "--output=" + outFile); byte[] decoded = @@ -89,8 +114,8 @@ public class GhostrydeCommandTest extends CommandTestCase { @Test public void testEncrypt_outputIsADirectory_appendsGhostrydeExtension() throws Exception { - Path inFile = Paths.get(tmpDir.newFile("atrain.txt").toString()); - Path outDir = Paths.get(tmpDir.newFolder().toString()); + Path inFile = tmpDir.newFile("atrain.txt").toPath(); + Path outDir = tmpDir.newFolder().toPath(); Files.write(inFile, SONG_BY_CHRISTINA_ROSSETTI); runCommand("--encrypt", "--input=" + inFile, "--output=" + outDir); Path lenOutFile = outDir.resolve("atrain.txt.length"); @@ -104,8 +129,8 @@ public class GhostrydeCommandTest extends CommandTestCase { @Test public void testDecrypt_outputIsAFile_writesToFile() throws Exception { - Path inFile = Paths.get(tmpDir.newFile().toString()); - Path outFile = Paths.get(tmpDir.newFile().toString()); + Path inFile = tmpDir.newFile().toPath(); + Path outFile = tmpDir.newFile().toPath(); Files.write( inFile, Ghostryde.encode(SONG_BY_CHRISTINA_ROSSETTI, keyring.getRdeStagingEncryptionKey())); runCommand("--decrypt", "--input=" + inFile, "--output=" + outFile); @@ -114,8 +139,8 @@ public class GhostrydeCommandTest extends CommandTestCase { @Test public void testDecrypt_outputIsADirectory_AppendsDecryptExtension() throws Exception { - Path inFile = Paths.get(tmpDir.newFolder().toString()).resolve("atrain.ghostryde"); - Path outDir = Paths.get(tmpDir.newFolder().toString()); + Path inFile = tmpDir.newFolder().toPath().resolve("atrain.ghostryde"); + Path outDir = tmpDir.newFolder().toPath(); Files.write( inFile, Ghostryde.encode(SONG_BY_CHRISTINA_ROSSETTI, keyring.getRdeStagingEncryptionKey())); runCommand("--decrypt", "--input=" + inFile, "--output=" + outDir); @@ -125,7 +150,7 @@ public class GhostrydeCommandTest extends CommandTestCase { @Test public void testDecrypt_outputIsStdOut() throws Exception { - Path inFile = Paths.get(tmpDir.newFolder().toString()).resolve("atrain.ghostryde"); + Path inFile = tmpDir.newFolder().toPath().resolve("atrain.ghostryde"); Files.write( inFile, Ghostryde.encode(SONG_BY_CHRISTINA_ROSSETTI, keyring.getRdeStagingEncryptionKey())); ByteArrayOutputStream out = new ByteArrayOutputStream();