Standardize file reading and writing

- Introduce helper methods to read and write files that enforce the character set used, using the singleton for UTF-8 in StandardCharsets instead of retrieving it using Charsets.forName - Using the UTF-8 singleton in StandardCharsets whereever else the UTF-8 character set is referenced and used. - Where encoding is not under our control (i.e., the user uploads their own file to us), add a note saying that it should be encoded with UTF-8.
mtughan · May 1, 2022 · c9ee54f · c9ee54f
1 parent ab3d736
commit c9ee54f
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 30 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/scriptler/ScriptlerManagement.java b/src/main/java/org/jenkinsci/plugins/scriptler/ScriptlerManagement.java
@@ -31,7 +31,6 @@
 import hudson.security.Permission;
 import jenkins.model.Jenkins;
 import org.apache.commons.fileupload.FileItem;
-import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang.StringUtils;
 import org.jenkinsci.plugins.scriptler.config.Parameter;
 import org.jenkinsci.plugins.scriptler.config.Script;
@@ -49,7 +48,6 @@
 import javax.servlet.http.HttpServletResponse;
 import java.io.File;
 import java.io.IOException;
-import java.nio.charset.StandardCharsets;
 import java.text.SimpleDateFormat;
 import java.util.*;
 import java.util.logging.Level;
@@ -268,7 +266,7 @@ private String saveScriptAndForward(String id, String name, String comment, Stri
             throw new IOException("Invalid file path received: " + id);
         }
 
-        FileUtils.writeStringToFile(newScriptFile, script, StandardCharsets.UTF_8);
+        ScriptHelper.writeScriptToFile(newScriptFile, script);
 
         commitFileToGitRepo(finalFileName);
 
@@ -416,7 +414,7 @@ public HttpResponse doUploadScript(StaplerRequest req) throws IOException, Servl
             script = new Script(fixedFileName, fixedFileName, true, nonAdministerUsing, false);
         }
 
-        String scriptSource = FileUtils.readFileToString(f, "UTF-8");
+        String scriptSource = ScriptHelper.readScriptFromFile(f);
         ScriptHelper.putScriptInApprovalQueueIfRequired(scriptSource);
 
         ScriptlerConfiguration config = getConfiguration();

diff --git a/src/main/java/org/jenkinsci/plugins/scriptler/ScriptlerPluginImpl.java b/src/main/java/org/jenkinsci/plugins/scriptler/ScriptlerPluginImpl.java
@@ -37,7 +37,6 @@
 import hudson.security.PermissionGroup;
 import hudson.security.PermissionScope;
 import jenkins.model.Jenkins;
-import org.apache.commons.io.FileUtils;
 import org.jenkinsci.plugins.scriptler.config.Script;
 import org.jenkinsci.plugins.scriptler.config.ScriptlerConfiguration;
 import org.jenkinsci.plugins.scriptler.util.ScriptHelper;
