1 files changed, 28 insertions, 63 deletions
diff --git a/value/src/main/java/com/google/auto/value/processor/TemplateVars.java b/value/src/main/java/com/google/auto/value/processor/TemplateVars.java
index d9e3337b..73253629 100644
--- a/value/src/main/java/com/google/auto/value/processor/TemplateVars.java
+++ b/value/src/main/java/com/google/auto/value/processor/TemplateVars.java
@@ -15,27 +15,28 @@
  */
 package com.google.auto.value.processor;
 
-import com.google.common.base.Throwables;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.base.Ascii;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.io.ByteStreams;
 import com.google.escapevelocity.Template;
 import java.io.File;
 import java.io.FileInputStream;
-import java.io.FilterInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
-import java.io.UnsupportedEncodingException;
+import java.io.StringReader;
+import java.io.UncheckedIOException;
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
-import java.nio.charset.StandardCharsets;
 import java.util.Map;
 import java.util.TreeMap;
-import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 
 /**
@@ -95,7 +96,7 @@ abstract class TemplateVars {
    * concrete subclass of TemplateVars) into the template returned by {@link #parsedTemplate()}.
    */
   String toText() {
-    Map<String, Object> vars = toVars();
+    ImmutableMap<String, Object> vars = toVars();
     return parsedTemplate().evaluate(vars);
   }
 
@@ -121,69 +122,42 @@ abstract class TemplateVars {
 
   static Template parsedTemplateForResource(String resourceName) {
     try {
-      return Template.parseFrom(resourceName, TemplateVars::readerFromResource);
-    } catch (UnsupportedEncodingException e) {
-      throw new AssertionError(e);
-    } catch (IOException | NullPointerException | IllegalStateException e) {
-      // https://github.com/google/auto/pull/439 says that we can get NullPointerException.
-      // https://github.com/google/auto/issues/715 says that we can get IllegalStateException
-      return retryParseAfterException(resourceName, e);
-    }
-  }
-
-  private static Template retryParseAfterException(String resourceName, Exception exception) {
-    try {
       return Template.parseFrom(resourceName, TemplateVars::readerFromUrl);
-    } catch (IOException t) {
-      // Chain the original exception so we can see both problems.
-      Throwables.getRootCause(exception).initCause(t);
-      throw new AssertionError(exception);
+    } catch (IOException e) {
+      throw new UncheckedIOException(e);
     }
   }
 
-  private static Reader readerFromResource(String resourceName) {
-    InputStream in = TemplateVars.class.getResourceAsStream(resourceName);
-    if (in == null) {
-      throw new IllegalArgumentException("Could not find resource: " + resourceName);
-    }
-    return new InputStreamReader(in, StandardCharsets.UTF_8);
-  }
-
   // This is an ugly workaround for https://bugs.openjdk.java.net/browse/JDK-6947916, as
   // reported in https://github.com/google/auto/issues/365.
   // The issue is that sometimes the InputStream returned by JarURLCollection.getInputStream()
   // can be closed prematurely, which leads to an IOException saying "Stream closed".
-  // We catch all IOExceptions, and fall back on logic that opens the jar file directly and
-  // loads the resource from it. Since that doesn't use JarURLConnection, it shouldn't be
-  // susceptible to the same bug. We only use this as fallback logic rather than doing it always,
-  // because jars are memory-mapped by URLClassLoader, so loading a resource in the usual way
-  // through the getResourceAsStream should be a lot more efficient than reopening the jar.
+  // To avoid that issue, we open the jar file directly and load the resource from it. Since that
+  // doesn't use JarURLConnection, it shouldn't be susceptible to the same bug.
   private static Reader readerFromUrl(String resourceName) throws IOException {
     URL resourceUrl = TemplateVars.class.getResource(resourceName);
     if (resourceUrl == null) {
-      // This is unlikely, since getResourceAsStream has already succeeded for the same resource.
       throw new IllegalArgumentException("Could not find resource: " + resourceName);
     }
-    InputStream in;
     try {
-      if (resourceUrl.getProtocol().equalsIgnoreCase("file")) {
-        in = inputStreamFromFile(resourceUrl);
-      } else if (resourceUrl.getProtocol().equalsIgnoreCase("jar")) {
-        in = inputStreamFromJar(resourceUrl);
+      if (Ascii.equalsIgnoreCase(resourceUrl.getProtocol(), "file")) {
+        return readerFromFile(resourceUrl);
+      } else if (Ascii.equalsIgnoreCase(resourceUrl.getProtocol(), "jar")) {
+        return readerFromJar(resourceUrl);
       } else {
-        throw new AssertionError("Template fallback logic fails for: " + resourceUrl);
+        throw new AssertionError("Template search logic fails for: " + resourceUrl);
       }
     } catch (URISyntaxException e) {
       throw new IOException(e);
     }
-    return new InputStreamReader(in, StandardCharsets.UTF_8);
   }
 
-  private static InputStream inputStreamFromJar(URL resourceUrl)
-      throws URISyntaxException, IOException {
+  private static Reader readerFromJar(URL resourceUrl) throws URISyntaxException, IOException {
     // Jar URLs look like this: jar:file:/path/to/file.jar!/entry/within/jar
     // So take apart the URL to open the jar /path/to/file.jar and read the entry
     // entry/within/jar from it.
+    // We could use the methods from JarURLConnection here, but that would risk provoking the same
+    // problem that prompted this workaround in the first place.
     String resourceUrlString = resourceUrl.toString().substring("jar:".length());
     int bang = resourceUrlString.lastIndexOf('!');
     String entryName = resourceUrlString.substring(bang + 1);
@@ -191,28 +165,19 @@ abstract class TemplateVars {
       entryName = entryName.substring(1);
     }
     URI jarUri = new URI(resourceUrlString.substring(0, bang));
-    JarFile jar = new JarFile(new File(jarUri));
-    JarEntry entry = jar.getJarEntry(entryName);
-    InputStream in = jar.getInputStream(entry);
-    // We have to be careful not to close the JarFile before the stream has been read, because
-    // that would also close the stream. So we defer closing the JarFile until the stream is closed.
-    return new FilterInputStream(in) {
-      @Override
-      public void close() throws IOException {
-        super.close();
-        jar.close();
-      }
-    };
+    try (JarFile jar = new JarFile(new File(jarUri));
+        InputStream in = jar.getInputStream(jar.getJarEntry(entryName))) {
+      String contents = new String(ByteStreams.toByteArray(in), UTF_8);
+      return new StringReader(contents);
+    }
   }
 
-  // We don't really expect this case to arise, since the bug we're working around concerns jars
-  // not individual files. However, when running the test for this workaround from Maven, we do
-  // have files. That does mean the test is basically useless there, but Google's internal build
-  // system does run it using a jar, so we do have coverage.
-  private static InputStream inputStreamFromFile(URL resourceUrl)
+  // In most execution environments, we'll be dealing with a jar, but we handle individual files
+  // just for cases like running our tests with Maven.
+  private static Reader readerFromFile(URL resourceUrl)
       throws IOException, URISyntaxException {
     File resourceFile = new File(resourceUrl.toURI());
-    return new FileInputStream(resourceFile);
+    return new InputStreamReader(new FileInputStream(resourceFile), UTF_8);
   }
 
   private static Object fieldValue(Field field, Object container) {