Fix JSON serialization not closing the InputStream in some cases, as …

…well as not setting the UTF-8 character set explicitly (and thus falling back to the default platform encoding). (#69)
neoforged · Dec 8, 2023 · 0882c21 · 0882c21
1 parent 54008f5
commit 0882c21
Show file tree

Hide file tree

Showing 14 changed files with 91 additions and 134 deletions.
diff --git a/...src/main/java/net/neoforged/gradle/common/extensions/MinecraftArtifactCacheExtension.java b/...src/main/java/net/neoforged/gradle/common/extensions/MinecraftArtifactCacheExtension.java
@@ -1,7 +1,6 @@
 package net.neoforged.gradle.common.extensions;
 
 import com.google.common.collect.ImmutableMap;
-import com.google.gson.Gson;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
 import net.minecraftforge.gdi.ConfigurableDSLElement;
@@ -10,6 +9,7 @@
 import net.neoforged.gradle.common.util.FileCacheUtils;
 import net.neoforged.gradle.common.util.FileDownloadingUtils;
 import net.neoforged.gradle.common.util.MinecraftArtifactType;
+import net.neoforged.gradle.common.util.SerializationUtils;
 import net.neoforged.gradle.dsl.common.extensions.MinecraftArtifactCache;
 import net.neoforged.gradle.dsl.common.tasks.WithOutput;
 import net.neoforged.gradle.dsl.common.util.CacheFileSelector;
@@ -24,9 +24,7 @@
 
 import javax.inject.Inject;
 import java.io.File;
-import java.io.FileReader;
 import java.io.IOException;
-import java.io.Reader;
 import java.net.URL;
 import java.util.EnumMap;
 import java.util.Map;
@@ -177,21 +175,16 @@ public final File cache(final String url, final CacheFileSelector selector) {
     private File downloadVersionManifestToCache(Project project, final File cacheDirectory, final String minecraftVersion) {
         final File manifestFile = new File(new File(cacheDirectory, CacheFileSelector.launcherMetadata().getCacheDirectory()), CacheFileSelector.launcherMetadata().getCacheFileName());
 
-        Gson gson = new Gson();
         String url = null;
-        try(final Reader reader = new FileReader(manifestFile)) {
-            JsonObject json = gson.fromJson(reader, JsonObject.class);
-
-            for (JsonElement e : json.getAsJsonArray("versions")) {
-                String v = e.getAsJsonObject().get("id").getAsString();
-                if (Objects.equals(minecraftVersion, "+") || v.equals(minecraftVersion)) {
-                    url = e.getAsJsonObject().get("url").getAsString();
-                    break;
-                }
-            }
 
-        } catch (IOException e) {
-            throw new RuntimeException("Could not read the launcher manifest", e);
+        JsonObject json = SerializationUtils.fromJson(manifestFile, JsonObject.class);
+
+        for (JsonElement e : json.getAsJsonArray("versions")) {
+            String v = e.getAsJsonObject().get("id").getAsString();
+            if (Objects.equals(minecraftVersion, "+") || v.equals(minecraftVersion)) {
+                url = e.getAsJsonObject().get("url").getAsString();
+                break;
+            }
         }
 
         if (url == null) {
@@ -229,10 +222,7 @@ private File doDownloadVersionDownloadToCache(Project project, File cacheDirecto
         final File versionManifestFile = this.cacheVersionManifest(minecraftVersion);
 
         try {
-            Gson gson = new Gson();
-            Reader reader = new FileReader(versionManifestFile);
-            JsonObject json = gson.fromJson(reader, JsonObject.class);
-            reader.close();
+            JsonObject json = SerializationUtils.fromJson(versionManifestFile, JsonObject.class);
 
             JsonObject artifactInfo = json.getAsJsonObject("downloads").getAsJsonObject(artifact);
             String url = artifactInfo.get("url").getAsString();
@@ -274,17 +264,10 @@ public String resolveVersion(final String gameVersion) {
 
         final File launcherMetadata = this.cacheLauncherMetadata();
 
-        Gson gson = new Gson();
-        String url = null;
-        try(final Reader reader = new FileReader(launcherMetadata)) {
-            JsonObject json = gson.fromJson(reader, JsonObject.class);
-
-            for (JsonElement e : json.getAsJsonArray("versions")) {
-                return e.getAsJsonObject().get("id").getAsString();
-            }
+        JsonObject json = SerializationUtils.fromJson(launcherMetadata, JsonObject.class);
 
-        } catch (IOException e) {
-            throw new RuntimeException("Could not read the launcher manifest", e);
+        for (JsonElement e : json.getAsJsonArray("versions")) {
+            return e.getAsJsonObject().get("id").getAsString();
         }
 
         throw new IllegalStateException("Could not find the correct version json.");

diff --git a/common/src/main/java/net/neoforged/gradle/common/runtime/tasks/ListLibraries.java b/common/src/main/java/net/neoforged/gradle/common/runtime/tasks/ListLibraries.java
@@ -1,12 +1,12 @@
 package net.neoforged.gradle.common.runtime.tasks;
 
-import com.google.gson.Gson;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
 import net.neoforged.gradle.common.CommonProjectPlugin;
 import net.neoforged.gradle.common.caching.CentralCacheService;
 import net.neoforged.gradle.common.runtime.tasks.action.DownloadFileAction;
 import net.neoforged.gradle.common.util.FileCacheUtils;
+import net.neoforged.gradle.common.util.SerializationUtils;
 import net.neoforged.gradle.util.HashFunction;
 import net.neoforged.gradle.util.TransformerUtils;
 import org.gradle.api.file.DirectoryProperty;
@@ -94,12 +94,9 @@ private List<FileList.Entry> listBundleLibraries(FileSystem bundleFs) throws IOE
         return FileList.read(bundleFs.getPath("META-INF", "libraries.list")).entries;
     }
 
-    private Set<PathAndUrl> listDownloadJsonLibraries() throws IOException {
-        Gson gson = new Gson();
-        Reader reader = new FileReader(getDownloadedVersionJsonFile().getAsFile().get());
-        JsonObject json = gson.fromJson(reader, JsonObject.class);
-        reader.close();
-
+    private Set<PathAndUrl> listDownloadJsonLibraries() {
+        JsonObject json = SerializationUtils.fromJson(getDownloadedVersionJsonFile().getAsFile().get(), JsonObject.class);
+
         // Gather all the libraries
         Set<PathAndUrl> artifacts = new HashSet<>();
         for (JsonElement libElement : json.getAsJsonArray("libraries")) {

diff --git a/common/src/main/java/net/neoforged/gradle/common/tasks/FileCacheProviding.java b/common/src/main/java/net/neoforged/gradle/common/tasks/FileCacheProviding.java
@@ -1,9 +1,8 @@
 package net.neoforged.gradle.common.tasks;
 
-import com.google.gson.Gson;
-import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
 import net.neoforged.gradle.common.util.FileDownloadingUtils;
+import net.neoforged.gradle.common.util.SerializationUtils;
 import net.neoforged.gradle.dsl.common.tasks.NeoGradleBase;
 import net.neoforged.gradle.dsl.common.tasks.WithOutput;
 import net.neoforged.gradle.dsl.common.tasks.WithWorkspace;
@@ -16,10 +15,7 @@
 import org.gradle.work.DisableCachingByDefault;
 
 import java.io.File;
-import java.io.FileReader;
 import java.io.IOException;
-import java.io.Reader;
-import java.util.Objects;
 
 @DisableCachingByDefault(because = "This is an abstract underlying task which provides defaults and systems for caching game artifacts.")
 public abstract class FileCacheProviding extends NeoGradleBase implements WithOutput, WithWorkspace {
@@ -62,48 +58,27 @@ protected void downloadJsonTo(String url) {
     }
 
     protected void doDownloadVersionDownloadToCache(final String artifact, final String potentialError, File versionManifest) {
-        try(final Reader reader = new FileReader(versionManifest)) {
-            final File output = getOutput().get().getAsFile();
-
-            final Gson gson = new Gson();
-            final JsonObject json = gson.fromJson(reader, JsonObject.class);
+        JsonObject json = SerializationUtils.fromJson(versionManifest, JsonObject.class);
 
-            final JsonObject artifactInfo = json.getAsJsonObject("downloads").getAsJsonObject(artifact);
-            final String url = artifactInfo.get("url").getAsString();
-            final String hash = artifactInfo.get("sha1").getAsString();
-            final String version = json.getAsJsonObject().get("id").getAsString();
-
-            final FileDownloadingUtils.DownloadInfo info = new FileDownloadingUtils.DownloadInfo(url, hash, "jar", version, artifact);
-
+        final JsonObject artifactInfo = json.getAsJsonObject("downloads").getAsJsonObject(artifact);
+        final String url = artifactInfo.get("url").getAsString();
+        final String hash = artifactInfo.get("sha1").getAsString();
+        final String version = json.getAsJsonObject().get("id").getAsString();
+
+        final FileDownloadingUtils.DownloadInfo info = new FileDownloadingUtils.DownloadInfo(url, hash, "jar", version, artifact);
+
+        final File output = getOutput().get().getAsFile();
+        try {
             if (output.exists()) {
                 final String fileHash = HashFunction.SHA1.hash(output);
                 if (fileHash.equals(hash)) {
                     return;
                 }
             }
-            
+
             FileDownloadingUtils.downloadTo(getIsOffline().get(), info, output);
         } catch (IOException e) {
             throw new RuntimeException(potentialError, e);
         }
     }
-
-    protected static String resolveVersion(final String gameVersion, final File launcherMetadata) {
-        if (!Objects.equals(gameVersion, "+"))
-            return gameVersion;
-
-        Gson gson = new Gson();
-        try(final Reader reader = new FileReader(launcherMetadata)) {
-            JsonObject json = gson.fromJson(reader, JsonObject.class);
-
-            for (JsonElement e : json.getAsJsonArray("versions")) {
-                return e.getAsJsonObject().get("id").getAsString();
-            }
-
-        } catch (IOException e) {
-            throw new RuntimeException("Could not read the launcher manifest", e);
-        }
-
-        throw new IllegalStateException("Could not find the correct version json.");
-    }
 }
diff --git a/...ain/java/net/neoforged/gradle/common/tasks/MinecraftVersionManifestFileCacheProvider.java b/...ain/java/net/neoforged/gradle/common/tasks/MinecraftVersionManifestFileCacheProvider.java
@@ -1,16 +1,13 @@
 package net.neoforged.gradle.common.tasks;
 
-import com.google.gson.Gson;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
+import net.neoforged.gradle.common.util.SerializationUtils;
 import net.neoforged.gradle.dsl.common.util.CacheFileSelector;
 import org.gradle.api.file.RegularFileProperty;
 import org.gradle.api.provider.Property;
 import org.gradle.api.tasks.*;
 
-import java.io.FileReader;
-import java.io.IOException;
-import java.io.Reader;
 import java.util.Objects;
 
 @CacheableTask
@@ -36,23 +33,17 @@ public void doDownload() {
 
     private void downloadVersionManifestToCache() {
         final String minecraftVersion = getMinecraftVersion().get();
-        final Gson gson = new Gson();
-
-        try(final Reader reader = new FileReader(getLauncherManifest().get().getAsFile())) {
-            JsonObject json = gson.fromJson(reader, JsonObject.class);
-
-            for (JsonElement e : json.getAsJsonArray("versions")) {
-                String v = e.getAsJsonObject().get("id").getAsString();
-                if (Objects.equals(minecraftVersion, "+") || v.equals(minecraftVersion)) {
-                    downloadJsonTo(e.getAsJsonObject().get("url").getAsString());
-                    return;
-                }
+
+        JsonObject json = SerializationUtils.fromJson(getLauncherManifest().get().getAsFile(), JsonObject.class);
+
+        for (JsonElement e : json.getAsJsonArray("versions")) {
+            String v = e.getAsJsonObject().get("id").getAsString();
+            if (Objects.equals(minecraftVersion, "+") || v.equals(minecraftVersion)) {
+                downloadJsonTo(e.getAsJsonObject().get("url").getAsString());
+                return;
             }
-
-        } catch (IOException e) {
-            throw new RuntimeException("Could not read the launcher manifest", e);
         }
-        
+
         throw new IllegalStateException("Could not find the correct version json for version: " + minecraftVersion);
     }
 }
diff --git a/common/src/main/java/net/neoforged/gradle/common/util/FileDownloadingUtils.java b/common/src/main/java/net/neoforged/gradle/common/util/FileDownloadingUtils.java
@@ -100,13 +100,7 @@ private static boolean copyURLToFileIfNewer(URL url, Path target) throws IOExcep
                     Files.copy(stream, tempFile, StandardCopyOption.REPLACE_EXISTING);
                 }
 
-                try {
-                    Files.move(tempFile, target, StandardCopyOption.ATOMIC_MOVE);
-                } catch (AtomicMoveNotSupportedException e) {
-                    // Atomic moves within the same directory should have worked.
-                    // We fall back to the inferior normal move. We should log this issue, but there is no logger here.
-                    Files.move(tempFile, target, StandardCopyOption.REPLACE_EXISTING);
-                }
+                move(target, tempFile);
 
                 if (urlConnection.getLastModified() != 0) {
                     Files.setLastModifiedTime(target, FileTime.fromMillis(urlConnection.getLastModified()));
@@ -121,6 +115,33 @@ private static boolean copyURLToFileIfNewer(URL url, Path target) throws IOExcep
         }
     }
 
+    private static void move(Path target, Path tempFile) throws IOException {
+        int tries = 0;
+        while (true) {
+            tries++;
+            try {
+                try {
+                    Files.move(tempFile, target, StandardCopyOption.ATOMIC_MOVE);
+                } catch (AtomicMoveNotSupportedException e) {
+                    // Atomic moves within the same directory should have worked.
+                    // We fall back to the inferior normal move. We should log this issue, but there is no logger here.
+                    Files.move(tempFile, target, StandardCopyOption.REPLACE_EXISTING);
+                }
+                break;
+            } catch (IOException e) {
+                if (tries >= 5) {
+                    throw e;
+                }
+                // Wait a bit to give whatever concurrent process has it locked to unlock...
+                try {
+                    Thread.sleep(5000);
+                } catch (InterruptedException ex) {
+                    Thread.currentThread().interrupt();
+                }
+            }
+        }
+    }
+
     public static File getMCDir() {
         switch (VersionJson.OS.getCurrent()) {
             case OSX:

diff --git a/common/src/main/java/net/neoforged/gradle/common/util/VersionJson.java b/common/src/main/java/net/neoforged/gradle/common/util/VersionJson.java
@@ -27,6 +27,7 @@
 import java.io.*;
 import java.lang.reflect.Type;
 import java.net.URL;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import java.util.*;
 import java.util.regex.Pattern;
@@ -42,19 +43,21 @@ public class VersionJson implements Serializable {
             .setPrettyPrinting().create();
 
 
-    public static VersionJson get(Path path) throws FileNotFoundException {
+    public static VersionJson get(Path path) throws IOException {
         return get(path.toFile());
     }
 
-    public static VersionJson get(@Nullable File file) throws FileNotFoundException {
+    public static VersionJson get(@Nullable File file) throws IOException {
         if (file == null) {
             throw new IllegalArgumentException("VersionJson File can not be null!");
         }
-        return get(new FileInputStream(file));
+        try (InputStream in = new FileInputStream(file)) {
+            return get(in);
+        }
     }
 
     public static VersionJson get(InputStream stream) {
-        return GSON.fromJson(new InputStreamReader(stream), VersionJson.class);
+        return GSON.fromJson(new InputStreamReader(stream, StandardCharsets.UTF_8), VersionJson.class);
     }
 
     private String id;

diff --git a/...src/main/groovy/net/neoforged/gradle/dsl/common/configuration/VersionedConfiguration.java b/...src/main/groovy/net/neoforged/gradle/dsl/common/configuration/VersionedConfiguration.java
@@ -27,6 +27,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
 
 public class VersionedConfiguration {
 
@@ -35,7 +36,7 @@ public class VersionedConfiguration {
     public int spec = 1;
 
     public static int getSpec(InputStream stream) throws IOException {
-        return GSON.fromJson(new InputStreamReader(stream), VersionedConfiguration.class).spec;
+        return GSON.fromJson(new InputStreamReader(stream, StandardCharsets.UTF_8), VersionedConfiguration.class).spec;
     }
     public static int getSpec(byte[] data) throws IOException {
         return getSpec(new ByteArrayInputStream(data));

diff --git a/...oovy/net/neoforged/gradle/dsl/neoform/configuration/NeoFormConfigConfigurationSpecV1.java b/...oovy/net/neoforged/gradle/dsl/neoform/configuration/NeoFormConfigConfigurationSpecV1.java
@@ -32,6 +32,7 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.lang.reflect.Type;
+import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -46,7 +47,7 @@ public class NeoFormConfigConfigurationSpecV1 extends VersionedConfiguration {
             .setPrettyPrinting().create();
 
     public static NeoFormConfigConfigurationSpecV1 get(InputStream stream) {
-        return GSON.fromJson(new InputStreamReader(stream), NeoFormConfigConfigurationSpecV1.class);
+        return GSON.fromJson(new InputStreamReader(stream, StandardCharsets.UTF_8), NeoFormConfigConfigurationSpecV1.class);
     }
     public static NeoFormConfigConfigurationSpecV1 get(byte[] data) {
         return get(new ByteArrayInputStream(data));

diff --git a/...oovy/net/neoforged/gradle/dsl/neoform/configuration/NeoFormConfigConfigurationSpecV2.java b/...oovy/net/neoforged/gradle/dsl/neoform/configuration/NeoFormConfigConfigurationSpecV2.java
@@ -24,6 +24,7 @@
 import org.gradle.api.tasks.Input;
 
 import java.io.*;
+import java.nio.charset.StandardCharsets;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
 
@@ -39,7 +40,7 @@ public static NeoFormConfigConfigurationSpecV2 get(File file) {
         }
     }
     public static NeoFormConfigConfigurationSpecV2 get(InputStream stream) {
-        try(final InputStreamReader reader = new InputStreamReader(stream)) {
+        try (final InputStreamReader reader = new InputStreamReader(stream, StandardCharsets.UTF_8)) {
             return GSON.fromJson(reader, NeoFormConfigConfigurationSpecV2.class);
         } catch (IOException e) {
             throw new UncheckedIOException(e);