From 44ae6d99d0411cb52d592a7bdb8bd1d2065762cf Mon Sep 17 00:00:00 2001 From: Daniel Gredler Date: Fri, 24 Aug 2018 11:01:04 -0400 Subject: [PATCH 1/2] Use try-with-resources: in some cases, resources were not always being released; in others, behavior was correct and we are just simplifying code --- .../threerings/getdown/data/Application.java | 59 ++--- .../com/threerings/getdown/data/Digest.java | 11 +- .../com/threerings/getdown/data/Resource.java | 20 +- .../threerings/getdown/launcher/Getdown.java | 6 +- .../getdown/launcher/GetdownApp.java | 4 +- .../getdown/launcher/GetdownApplet.java | 11 +- .../getdown/launcher/GetdownAppletConfig.java | 5 +- .../getdown/net/HTTPDownloader.java | 15 +- .../com/threerings/getdown/tools/Differ.java | 48 ++-- .../threerings/getdown/tools/Digester.java | 16 +- .../com/threerings/getdown/tools/JarDiff.java | 214 ++++++++---------- .../getdown/tools/JarDiffPatcher.java | 38 +--- .../com/threerings/getdown/tools/Patcher.java | 77 +++---- .../com/threerings/getdown/util/FileUtil.java | 124 ++++------ .../threerings/getdown/util/LaunchUtil.java | 8 +- .../threerings/getdown/util/VersionUtil.java | 23 +- 16 files changed, 273 insertions(+), 406 deletions(-) diff --git a/src/main/java/com/threerings/getdown/data/Application.java b/src/main/java/com/threerings/getdown/data/Application.java index 4af9846..e110e9e 100644 --- a/src/main/java/com/threerings/getdown/data/Application.java +++ b/src/main/java/com/threerings/getdown/data/Application.java @@ -1241,27 +1241,23 @@ public class Application } if (_latest != null) { - InputStream in = null; - PrintStream out = null; - try { - in = ConnectionUtil.open(_latest, 0, 0).getInputStream(); + try (InputStream in = ConnectionUtil.open(_latest, 0, 0).getInputStream()) { BufferedReader bin = new BufferedReader(new InputStreamReader(in)); for (String[] pair : Config.parsePairs(bin, Config.createOpts(false))) { if (pair[0].equals("version")) { _targetVersion = Math.max(Long.parseLong(pair[1]), _targetVersion); if (fileVersion != -1 && _targetVersion > fileVersion) { // replace the file with the newest version - out = new PrintStream(new FileOutputStream(vfile)); - out.println(_targetVersion); + try (FileOutputStream fos = new FileOutputStream(vfile); + PrintStream out = new PrintStream(fos)) { + out.println(_targetVersion); + } } break; } } } catch (Exception e) { log.warning("Unable to retrieve version from latest config file.", e); - } finally { - StreamUtil.close(in); - StreamUtil.close(out); } } } @@ -1567,21 +1563,16 @@ public class Application } else { File signatureFile = downloadFile(path + SIGNATURE_SUFFIX); byte[] signature = null; - FileReader reader = null; - try { - reader = new FileReader(signatureFile); + try (FileReader reader = new FileReader(signatureFile)) { signature = StreamUtil.toByteArray(new FileInputStream(signatureFile)); } finally { - StreamUtil.close(reader); FileUtil.deleteHarder(signatureFile); // delete the file regardless } byte[] buffer = new byte[8192]; int length, validated = 0; for (Certificate cert : _signers) { - FileInputStream dataInput = null; - try { - dataInput = new FileInputStream(target); + try (FileInputStream dataInput = new FileInputStream(target)) { Signature sig = Signature.getInstance(Digest.sigAlgorithm(sigVersion)); sig.initVerify(cert); while ((length = dataInput.read(buffer)) != -1) { @@ -1602,9 +1593,6 @@ public class Application } catch (GeneralSecurityException gse) { // no problem! - } finally { - StreamUtil.close(dataInput); - dataInput = null; } } @@ -1645,27 +1633,22 @@ public class Application log.info("Attempting to refetch '" + path + "' from '" + targetURL + "'."); // stream the URL into our temporary file - InputStream fin = null; - FileOutputStream fout = null; - try { - URLConnection uconn = ConnectionUtil.open(targetURL, 0, 0); - // we have to tell Java not to use caches here, otherwise it will cache any request for - // same URL for the lifetime of this JVM (based on the URL string, not the URL object); - // if the getdown.txt file, for example, changes in the meanwhile, we would never hear - // about it; turning off caches is not a performance concern, because when Getdown asks - // to download a file, it expects it to come over the wire, not from a cache - uconn.setUseCaches(false); - uconn.setRequestProperty("Accept-Encoding", "gzip"); - fin = uconn.getInputStream(); + URLConnection uconn = ConnectionUtil.open(targetURL, 0, 0); + // we have to tell Java not to use caches here, otherwise it will cache any request for + // same URL for the lifetime of this JVM (based on the URL string, not the URL object); + // if the getdown.txt file, for example, changes in the meanwhile, we would never hear + // about it; turning off caches is not a performance concern, because when Getdown asks + // to download a file, it expects it to come over the wire, not from a cache + uconn.setUseCaches(false); + uconn.setRequestProperty("Accept-Encoding", "gzip"); + try (InputStream fin = uconn.getInputStream()) { String encoding = uconn.getContentEncoding(); - if ("gzip".equalsIgnoreCase(encoding)) { - fin = new GZIPInputStream(fin); + boolean gzip = "gzip".equalsIgnoreCase(encoding); + try (InputStream fin2 = (gzip ? new GZIPInputStream(fin) : fin)) { + try (FileOutputStream fout = new FileOutputStream(target)) { + StreamUtil.copy(fin2, fout); + } } - fout = new FileOutputStream(target); - StreamUtil.copy(fin, fout); - } finally { - StreamUtil.close(fin); - StreamUtil.close(fout); } return target; diff --git a/src/main/java/com/threerings/getdown/data/Digest.java b/src/main/java/com/threerings/getdown/data/Digest.java index 32eb1fa..9aff446 100644 --- a/src/main/java/com/threerings/getdown/data/Digest.java +++ b/src/main/java/com/threerings/getdown/data/Digest.java @@ -6,12 +6,12 @@ package com.threerings.getdown.data; import java.io.*; +import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.*; import java.util.concurrent.*; -import com.samskivert.io.StreamUtil; import com.samskivert.text.MessageUtil; import com.samskivert.util.StringUtil; @@ -99,9 +99,9 @@ public class Digest } StringBuilder data = new StringBuilder(); - PrintWriter pout = null; - try { - pout = new PrintWriter(new OutputStreamWriter(new FileOutputStream(output), "UTF-8")); + try (FileOutputStream fos = new FileOutputStream(output); + OutputStreamWriter osw = new OutputStreamWriter(fos, StandardCharsets.UTF_8); + PrintWriter pout = new PrintWriter(osw)) { // compute and append the digest of each resource in the list for (Resource rsrc : resources) { String path = rsrc.getPath(); @@ -114,9 +114,6 @@ public class Digest byte[] contents = data.toString().getBytes("UTF-8"); String filename = digestFile(version); pout.println(filename + " = " + StringUtil.hexlate(md.digest(contents))); - - } finally { - StreamUtil.close(pout); } long elapsed = System.currentTimeMillis() - start; diff --git a/src/main/java/com/threerings/getdown/data/Resource.java b/src/main/java/com/threerings/getdown/data/Resource.java index 262dd09..0bacc33 100644 --- a/src/main/java/com/threerings/getdown/data/Resource.java +++ b/src/main/java/com/threerings/getdown/data/Resource.java @@ -15,7 +15,6 @@ import java.util.List; import java.util.jar.JarEntry; import java.util.jar.JarFile; -import com.samskivert.io.StreamUtil; import com.samskivert.util.StringUtil; import com.threerings.getdown.util.FileUtil; @@ -83,15 +82,12 @@ public class Resource implements Comparable } } - InputStream in = null; - try { - in = jar.getInputStream(entry); + try (InputStream in = jar.getInputStream(entry)) { while ((read = in.read(buffer)) != -1) { md.update(buffer, 0, read); } - } finally { - StreamUtil.close(in); } + updateProgress(obs, eidx, entries.size()); } @@ -110,16 +106,12 @@ public class Resource implements Comparable } else { long totalSize = target.length(), position = 0L; - FileInputStream fin = null; - try { - fin = new FileInputStream(target); + try (FileInputStream fin = new FileInputStream(target)) { while ((read = fin.read(buffer)) != -1) { md.update(buffer, 0, read); position += read; updateProgress(obs, position, totalSize); } - } finally { - StreamUtil.close(fin); } } return StringUtil.hexlate(md.digest()); @@ -282,8 +274,10 @@ public class Resource implements Comparable throw new IOException("Requested to unpack non-jar file '" + _local + "'."); } if (_isJar) { - FileUtil.unpackJar(new JarFile(_local), _unpacked); - } else{ + try (JarFile jar = new JarFile(_local)) { + FileUtil.unpackJar(jar, _unpacked); + } + } else { FileUtil.unpackPacked200Jar(_local, _unpacked); } } diff --git a/src/main/java/com/threerings/getdown/launcher/Getdown.java b/src/main/java/com/threerings/getdown/launcher/Getdown.java index 8084861..0b3d058 100644 --- a/src/main/java/com/threerings/getdown/launcher/Getdown.java +++ b/src/main/java/com/threerings/getdown/launcher/Getdown.java @@ -229,13 +229,11 @@ public abstract class Getdown extends Thread // if we're provided with valid values, create a proxy.txt file if (!StringUtil.isBlank(host)) { File pfile = _app.getLocalPath("proxy.txt"); - try { - PrintStream pout = new PrintStream(new FileOutputStream(pfile)); + try (PrintStream pout = new PrintStream(new FileOutputStream(pfile))) { pout.println("host = " + host); if (!StringUtil.isBlank(port)) { pout.println("port = " + port); } - pout.close(); } catch (IOException ioe) { log.warning("Error creating proxy file '" + pfile + "': " + ioe); } @@ -570,6 +568,7 @@ public abstract class Getdown extends Thread } // documentation inherited from interface + @Override public void updateStatus (String message) { setStatusAsync(message, -1, -1L, true); @@ -580,6 +579,7 @@ public abstract class Getdown extends Thread * if we can find a localized version by sticking a {@code _} in front of the "." in * the filename. */ + @Override public BufferedImage loadImage (String path) { if (StringUtil.isBlank(path)) { diff --git a/src/main/java/com/threerings/getdown/launcher/GetdownApp.java b/src/main/java/com/threerings/getdown/launcher/GetdownApp.java index 2e5a554..4de0770 100644 --- a/src/main/java/com/threerings/getdown/launcher/GetdownApp.java +++ b/src/main/java/com/threerings/getdown/launcher/GetdownApp.java @@ -92,11 +92,9 @@ public class GetdownApp File crtFile = new File(appDir, Digest.digestFile(Digest.VERSION) + ".crt"); List crts = new ArrayList<>(); if (crtFile.exists()) { - try { - FileInputStream fis = new FileInputStream(crtFile); + try (FileInputStream fis = new FileInputStream(crtFile)) { X509Certificate certificate = (X509Certificate) CertificateFactory.getInstance("X.509").generateCertificate(fis); - fis.close(); crts.add(certificate); } catch (Exception e) { log.warning("Certificate error: " + e.getMessage()); diff --git a/src/main/java/com/threerings/getdown/launcher/GetdownApplet.java b/src/main/java/com/threerings/getdown/launcher/GetdownApplet.java index 8df73f3..68b46e3 100644 --- a/src/main/java/com/threerings/getdown/launcher/GetdownApplet.java +++ b/src/main/java/com/threerings/getdown/launcher/GetdownApplet.java @@ -233,6 +233,7 @@ public class GetdownApplet extends JApplet } // implemented from ImageLoader + @Override public Image loadImage (String path) { try { @@ -293,10 +294,9 @@ public class GetdownApplet extends JApplet */ protected boolean writeToFile (File tofile, String contents) { - try { - PrintStream out = new PrintStream(new FileOutputStream(tofile)); + try (FileOutputStream fos = new FileOutputStream(tofile); + PrintStream out = new PrintStream(fos)) { out.println(contents); - out.close(); return true; } catch (IOException ioe) { log.warning("Failed to create '" + tofile + "'.", ioe); @@ -311,11 +311,8 @@ public class GetdownApplet extends JApplet if (keyUrl == null) { return null; } - InputStream is = keyUrl.openStream(); - try { + try (InputStream is = keyUrl.openStream()) { return CertificateFactory.getInstance("X.509").generateCertificate(is); - } finally { - is.close(); } } catch (CertificateException e) { throw new RuntimeException(e); diff --git a/src/main/java/com/threerings/getdown/launcher/GetdownAppletConfig.java b/src/main/java/com/threerings/getdown/launcher/GetdownAppletConfig.java index d6544e8..449efab 100644 --- a/src/main/java/com/threerings/getdown/launcher/GetdownAppletConfig.java +++ b/src/main/java/com/threerings/getdown/launcher/GetdownAppletConfig.java @@ -394,10 +394,9 @@ public class GetdownAppletConfig */ protected boolean writeToFile (File tofile, String contents) { - try { - PrintStream out = new PrintStream(new FileOutputStream(tofile)); + try (FileOutputStream fos = new FileOutputStream(tofile); + PrintStream out = new PrintStream(fos)) { out.println(contents); - out.close(); return true; } catch (IOException ioe) { log.warning("Failed to create '" + tofile + "'.", ioe); diff --git a/src/main/java/com/threerings/getdown/net/HTTPDownloader.java b/src/main/java/com/threerings/getdown/net/HTTPDownloader.java index 6317016..720dd85 100644 --- a/src/main/java/com/threerings/getdown/net/HTTPDownloader.java +++ b/src/main/java/com/threerings/getdown/net/HTTPDownloader.java @@ -12,10 +12,7 @@ import java.net.HttpURLConnection; import java.net.URLConnection; import java.util.Collection; -import com.samskivert.io.StreamUtil; - import com.threerings.getdown.data.Resource; -import com.threerings.getdown.data.SysProps; import com.threerings.getdown.util.ConnectionUtil; import static com.threerings.getdown.Log.log; @@ -74,19 +71,16 @@ public class HTTPDownloader extends Downloader long actualSize = conn.getContentLength(); log.info("Downloading resource", "url", rsrc.getRemote(), "size", actualSize); - InputStream in = null; - FileOutputStream out = null; long currentSize = 0L; - try { - in = conn.getInputStream(); - out = new FileOutputStream(rsrc.getLocalNew()); - int read; + try (InputStream in = conn.getInputStream(); + FileOutputStream out = new FileOutputStream(rsrc.getLocalNew())) { // TODO: look to see if we have a download info file // containing info on potentially partially downloaded data; // if so, use a "Range: bytes=HAVE-" header. // read in the file data + int read; while ((read = in.read(_buffer)) != -1) { // write it out to our local copy out.write(_buffer, 0, read); @@ -100,9 +94,6 @@ public class HTTPDownloader extends Downloader currentSize += read; updateObserver(rsrc, currentSize, actualSize); } - } finally { - StreamUtil.close(in); - StreamUtil.close(out); } } } diff --git a/src/main/java/com/threerings/getdown/tools/Differ.java b/src/main/java/com/threerings/getdown/tools/Differ.java index 74a8dc2..8f1cede 100644 --- a/src/main/java/com/threerings/getdown/tools/Differ.java +++ b/src/main/java/com/threerings/getdown/tools/Differ.java @@ -96,10 +96,9 @@ public class Differ { int version = Digest.VERSION; MessageDigest md = Digest.getMessageDigest(version); - JarOutputStream jout = null; - try { - jout = new JarOutputStream( - new BufferedOutputStream(new FileOutputStream(patch))); + try (FileOutputStream fos = new FileOutputStream(patch); + BufferedOutputStream buffered = new BufferedOutputStream(fos); + JarOutputStream jout = new JarOutputStream(buffered)) { // for each file in the new application, it either already exists // in the old application, or it is new @@ -160,11 +159,9 @@ public class Differ jout.putNextEntry(new ZipEntry(rsrc.getPath() + Patcher.DELETE)); } - StreamUtil.close(jout); System.out.println("Created patch file: " + patch); } catch (IOException ioe) { - StreamUtil.close(jout); patch.delete(); throw ioe; } @@ -173,25 +170,25 @@ public class Differ protected File rebuildJar (File target) throws IOException { - JarFile jar = new JarFile(target); File temp = File.createTempFile("differ", "jar"); - JarOutputStream jout = new JarOutputStream( - new BufferedOutputStream(new FileOutputStream(temp))); - byte[] buffer = new byte[4096]; - for (Enumeration iter = jar.entries(); iter.hasMoreElements(); ) { - JarEntry entry = iter.nextElement(); - entry.setCompressedSize(-1); - jout.putNextEntry(entry); - InputStream in = jar.getInputStream(entry); - int size = in.read(buffer); - while (size != -1) { - jout.write(buffer, 0, size); - size = in.read(buffer); + try (JarFile jar = new JarFile(target); + FileOutputStream tempFos = new FileOutputStream(temp); + BufferedOutputStream tempBos = new BufferedOutputStream(tempFos); + JarOutputStream jout = new JarOutputStream(tempBos)) { + byte[] buffer = new byte[4096]; + for (Enumeration< JarEntry > iter = jar.entries(); iter.hasMoreElements();) { + JarEntry entry = iter.nextElement(); + entry.setCompressedSize(-1); + jout.putNextEntry(entry); + try (InputStream in = jar.getInputStream(entry)) { + int size = in.read(buffer); + while (size != -1) { + jout.write(buffer, 0, size); + size = in.read(buffer); + } + } } - in.close(); } - jout.close(); - jar.close(); return temp; } @@ -227,11 +224,8 @@ public class Differ protected static void pipe (File file, JarOutputStream jout) throws IOException { - FileInputStream fin = null; - try { - StreamUtil.copy(fin = new FileInputStream(file), jout); - } finally { - StreamUtil.close(fin); + try (FileInputStream fin = new FileInputStream(file)) { + StreamUtil.copy(fin, jout); } } } diff --git a/src/main/java/com/threerings/getdown/tools/Digester.java b/src/main/java/com/threerings/getdown/tools/Digester.java index 90446b7..9904bd8 100644 --- a/src/main/java/com/threerings/getdown/tools/Digester.java +++ b/src/main/java/com/threerings/getdown/tools/Digester.java @@ -18,7 +18,6 @@ import java.security.Signature; import java.util.ArrayList; import java.util.List; -import com.samskivert.io.StreamUtil; import com.threerings.getdown.data.Application; import com.threerings.getdown.data.Digest; import com.threerings.getdown.data.Resource; @@ -99,19 +98,18 @@ public class Digester File inputFile = new File(appdir, filename); File signatureFile = new File(appdir, filename + Application.SIGNATURE_SUFFIX); - FileInputStream storeInput = null, dataInput = null; - FileOutputStream signatureOutput = null; - try { + try (FileInputStream storeInput = new FileInputStream(storePath); + FileInputStream dataInput = new FileInputStream(inputFile); + FileOutputStream signatureOutput = new FileOutputStream(signatureFile)) { + // initialize the keystore KeyStore store = KeyStore.getInstance("JKS"); - storeInput = new FileInputStream(storePath); store.load(storeInput, storePass.toCharArray()); PrivateKey key = (PrivateKey)store.getKey(storeAlias, storePass.toCharArray()); // sign the digest file String algo = Digest.sigAlgorithm(version); Signature sig = Signature.getInstance(algo); - dataInput = new FileInputStream(inputFile); byte[] buffer = new byte[8192]; int length; @@ -121,14 +119,8 @@ public class Digester } // Write out the signature - signatureOutput = new FileOutputStream(signatureFile); String signed = Base64.encodeToString(sig.sign(), Base64.DEFAULT); signatureOutput.write(signed.getBytes("utf8")); - - } finally { - StreamUtil.close(signatureOutput); - StreamUtil.close(dataInput); - StreamUtil.close(storeInput); } } } diff --git a/src/main/java/com/threerings/getdown/tools/JarDiff.java b/src/main/java/com/threerings/getdown/tools/JarDiff.java index ccc8d39..e921f40 100644 --- a/src/main/java/com/threerings/getdown/tools/JarDiff.java +++ b/src/main/java/com/threerings/getdown/tools/JarDiff.java @@ -41,9 +41,25 @@ package com.threerings.getdown.tools; -import java.io.*; -import java.util.*; -import java.util.jar.*; +import java.io.Closeable; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.StringWriter; +import java.io.Writer; +import java.util.ArrayList; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.ListIterator; +import java.util.Map; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import java.util.jar.JarOutputStream; /** * JarDiff is able to create a jar file containing the delta between two jar files (old and new). @@ -69,10 +85,9 @@ public class JarDiff implements JarDiffCodes public static void createPatch (String oldPath, String newPath, OutputStream os, boolean minimal) throws IOException { - JarFile2 oldJar = new JarFile2(oldPath); - JarFile2 newJar = new JarFile2(newPath); + try (JarFile2 oldJar = new JarFile2(oldPath); + JarFile2 newJar = new JarFile2(newPath)) { - try { HashMap moved = new HashMap<>(); HashSet implicit = new HashSet<>(); HashSet moveSrc = new HashSet<>(); @@ -194,21 +209,7 @@ public class JarDiff implements JarDiffCodes jos.finish(); // jos.close(); - - } catch (IOException ioE){ - throw ioE; - } finally { - try { - oldJar.getJarFile().close(); - } catch (IOException e1) { - //ignore - } - try { - newJar.getJarFile().close(); - } catch (IOException e1) { - //ignore - } - } // finally + } } /** @@ -220,36 +221,36 @@ public class JarDiff implements JarDiffCodes Map movedMap) throws IOException { - StringWriter writer = new StringWriter(); + try (StringWriter writer = new StringWriter()) { - writer.write(VERSION_HEADER); - writer.write("\r\n"); - - // Write out entries that have been removed - for (String name : oldEntries) { - writer.write(REMOVE_COMMAND); - writer.write(" "); - writeEscapedString(writer, name); + writer.write(VERSION_HEADER); writer.write("\r\n"); + + // Write out entries that have been removed + for (String name : oldEntries) { + writer.write(REMOVE_COMMAND); + writer.write(" "); + writeEscapedString(writer, name); + writer.write("\r\n"); + } + + // And those that have moved + for (String newName : movedMap.keySet()) { + String oldName = movedMap.get(newName); + writer.write(MOVE_COMMAND); + writer.write(" "); + writeEscapedString(writer, oldName); + writer.write(" "); + writeEscapedString(writer, newName); + writer.write("\r\n"); + } + + JarEntry je = new JarEntry(INDEX_NAME); + byte[] bytes = writer.toString().getBytes("UTF-8"); + + jos.putNextEntry(je); + jos.write(bytes, 0, bytes.length); } - - // And those that have moved - for (String newName : movedMap.keySet()) { - String oldName = movedMap.get(newName); - writer.write(MOVE_COMMAND); - writer.write(" "); - writeEscapedString(writer, oldName); - writer.write(" "); - writeEscapedString(writer, newName); - writer.write("\r\n"); - } - - JarEntry je = new JarEntry(INDEX_NAME); - byte[] bytes = writer.toString().getBytes("UTF-8"); - - writer.close(); - jos.putNextEntry(je); - jos.write(bytes, 0, bytes.length); } private static void writeEscapedString (Writer writer, String string) @@ -282,7 +283,9 @@ public class JarDiff implements JarDiffCodes private static void writeEntry (JarOutputStream jos, JarEntry entry, JarFile2 file) throws IOException { - writeEntry(jos, entry, file.getJarFile().getInputStream(entry)); + try (InputStream data = file.getJarFile().getInputStream(entry)) { + writeEntry(jos, entry, data); + } } private static void writeEntry (JarOutputStream jos, JarEntry entry, InputStream data) @@ -290,30 +293,19 @@ public class JarDiff implements JarDiffCodes { jos.putNextEntry(entry); - try { - // Read the entry - int size = data.read(newBytes); - - while (size != -1) { - jos.write(newBytes, 0, size); - size = data.read(newBytes); - } - } catch(IOException ioE) { - throw ioE; - } finally { - try { - data.close(); - } catch(IOException e){ - //Ignore - } + // Read the entry + int size = data.read(newBytes); + while (size != -1) { + jos.write(newBytes, 0, size); + size = data.read(newBytes); } } /** * JarFile2 wraps a JarFile providing some convenience methods. */ - private static class JarFile2 implements Iterable + private static class JarFile2 implements Iterable, Closeable { private JarFile _jar; private List _entries; @@ -330,6 +322,7 @@ public class JarDiff implements JarDiffCodes } // from interface Iterable + @Override public Iterator iterator () { return _entries.iterator(); } @@ -347,52 +340,38 @@ public class JarDiff implements JarDiffCodes int total = 0; boolean retVal = false; - try{ - while (newSize != -1) { - newSize = newIS.read(newBytes); - oldSize = oldIS.read(oldBytes); + while (newSize != -1) { + newSize = newIS.read(newBytes); + oldSize = oldIS.read(oldBytes); - if (newSize != oldSize) { - if (_debug) { - System.out.println("\tread sizes differ: " + newSize + - " " + oldSize + " total " + total); - } - retVal = true; - break; + if (newSize != oldSize) { + if (_debug) { + System.out.println("\tread sizes differ: " + newSize + + " " + oldSize + " total " + total); } - if (newSize > 0) { - while (--newSize >= 0) { - total++; - if (newBytes[newSize] != oldBytes[newSize]) { - if (_debug) { - System.out.println("\tbytes differ at " + - total); - } - retVal = true; - break; + retVal = true; + break; + } + if (newSize > 0) { + while (--newSize >= 0) { + total++; + if (newBytes[newSize] != oldBytes[newSize]) { + if (_debug) { + System.out.println("\tbytes differ at " + + total); } - if ( retVal ) { - //Jump out - break; - } - newSize = 0; + retVal = true; + break; } + if ( retVal ) { + //Jump out + break; + } + newSize = 0; } } - } catch(IOException ioE){ - throw ioE; - } finally { - try { - oldIS.close(); - } catch(IOException e){ - //Ignore - } - try { - newIS.close(); - } catch(IOException e){ - //Ignore - } } + return retVal; } @@ -419,11 +398,10 @@ public class JarDiff implements JarDiffCodes return false; // Check contents - if no match - return false - InputStream oldIS = getJarFile().getInputStream(thisEntry); - InputStream newIS = f.getJarFile().getInputStream(e); - boolean retValue = differs(oldIS, newIS); - - return !retValue; + try (InputStream oldIS = getJarFile().getInputStream(thisEntry); + InputStream newIS = f.getJarFile().getInputStream(e)) { + return !differs(oldIS, newIS); + } } public String hasSameContent (JarFile2 file, JarEntry entry) throws IOException { @@ -438,11 +416,12 @@ public class JarDiff implements JarDiffCodes while (li.hasNext()) { JarEntry thisEntry = li.next(); // check for content match - InputStream oldIS = getJarFile().getInputStream(thisEntry); - InputStream newIS = file.getJarFile().getInputStream(entry); - if (!differs(oldIS, newIS)) { - thisName = thisEntry.getName(); - return thisName; + try (InputStream oldIS = getJarFile().getInputStream(thisEntry); + InputStream newIS = file.getJarFile().getInputStream(entry)) { + if (!differs(oldIS, newIS)) { + thisName = thisEntry.getName(); + return thisName; + } } } } @@ -486,5 +465,10 @@ public class JarDiff implements JarDiffCodes } } } + + @Override + public void close() throws IOException { + _jar.close(); + } } } diff --git a/src/main/java/com/threerings/getdown/tools/JarDiffPatcher.java b/src/main/java/com/threerings/getdown/tools/JarDiffPatcher.java index ff96cb6..9d8bc19 100644 --- a/src/main/java/com/threerings/getdown/tools/JarDiffPatcher.java +++ b/src/main/java/com/threerings/getdown/tools/JarDiffPatcher.java @@ -25,7 +25,6 @@ import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.jar.JarOutputStream; -import com.samskivert.io.StreamUtil; import com.threerings.getdown.util.ProgressObserver; /** @@ -48,14 +47,13 @@ public class JarDiffPatcher implements JarDiffCodes throws IOException { File oldFile = new File(jarPath), diffFile = new File(diffPath); - JarOutputStream jos = null; - JarFile oldJar = null, jarDiff = null; - try { - jos = new JarOutputStream(new FileOutputStream(target)); - oldJar = new JarFile(oldFile); - jarDiff = new JarFile(diffFile); - Set ignoreSet = new HashSet<>(); + try (JarFile oldJar = new JarFile(oldFile); + JarFile jarDiff = new JarFile(diffFile); + FileOutputStream fos = new FileOutputStream(target); + JarOutputStream jos = new JarOutputStream(fos)) { + + Set ignoreSet = new HashSet<>(); Map renameMap = new HashMap<>(); determineNameMapping(jarDiff, ignoreSet, renameMap); @@ -135,7 +133,9 @@ public class JarDiffPatcher implements JarDiffCodes updateObserver(observer, currentEntry, size); currentEntry++; - writeEntry(jos, newEntry, oldJar.getInputStream(oldEntry)); + try (InputStream data = oldJar.getInputStream(oldEntry)) { + writeEntry(jos, newEntry, data); + } // Remove entry from oldjarNames since no implicit move is needed boolean wasInOld = oldjarNames.remove(oldName); @@ -159,11 +159,6 @@ public class JarDiffPatcher implements JarDiffCodes } } updateObserver(observer, currentEntry, size); - - } finally { - StreamUtil.close(jos); - closeFile(oldJar); - closeFile(jarDiff); } } @@ -268,7 +263,9 @@ public class JarDiffPatcher implements JarDiffCodes JarOutputStream jos, JarEntry entry, JarFile file) throws IOException { - writeEntry(jos, entry, file.getInputStream(entry)); + try (InputStream data = file.getInputStream(entry)) { + writeEntry(jos, entry, data); + } } protected void writeEntry ( @@ -283,17 +280,6 @@ public class JarDiffPatcher implements JarDiffCodes jos.write(newBytes, 0, size); size = data.read(newBytes); } - data.close(); - } - - private static void closeFile (JarFile jar) { - if (jar != null) { - try { - jar.close(); - } catch (IOException e) { - e.printStackTrace(System.err); - } - } } protected static final int DEFAULT_READ_SIZE = 2048; diff --git a/src/main/java/com/threerings/getdown/tools/Patcher.java b/src/main/java/com/threerings/getdown/tools/Patcher.java index 6641c1d..3445640 100644 --- a/src/main/java/com/threerings/getdown/tools/Patcher.java +++ b/src/main/java/com/threerings/getdown/tools/Patcher.java @@ -56,40 +56,40 @@ public class Patcher _obs = obs; _plength = patch.length(); - JarFile file = new JarFile(patch); - Enumeration entries = file.entries(); // old skool! - while (entries.hasMoreElements()) { - JarEntry entry = entries.nextElement(); - String path = entry.getName(); - long elength = entry.getCompressedSize(); + try (JarFile file = new JarFile(patch)) { + Enumeration entries = file.entries(); // old skool! + while (entries.hasMoreElements()) { + JarEntry entry = entries.nextElement(); + String path = entry.getName(); + long elength = entry.getCompressedSize(); - // depending on the suffix, we do The Right Thing (tm) - if (path.endsWith(CREATE)) { - path = strip(path, CREATE); - System.out.println("Creating " + path + "..."); - createFile(file, entry, new File(appdir, path)); + // depending on the suffix, we do The Right Thing (tm) + if (path.endsWith(CREATE)) { + path = strip(path, CREATE); + System.out.println("Creating " + path + "..."); + createFile(file, entry, new File(appdir, path)); - } else if (path.endsWith(PATCH)) { - path = strip(path, PATCH); - System.out.println("Patching " + path + "..."); - patchFile(file, entry, appdir, path); + } else if (path.endsWith(PATCH)) { + path = strip(path, PATCH); + System.out.println("Patching " + path + "..."); + patchFile(file, entry, appdir, path); - } else if (path.endsWith(DELETE)) { - path = strip(path, DELETE); - System.out.println("Removing " + path + "..."); - File target = new File(appdir, path); - if (!target.delete()) { - System.err.println("Failure deleting '" + target + "'."); + } else if (path.endsWith(DELETE)) { + path = strip(path, DELETE); + System.out.println("Removing " + path + "..."); + File target = new File(appdir, path); + if (!target.delete()) { + System.err.println("Failure deleting '" + target + "'."); + } + + } else { + System.err.println("Skipping bogus patch file entry: " + path); } - } else { - System.err.println("Skipping bogus patch file entry: " + path); + // note that we've completed this entry + _complete += elength; } - - // note that we've completed this entry - _complete += elength; } - file.close(); } protected String strip (String path, String suffix) @@ -110,11 +110,9 @@ public class Patcher log.warning("Failed to create parent for '" + target + "'."); } - InputStream in = null; - FileOutputStream fout = null; - try { - in = file.getInputStream(entry); - fout = new FileOutputStream(target); + try (InputStream in = file.getInputStream(entry); + FileOutputStream fout = new FileOutputStream(target)) { + int total = 0, read; while ((read = in.read(_buffer)) != -1) { total += read; @@ -124,10 +122,6 @@ public class Patcher } catch (IOException ioe) { System.err.println("Error creating '" + target + "': " + ioe); - - } finally { - StreamUtil.close(in); - StreamUtil.close(fout); } } @@ -143,12 +137,11 @@ public class Patcher otarget.delete(); // pipe the contents of the patch into a file - InputStream in = null; - FileOutputStream fout = null; - try { - StreamUtil.copy(in = file.getInputStream(entry), fout = new FileOutputStream(patch)); + try (InputStream in = file.getInputStream(entry); + FileOutputStream fout = new FileOutputStream(patch)) { + + StreamUtil.copy(in, fout); StreamUtil.close(fout); - fout = null; // move the current version of the jar to .old if (!FileUtil.renameTo(target, otarget)) { @@ -176,8 +169,6 @@ public class Patcher } } finally { - StreamUtil.close(fout); - StreamUtil.close(in); // clean up our temporary files if (!patch.delete()) { patch.deleteOnExit(); diff --git a/src/main/java/com/threerings/getdown/util/FileUtil.java b/src/main/java/com/threerings/getdown/util/FileUtil.java index 9fdcc93..cc10189 100644 --- a/src/main/java/com/threerings/getdown/util/FileUtil.java +++ b/src/main/java/com/threerings/getdown/util/FileUtil.java @@ -5,7 +5,14 @@ package com.threerings.getdown.util; -import java.io.*; +import java.io.BufferedOutputStream; +import java.io.BufferedReader; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.Reader; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; @@ -58,15 +65,13 @@ public class FileUtil } // as a last resort, try copying the old data over the new - FileInputStream fin = null; - FileOutputStream fout = null; - try { - fin = new FileInputStream(source); - fout = new FileOutputStream(dest); + try (FileInputStream fin = new FileInputStream(source); + FileOutputStream fout = new FileOutputStream(dest)) { + StreamUtil.copy(fin, fout); + // close the input stream now so we can delete 'source' fin.close(); - fin = null; if (!deleteHarder(source)) { log.warning("Failed to delete " + source + " after brute force copy to " + dest + "."); @@ -76,10 +81,6 @@ public class FileUtil } catch (IOException ioe) { log.warning("Failed to copy " + source + " to " + dest + ": " + ioe); return false; - - } finally { - StreamUtil.close(fin); - StreamUtil.close(fout); } } @@ -103,63 +104,45 @@ public class FileUtil throws IOException { List lines = new ArrayList<>(); - try { - BufferedReader bin = new BufferedReader(in); + try (BufferedReader bin = new BufferedReader(in)) { for (String line = null; (line = bin.readLine()) != null; lines.add(line)) {} - } finally { - StreamUtil.close(in); } return lines; } /** - * Unpacks the specified jar file intto the specified target directory. + * Unpacks the specified jar file into the specified target directory. */ public static void unpackJar (JarFile jar, File target) throws IOException { - try { - Enumeration entries = jar.entries(); - while (entries.hasMoreElements()) { - JarEntry entry = (JarEntry)entries.nextElement(); - File efile = new File(target, entry.getName()); + Enumeration entries = jar.entries(); + while (entries.hasMoreElements()) { + JarEntry entry = (JarEntry)entries.nextElement(); + File efile = new File(target, entry.getName()); - // if we're unpacking a normal jar file, it will have special path - // entries that allow us to create our directories first - if (entry.isDirectory()) { - if (!efile.exists() && !efile.mkdir()) { - log.warning("Failed to create jar entry path", "jar", jar, "entry", entry); - } - continue; - } - - // but some do not, so we want to ensure that our directories exist - // prior to getting down and funky - File parent = new File(efile.getParent()); - if (!parent.exists() && !parent.mkdirs()) { - log.warning("Failed to create jar entry parent", "jar", jar, "parent", parent); - continue; - } - - BufferedOutputStream fout = null; - InputStream jin = null; - try { - fout = new BufferedOutputStream(new FileOutputStream(efile)); - jin = jar.getInputStream(entry); - StreamUtil.copy(jin, fout); - } catch (Exception e) { - throw new IOException( - Logger.format("Failure unpacking", "jar", jar, "entry", efile), e); - } finally { - StreamUtil.close(jin); - StreamUtil.close(fout); + // if we're unpacking a normal jar file, it will have special path + // entries that allow us to create our directories first + if (entry.isDirectory()) { + if (!efile.exists() && !efile.mkdir()) { + log.warning("Failed to create jar entry path", "jar", jar, "entry", entry); } + continue; } - } finally { - try { - jar.close(); + // but some do not, so we want to ensure that our directories exist + // prior to getting down and funky + File parent = new File(efile.getParent()); + if (!parent.exists() && !parent.mkdirs()) { + log.warning("Failed to create jar entry parent", "jar", jar, "parent", parent); + continue; + } + + try (BufferedOutputStream fout = new BufferedOutputStream(new FileOutputStream(efile)); + InputStream jin = jar.getInputStream(entry)) { + StreamUtil.copy(jin, fout); } catch (Exception e) { - log.warning("Failed to close jar file", "jar", jar, "error", e); + throw new IOException( + Logger.format("Failure unpacking", "jar", jar, "entry", efile), e); } } } @@ -170,23 +153,14 @@ public class FileUtil */ public static void unpackPacked200Jar (File packedJar, File target) throws IOException { - InputStream packedJarIn = null; - FileOutputStream extractedJarFileOut = null; - JarOutputStream jarOutputStream = null; - try { - extractedJarFileOut = new FileOutputStream(target); - jarOutputStream = new JarOutputStream(extractedJarFileOut); - packedJarIn = new FileInputStream(packedJar); - if (packedJar.getName().endsWith(".gz") || packedJar.getName().endsWith(".gz_new")) { - packedJarIn = new GZIPInputStream(packedJarIn); + try (InputStream packedJarIn = new FileInputStream(packedJar); + FileOutputStream extractedJarFileOut = new FileOutputStream(target); + JarOutputStream jarOutputStream = new JarOutputStream(extractedJarFileOut)) { + boolean gz = (packedJar.getName().endsWith(".gz") || packedJar.getName().endsWith(".gz_new")); + try (InputStream packedJarIn2 = (gz ? new GZIPInputStream(packedJarIn) : packedJarIn)) { + Pack200.Unpacker unpacker = Pack200.newUnpacker(); + unpacker.unpack(packedJarIn2, jarOutputStream); } - Pack200.Unpacker unpacker = Pack200.newUnpacker(); - unpacker.unpack(packedJarIn, jarOutputStream); - - } finally { - StreamUtil.close(jarOutputStream); - StreamUtil.close(extractedJarFileOut); - StreamUtil.close(packedJarIn); } } @@ -194,15 +168,9 @@ public class FileUtil * Copies the given {@code source} file to the given {@code target}. */ public static void copy (File source, File target) throws IOException { - FileInputStream in = null; - FileOutputStream out = null; - try { - in = new FileInputStream(source); - out = new FileOutputStream(target); + try (FileInputStream in = new FileInputStream(source); + FileOutputStream out = new FileOutputStream(target)) { StreamUtil.copy(in, out); - } finally { - StreamUtil.close(in); - StreamUtil.close(out); } } diff --git a/src/main/java/com/threerings/getdown/util/LaunchUtil.java b/src/main/java/com/threerings/getdown/util/LaunchUtil.java index e60abb4..1f03663 100644 --- a/src/main/java/com/threerings/getdown/util/LaunchUtil.java +++ b/src/main/java/com/threerings/getdown/util/LaunchUtil.java @@ -6,7 +6,6 @@ package com.threerings.getdown.util; import java.io.File; -import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; import java.io.PrintStream; @@ -52,9 +51,10 @@ public class LaunchUtil { // create the file that instructs Getdown to upgrade File vfile = new File(appdir, "version.txt"); - PrintStream ps = new PrintStream(new FileOutputStream(vfile)); - ps.println(newVersion); - ps.close(); + try (FileOutputStream fos = new FileOutputStream(vfile); + PrintStream ps = new PrintStream(fos)) { + ps.println(newVersion); + } // make sure that we can find our getdown.jar file and can safely launch children File pro = new File(appdir, getdownJarName); diff --git a/src/main/java/com/threerings/getdown/util/VersionUtil.java b/src/main/java/com/threerings/getdown/util/VersionUtil.java index 12f1343..45693a4 100644 --- a/src/main/java/com/threerings/getdown/util/VersionUtil.java +++ b/src/main/java/com/threerings/getdown/util/VersionUtil.java @@ -16,7 +16,6 @@ import java.io.PrintStream; import java.util.regex.Matcher; import java.util.regex.Pattern; -import com.samskivert.io.StreamUtil; import com.samskivert.util.StringUtil; import com.threerings.getdown.data.SysProps; @@ -33,17 +32,15 @@ public class VersionUtil public static long readVersion (File vfile) { long fileVersion = -1; - BufferedReader bin = null; - try { - bin = new BufferedReader(new InputStreamReader(new FileInputStream(vfile))); + try (FileInputStream fis = new FileInputStream(vfile); + InputStreamReader reader = new InputStreamReader(fis); + BufferedReader bin = new BufferedReader(reader)) { String vstr = bin.readLine(); if (!StringUtil.isBlank(vstr)) { fileVersion = Long.parseLong(vstr); } } catch (Exception e) { log.info("Unable to read version file: " + e.getMessage()); - } finally { - StreamUtil.close(bin); } return fileVersion; @@ -54,13 +51,11 @@ public class VersionUtil */ public static void writeVersion (File vfile, long version) throws IOException { - PrintStream out = new PrintStream(new FileOutputStream(vfile)); - try { + try (FileOutputStream fos = new FileOutputStream(vfile); + PrintStream out = new PrintStream(fos)) { out.println(version); } catch (Exception e) { log.warning("Unable to write version file: " + e.getMessage()); - } finally { - StreamUtil.close(out); } } @@ -88,9 +83,9 @@ public class VersionUtil */ public static long readReleaseVersion (File relfile, String versRegex) { - BufferedReader in = null; - try { - in = new BufferedReader(new FileReader(relfile)); + try (FileReader reader = new FileReader(relfile); + BufferedReader in = new BufferedReader(reader)) { + String line = null, relvers = null; while ((line = in.readLine()) != null) { if (line.startsWith("JAVA_VERSION=")) { @@ -107,8 +102,6 @@ public class VersionUtil } catch (Exception e) { log.warning("Failed to read version from 'release' file", "file", relfile, e); return 0L; - } finally { - StreamUtil.close(in); } } From a083f32ce00c9a4236743f74fdeb0298ca65df56 Mon Sep 17 00:00:00 2001 From: Daniel Gredler Date: Fri, 24 Aug 2018 12:11:06 -0400 Subject: [PATCH 2/2] Add a few more try-with-resources --- .../threerings/getdown/data/Application.java | 5 +-- .../getdown/tools/AppletParamSigner.java | 33 ++++++++++--------- .../com/threerings/getdown/util/Config.java | 8 +++-- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/threerings/getdown/data/Application.java b/src/main/java/com/threerings/getdown/data/Application.java index e110e9e..55342c8 100644 --- a/src/main/java/com/threerings/getdown/data/Application.java +++ b/src/main/java/com/threerings/getdown/data/Application.java @@ -1241,8 +1241,9 @@ public class Application } if (_latest != null) { - try (InputStream in = ConnectionUtil.open(_latest, 0, 0).getInputStream()) { - BufferedReader bin = new BufferedReader(new InputStreamReader(in)); + try (InputStream in = ConnectionUtil.open(_latest, 0, 0).getInputStream(); + InputStreamReader reader = new InputStreamReader(in); + BufferedReader bin = new BufferedReader(reader)) { for (String[] pair : Config.parsePairs(bin, Config.createOpts(false))) { if (pair[0].equals("version")) { _targetVersion = Math.max(Long.parseLong(pair[1]), _targetVersion); diff --git a/src/main/java/com/threerings/getdown/tools/AppletParamSigner.java b/src/main/java/com/threerings/getdown/tools/AppletParamSigner.java index 4b93cda..0ff4d89 100644 --- a/src/main/java/com/threerings/getdown/tools/AppletParamSigner.java +++ b/src/main/java/com/threerings/getdown/tools/AppletParamSigner.java @@ -22,25 +22,26 @@ public class AppletParamSigner { public static void main (String[] args) { - try { - if (args.length != 7) { - System.err.println("AppletParamSigner keystore storepass alias keypass " + - "appbase appname imgpath"); - System.exit(255); - } + if (args.length != 7) { + System.err.println("AppletParamSigner keystore storepass alias keypass " + + "appbase appname imgpath"); + System.exit(255); + } - String keystore = args[0]; - String storepass = args[1]; - String alias = args[2]; - String keypass = args[3]; - String appbase = args[4]; - String appname = args[5]; - String imgpath = args[6]; - String params = appbase + appname + imgpath; + String keystore = args[0]; + String storepass = args[1]; + String alias = args[2]; + String keypass = args[3]; + String appbase = args[4]; + String appname = args[5]; + String imgpath = args[6]; + String params = appbase + appname + imgpath; + + try (FileInputStream fis = new FileInputStream(keystore); + BufferedInputStream bis = new BufferedInputStream(fis)) { KeyStore store = KeyStore.getInstance("JKS"); - store.load(new BufferedInputStream(new FileInputStream(keystore)), - storepass.toCharArray()); + store.load(bis, storepass.toCharArray()); PrivateKey key = (PrivateKey)store.getKey(alias, keypass.toCharArray()); Signature sig = Signature.getInstance(Digest.sigAlgorithm(Digest.VERSION)); sig.initSign(key); diff --git a/src/main/java/com/threerings/getdown/util/Config.java b/src/main/java/com/threerings/getdown/util/Config.java index 61d20b0..cce3113 100644 --- a/src/main/java/com/threerings/getdown/util/Config.java +++ b/src/main/java/com/threerings/getdown/util/Config.java @@ -13,7 +13,7 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStreamReader; import java.io.Reader; - +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -70,8 +70,10 @@ public class Config throws IOException { // annoyingly FileReader does not allow encoding to be specified (uses platform default) - InputStreamReader input = new InputStreamReader(new FileInputStream(source), "UTF-8"); - return parsePairs(input, opts); + try (FileInputStream fis = new FileInputStream(source); + InputStreamReader input = new InputStreamReader(fis, StandardCharsets.UTF_8)) { + return parsePairs(input, opts); + } } /**