Merge pull request #52 from mtughan/JENKINS-73394

JENKINS-73394: Encode special characters

Author: jonesbusy

src/main/java/io/jenkins/plugins/artifactory_artifacts/ArtifactoryClient.java

@@ -5,8 +5,6 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Serializable;
-import java.net.URLEncoder;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
@@ -42,7 +40,7 @@ public ArtifactoryClient(@NonNull ArtifactoryConfig config) {
      */
     public void uploadArtifact(Path file, String targetPath) throws IOException {
         UploadableArtifact artifact =
-                artifactory.repository(this.config.repository).upload(urlEncodeParts(targetPath), file.toFile());
+                artifactory.repository(this.config.repository).upload(Utils.urlEncodeParts(targetPath), file.toFile());
         artifact.withSize(Files.size(file));
         artifact.withListener(
                 (bytesRead, totalBytes) -> LOGGER.trace(String.format("Uploaded %d/%d", bytesRead, totalBytes)));
@@ -55,7 +53,7 @@ public void uploadArtifact(Path file, String targetPath) throws IOException {
      * @param targetPath the path of the artifact to delete
      */
     public void deleteArtifact(String targetPath) {
-        artifactory.repository(this.config.repository).delete(urlEncodeParts(targetPath));
+        artifactory.repository(this.config.repository).delete(Utils.urlEncodeParts(targetPath));
     }
 
     /**
@@ -64,8 +62,8 @@ public void deleteArtifact(String targetPath) {
      * @param targetPath the target path
      */
     public void move(String sourcePath, String targetPath) {
-        ItemHandle sourceItem = artifactory.repository(this.config.repository).folder(urlEncodeParts(sourcePath));
-        sourceItem.move(this.config.repository, urlEncodeParts(targetPath));
+        ItemHandle sourceItem = artifactory.repository(this.config.repository).folder(Utils.urlEncodeParts(sourcePath));
+        sourceItem.move(this.config.repository, Utils.urlEncodeParts(targetPath));
     }
 
     /**
@@ -74,7 +72,7 @@ public void move(String sourcePath, String targetPath) {
      * @param targetPath the target path
      */
     public void copy(String sourcePath, String targetPath) {
-        ItemHandle sourceItem = artifactory.repository(this.config.repository).folder(urlEncodeParts(sourcePath));
+        ItemHandle sourceItem = artifactory.repository(this.config.repository).folder(Utils.urlEncodeParts(sourcePath));
         sourceItem.copy(this.config.repository, targetPath);
     }
 
@@ -86,7 +84,7 @@ public void copy(String sourcePath, String targetPath) {
      */
     public InputStream downloadArtifact(String targetPath) throws IOException {
         DownloadableArtifact artifact =
-                artifactory.repository(this.config.repository).download(urlEncodeParts(targetPath));
+                artifactory.repository(this.config.repository).download(Utils.urlEncodeParts(targetPath));
         return artifact.doDownload();
     }
 
@@ -98,7 +96,7 @@ public InputStream downloadArtifact(String targetPath) throws IOException {
      */
     public boolean isFolder(String targetPath) throws IOException {
         try {
-            return artifactory.repository(this.config.repository).isFolder(urlEncodeParts(targetPath));
+            return artifactory.repository(this.config.repository).isFolder(Utils.urlEncodeParts(targetPath));
         } catch (Exception e) {
             LOGGER.debug(String.format("Failed to check if %s is a folder", targetPath));
             return false;
@@ -140,7 +138,7 @@ public boolean isFile(String targetPath) throws IOException {
         try {
             File file = artifactory
                     .repository(this.config.repository)
-                    .file(urlEncodeParts(targetPath))
+                    .file(Utils.urlEncodeParts(targetPath))
                     .info();
             return !file.isFolder();
         } catch (Exception e) {
@@ -178,7 +176,7 @@ public long size(String targetPath) throws IOException {
         LOGGER.trace(String.format("Getting size for %s", targetPath));
         File file = artifactory
                 .repository(this.config.repository)
-                .file(urlEncodeParts(targetPath))
+                .file(Utils.urlEncodeParts(targetPath))
                 .info();
         return file.getSize();
     }
@@ -206,12 +204,6 @@ private Artifactory buildArtifactory() {
                 .build();
     }
 
-    private String urlEncodeParts(String s) {
-        return URLEncoder.encode(s, StandardCharsets.UTF_8)
-                .replaceAll("%2F", "/")
-                .replace("+", "%20");
-    }
-
     @Override
     public void close() throws Exception {
         artifactory.close();

src/main/java/io/jenkins/plugins/artifactory_artifacts/Utils.java

@@ -8,6 +8,8 @@
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.security.ACL;
 import hudson.util.DescribableList;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import java.util.Collections;
 import jenkins.model.ArtifactManagerConfiguration;
@@ -61,6 +63,10 @@ public static StandardUsernamePasswordCredentials getCredentials(ArtifactoryGene
         return getCredentials(config.getStorageCredentialId());
     }
 
+    static String urlEncodeParts(String s) {
+        return URLEncoder.encode(s, StandardCharsets.UTF_8).replace("%2F", "/").replace("+", "%20");
+    }
+
     /**
      * Get the URL of the artifact
      * @param name the name of the artifact
@@ -69,7 +75,7 @@ public static StandardUsernamePasswordCredentials getCredentials(ArtifactoryGene
     public static String getUrl(String name) {
         return String.format(
                 "%s/%s/%s",
-                getArtifactConfig().getServerUrl(), getArtifactConfig().getRepository(), name);
+                getArtifactConfig().getServerUrl(), getArtifactConfig().getRepository(), urlEncodeParts(name));
     }
 
     /**

src/test/java/io/jenkins/plugins/artifactory_artifacts/UtilsTest.java

@@ -19,6 +19,10 @@ public void shouldGetUrl(JenkinsRule jenkinsRule, WireMockRuntimeInfo wmRuntimeI
         assertThat(
                 Utils.getUrl("artifact.txt"),
                 is("http://localhost:" + wmRuntimeInfo.getHttpPort() + "/my-generic-repo/artifact.txt"));
+
+        assertThat(
+                Utils.getUrl("item#1.txt"),
+                is("http://localhost:" + wmRuntimeInfo.getHttpPort() + "/my-generic-repo/item%231.txt"));
     }
 
     @Test