@@ -108,7 +107,7 @@ private static void setupExistingScripts() throws Exception {
         for (Script script : ScriptlerConfiguration.getConfiguration().getScripts()) {
             File scriptFile = new File(ScriptlerManagement.getScriptDirectory(), script.getScriptPath());
             try{
-                String scriptSource = FileUtils.readFileToString(scriptFile, "UTF-8");
+                String scriptSource = ScriptHelper.readScriptFromFile(scriptFile);
 
                 // we cannot do that during start since the ScriptApproval is not yet loaded
                 // and only after JOB_LOADED to have the securityRealm configured

diff --git a/src/main/java/org/jenkinsci/plugins/scriptler/SyncUtil.java b/src/main/java/org/jenkinsci/plugins/scriptler/SyncUtil.java
@@ -11,7 +11,6 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import org.apache.commons.io.FileUtils;
 import org.jenkinsci.plugins.scriptler.config.Parameter;
 import org.jenkinsci.plugins.scriptler.config.Script;
 import org.jenkinsci.plugins.scriptler.config.ScriptlerConfiguration;
@@ -40,7 +39,7 @@ public static void syncDirWithCfg(File scriptDirectory, ScriptlerConfiguration c
         // if not, add it to the configuration
         for (File file : availablePhysicalScripts) {
             if (cfg.getScriptById(file.getName()) == null) {
-                final ScriptInfo info = ScriptHelper.extractScriptInfo(FileUtils.readFileToString(file, "UTF-8"));
+                final ScriptInfo info = ScriptHelper.extractScriptInfo(ScriptHelper.readScriptFromFile(file));
                 if (info != null) {
                     final List<String> paramList = info.getParameters();
                     Parameter[] parameters = new Parameter[paramList.size()];

diff --git a/src/main/java/org/jenkinsci/plugins/scriptler/share/gh/GHCatalog.java b/src/main/java/org/jenkinsci/plugins/scriptler/share/gh/GHCatalog.java
@@ -1,19 +1,20 @@
 package org.jenkinsci.plugins.scriptler.share.gh;
 
 import hudson.Extension;
-import hudson.Util;
 import hudson.ProxyConfiguration;
 
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.URL;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import org.apache.commons.io.IOUtils;
 import org.jenkinsci.plugins.scriptler.share.CatalogInfo;
 import org.jenkinsci.plugins.scriptler.share.ScriptInfo;
 import org.jenkinsci.plugins.scriptler.share.ScriptInfoCatalog;
@@ -79,13 +80,9 @@ public CatalogInfo getInfo() {
     @Override
     public String getScriptSource(ScriptInfo scriptInfo) {
 
-        try {
-
-            final String scriptUrl = CATALOG_INFO.getReplacedDownloadUrl(scriptInfo.getName(), scriptInfo.getId());
-            ByteArrayOutputStream out = new ByteArrayOutputStream();
-            Util.copyStreamAndClose(ProxyConfiguration.open(new URL(scriptUrl)).getInputStream(), out);
-            return out.toString("UTF-8");
-
+        final String scriptUrl = CATALOG_INFO.getReplacedDownloadUrl(scriptInfo.getName(), scriptInfo.getId());
+        try (InputStream is = ProxyConfiguration.getInputStream(new URL(scriptUrl))) {
+            return IOUtils.toString(is, StandardCharsets.UTF_8);
         } catch (Exception e) {
             LOGGER.log(Level.SEVERE, "not abe to load script sources from GH for: " + scriptInfo, e);
         }

diff --git a/src/main/java/org/jenkinsci/plugins/scriptler/util/ScriptHelper.java b/src/main/java/org/jenkinsci/plugins/scriptler/util/ScriptHelper.java
@@ -1,15 +1,14 @@
 package org.jenkinsci.plugins.scriptler.util;
 
+import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.model.Computer;
 import hudson.util.StreamTaskListener;
 
-import java.io.BufferedReader;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStreamReader;
 import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.logging.Level;
@@ -25,7 +24,7 @@
 import net.sf.json.JSONObject;
 import net.sf.json.JSONSerializer;
 
-import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang.StringUtils;
 import org.jenkinsci.plugins.scriptler.Messages;
 import org.jenkinsci.plugins.scriptler.ScriptlerManagement;
@@ -55,6 +54,15 @@ public class ScriptHelper {
         JSON_CLASS_MAPPING.put("parameters", Parameter.class);
     }
 
+    @NonNull
+    public static String readScriptFromFile(@NonNull File file) throws IOException {
+        return FileUtils.readFileToString(file, StandardCharsets.UTF_8);
+    }
+
+    public static void writeScriptToFile(@NonNull File file, @NonNull String script) throws IOException {
+        FileUtils.writeStringToFile(file, script, StandardCharsets.UTF_8);
+    }
+
     /**
      * Loads the script information.
      * 
@@ -71,10 +79,8 @@ public class ScriptHelper {
         Script s = ScriptlerConfiguration.getConfiguration().getScriptById(id);
         if (withSrc && s != null) {
             File scriptSrc = new File(ScriptlerManagement.getScriptDirectory(), s.getScriptPath());
-            try (BufferedReader reader = new BufferedReader(new InputStreamReader(new FileInputStream(scriptSrc),
-                            Charset.forName("UTF-8")))) {
-                String src = IOUtils.toString(reader);
-                s.setScript(src);
+            try {
+                s.setScript(readScriptFromFile(scriptSrc));
             } catch (IOException e) {
                 LOGGER.log(Level.SEVERE, Messages.scriptSourceNotFound(id));
             }
@@ -161,7 +167,7 @@ public static String runScript(String node, String scriptTxt, Parameter[] parame
             try {
                 Computer comp = Jenkins.getInstance().getComputer(node);
                 if (comp == null && "(master)".equals(node)) {
-                    output = MasterComputer.localChannel.call(new GroovyScript(scriptTxt, parameters, false, new StreamTaskListener(sos)));
+                    output = MasterComputer.localChannel.call(new GroovyScript(scriptTxt, parameters, false, new StreamTaskListener(sos, StandardCharsets.UTF_8)));
                 } else if (comp == null) {
                     output = Messages.node_not_found(node) + "\n";
                 } else {
@@ -170,15 +176,15 @@ public static String runScript(String node, String scriptTxt, Parameter[] parame
                     }
 
                     else {
-                        output = comp.getChannel().call(new GroovyScript(scriptTxt, parameters, false, new StreamTaskListener(sos)));
+                        output = comp.getChannel().call(new GroovyScript(scriptTxt, parameters, false, new StreamTaskListener(sos, StandardCharsets.UTF_8)));
                     }
                 }
 
             } catch (InterruptedException e) {
                 throw new ServletException(e);
             }
         }
-        return sos.toString(Charset.forName("UTF-8").name());
+        return new String(sos.toByteArray(), StandardCharsets.UTF_8);
     }
 
     /**

diff --git a/src/main/resources/org/jenkinsci/plugins/scriptler/ScriptlerManagement/scriptSettings.jelly b/src/main/resources/org/jenkinsci/plugins/scriptler/ScriptlerManagement/scriptSettings.jelly
@@ -61,7 +61,8 @@
 
 			<h1>${%Upload new Script}</h1>
 			<div style="margin-bottom: 1em;">
-				${%uploadtext}
+				${%uploadtext}<br/>
+				${%UploadEncoding}
               </div>
 			<f:form method="post" action="uploadScript" name="uploadScript"
 				enctype="multipart/form-data">

diff --git a/...n/resources/org/jenkinsci/plugins/scriptler/ScriptlerManagement/scriptSettings.properties b/...n/resources/org/jenkinsci/plugins/scriptler/ScriptlerManagement/scriptSettings.properties
@@ -25,7 +25,8 @@ intro=\
   save it. Later you will be able to execute it on any slave/node. Useful for trouble-shooting and diagnostics. \
   Use the ''println'' command to see the output (if you use <tt>System.out</tt>, \
   it will go to the server''s stdout, which is harder to see.). If you think your script might be useful to others too, please share it on <a href="https://github.com/jenkinsci/jenkins-scripts" target="_blank">https://github.com/jenkinsci/jenkins-scripts</a>.
-uploadtext=Select a Groovy script from your local system to be uploaded (*.groovy). 
+uploadtext=Select a Groovy script from your local system to be uploaded (*.groovy).
+UploadEncoding=Uploaded files should be encoded with UTF-8.
 Permission = Allow usage in build step
 Restriction = Restriction
 RestrictionDescription = Script is always executed on Master