Fixed GHContent to allow spaces in path

URI path encoding whack-a-mole. This fix should cover a significant number of cases. Also, fixed to NOT do URLEncode for URI path. The encoding is different between path and query. Fixes hub4j#624
bitwiseman · Nov 26, 2019 · dfa2ebb · dfa2ebb
1 parent e1df7bd
commit dfa2ebb
Show file tree

Hide file tree

Showing 59 changed files with 1,102 additions and 1,095 deletions.
diff --git a/src/main/java/org/kohsuke/github/GHContent.java b/src/main/java/org/kohsuke/github/GHContent.java
@@ -8,7 +8,6 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
-import java.net.URLEncoder;
 
 /**
  * A Content of a repository.
@@ -367,7 +366,7 @@ public GHContentUpdateResponse delete(String commitMessage, String branch) throw
     }
 
     static String getApiRoute(GHRepository repository, String path) {
-        return repository.getApiTailUrl("contents/" + Requester.urlEncode(path));
+        return repository.getApiTailUrl("contents/" + path);
     }
 
     GHContent wrap(GHRepository owner) {

diff --git a/src/main/java/org/kohsuke/github/GHContentBuilder.java b/src/main/java/org/kohsuke/github/GHContentBuilder.java
@@ -4,7 +4,6 @@
 
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
-import java.net.URLEncoder;
 
 /**
  * Used to create/update content.
@@ -109,8 +108,7 @@ public GHContentBuilder message(String commitMessage) {
      *             the io exception
      */
     public GHContentUpdateResponse commit() throws IOException {
-        GHContentUpdateResponse response = req.to(GHContent.getApiRoute(repo, path),
-                GHContentUpdateResponse.class);
+        GHContentUpdateResponse response = req.to(GHContent.getApiRoute(repo, path), GHContentUpdateResponse.class);
 
         response.getContent().wrap(repo);
         response.getCommit().wrapUp(repo);

diff --git a/src/main/java/org/kohsuke/github/GHRelease.java b/src/main/java/org/kohsuke/github/GHRelease.java
@@ -244,7 +244,7 @@ public GHAsset uploadAsset(String filename, InputStream stream, String contentTy
         String url = getUploadUrl();
         // strip the helpful garbage from the url
         url = url.substring(0, url.indexOf('{'));
-        url += "?name=" + URLEncoder.encode(filename, "UTF-8");
+        url += "?name=" + URLEncoder.encode(filename);
         return builder.contentType(contentType).with(stream).to(url, GHAsset.class).wrap(this);
     }
 

diff --git a/src/main/java/org/kohsuke/github/GHRepository.java b/src/main/java/org/kohsuke/github/GHRepository.java
@@ -34,9 +34,7 @@
 import java.io.InputStreamReader;
 import java.io.InterruptedIOException;
 import java.io.Reader;
-import java.io.UnsupportedEncodingException;
 import java.net.URL;
-import java.net.URLEncoder;
 import java.util.AbstractSet;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -51,8 +49,6 @@
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.WeakHashMap;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 import static java.util.Arrays.*;
 import static org.kohsuke.github.Previews.*;
@@ -1460,14 +1456,7 @@ public PagedIterable<GHRef> listRefs(String refType) throws IOException {
      *             on failure communicating with GitHub, potentially due to an invalid ref type being requested
      */
     public GHRef getRef(String refName) throws IOException {
-        // hashes in branch names must be replaced with the url encoded equivalent or this call will fail
-        // FIXME: how about other URL unsafe characters, like space, @, : etc? do we need to be using
-        // URLEncoder.encode()?
-        // OTOH, '/' need no escaping
-        refName = refName.replaceAll("#", "%23");
-        return root.retrieve()
-                .to(String.format("/repos/%s/%s/git/refs/%s", getOwnerName(), name, refName), GHRef.class)
-                .wrap(root);
+        return root.retrieve().to(getApiTailUrl(String.format("git/refs/%s", refName)), GHRef.class).wrap(root);
     }
 
     /**
@@ -2031,25 +2020,6 @@ public Map<String, GHBranch> getBranches() throws IOException {
         return r;
     }
 
-    /**
-     * Replace special characters (e.g. #) with standard values (e.g. %23) so GitHub understands what is being
-     * requested.
-     * 
-     * @param value
-     *            string to be encoded.
-     * @return The encoded string.
-     */
-    private String UrlEncode(String value) {
-        try {
-            return URLEncoder.encode(value, org.apache.commons.codec.CharEncoding.UTF_8);
-        } catch (UnsupportedEncodingException ex) {
-            Logger.getLogger(GHRepository.class.getName()).log(Level.SEVERE, null, ex);
-        }
-
-        // Something went wrong - just return original value as is.
-        return value;
-    }
-
     /**
      * Gets branch.
      *
@@ -2060,7 +2030,7 @@ private String UrlEncode(String value) {
      *             the io exception
      */
     public GHBranch getBranch(String name) throws IOException {
-        return root.retrieve().to(getApiTailUrl("branches/" + UrlEncode(name)), GHBranch.class).wrap(this);
+        return root.retrieve().to(getApiTailUrl("branches/" + name), GHBranch.class).wrap(this);
     }
 
     /**
@@ -2140,7 +2110,7 @@ public GHContent getFileContent(String path) throws IOException {
      */
     public GHContent getFileContent(String path, String ref) throws IOException {
         Requester requester = root.retrieve();
-        String target = GHContent.getApiRoute(this, path);
+        String target = getApiTailUrl("contents/" + path);
 
         return requester.with("ref", ref).to(target, GHContent.class).wrap(this);
     }
@@ -2174,7 +2144,7 @@ public List<GHContent> getDirectoryContent(String path, String ref) throws IOExc
         while (path.endsWith("/")) {
             path = path.substring(0, path.length() - 1);
         }
-        String target = GHContent.getApiRoute(this, path);
+        String target = getApiTailUrl("contents/" + path);
 
         GHContent[] files = requester.with("ref", ref).to(target, GHContent[].class);
 
@@ -2576,7 +2546,7 @@ public boolean equals(Object obj) {
     String getApiTailUrl(String tail) {
         if (tail.length() > 0 && !tail.startsWith("/"))
             tail = '/' + tail;
-        return "/repos/" + getOwnerName() + "/" + name + tail;
+        return Requester.urlPathEncode("/repos/" + getOwnerName() + "/" + name + tail);
     }
 
     /**

diff --git a/src/main/java/org/kohsuke/github/Requester.java b/src/main/java/org/kohsuke/github/Requester.java
@@ -40,6 +40,8 @@
 import java.net.MalformedURLException;
 import java.net.ProtocolException;
 import java.net.SocketTimeoutException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.net.URLEncoder;
 import java.util.ArrayList;
@@ -52,7 +54,6 @@
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.function.Consumer;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -944,17 +945,16 @@ static String transformEnum(Enum en) {
     }
 
     /**
-     * Replace special characters (e.g. #) with standard values (e.g. %23) so GitHub understands what is being
-     * requested.
+     * Encode the path to url safe string.
      *
      * @param value
-     *            string to be encoded.
+     *            string to be path encoded.
      * @return The encoded string.
      */
-    public static String urlEncode(String value) {
+    public static String urlPathEncode(String value) {
         try {
-            return URLEncoder.encode(value, org.apache.commons.codec.CharEncoding.UTF_8);
-        } catch (UnsupportedEncodingException ex) {
+            return new URI(null, null, value, null, null).toString();
+        } catch (URISyntaxException ex) {
             throw new AssertionError(ex);
         }
     }

diff --git a/src/test/java/org/kohsuke/github/GHContentIntegrationTest.java b/src/test/java/org/kohsuke/github/GHContentIntegrationTest.java
@@ -19,23 +19,21 @@ public class GHContentIntegrationTest extends AbstractGitHubWireMockTest {
     private GHRepository repo;
 
     // file name with spaces and other chars
-    private final String createdDirectory = "test directory #50";
-    private final String createdFilename = createdDirectory + "/test file-to-create-#1.txt";
+    private final String createdDirectory = "test+directory #50";
+    private final String createdFilename = createdDirectory + "/test file-to+create-#1.txt";
 
     @Before
     @After
     public void cleanup() throws Exception {
         if (mockGitHub.isUseProxy()) {
             repo = gitHubBeforeAfter.getRepository("github-api-test-org/GHContentIntegrationTest");
-            GHContent content = null;
             try {
-                content = repo.getFileContent(createdFilename);
+                GHContent content = repo.getFileContent(createdFilename);
+                if (content != null) {
+                    content.delete("Cleanup");
+                }
             } catch (IOException e) {
             }
-            if (content != null) {
-                content.delete("Cleanup");
-            }
-
         }
     }
 
@@ -83,23 +81,25 @@ public void testCRUDContent() throws Exception {
                 createdFilename);
         GHContent createdContent = created.getContent();
 
-        assertThat(created, is(notNullValue()));
         assertNotNull(created.getCommit());
         assertNotNull(created.getContent());
         assertNotNull(createdContent.getContent());
+        assertThat(createdContent.getPath(), equalTo(createdFilename));
         assertEquals("this is an awesome file I created\n", createdContent.getContent());
 
         GHContent content = repo.getFileContent(createdFilename);
         assertThat(content, is(notNullValue()));
-        assertThat(content.getSha(), is(created.getContent().getSha()));
-        assertThat(content.getContent(), is(created.getContent().getContent()));
+        assertThat(content.getSha(), equalTo(createdContent.getSha()));
+        assertThat(content.getContent(), equalTo(createdContent.getContent()));
+        assertThat(content.getPath(), equalTo(createdContent.getPath()));
 
         List<GHContent> directoryContents = repo.getDirectoryContent(createdDirectory);
         assertThat(directoryContents, is(notNullValue()));
         assertThat(directoryContents.size(), equalTo(1));
         content = directoryContents.get(0);
         assertThat(content.getSha(), is(created.getContent().getSha()));
         assertThat(content.getContent(), is(created.getContent().getContent()));
+        assertThat(content.getPath(), equalTo(createdFilename));
 
         GHContentUpdateResponse updatedContentResponse = createdContent.update("this is some new content\n",
                 "Updated file for integration tests.");
@@ -121,7 +121,8 @@ public void testCRUDContent() throws Exception {
             repo.getFileContent(createdFilename);
             fail("Delete didn't work!");
         } catch (GHFileNotFoundException e) {
-            assertThat(e.getMessage(), equalTo("{\"message\":\"Not Found\",\"documentation_url\":\"https://developer.github.com/v3/repos/contents/#get-contents\"}"));
+            assertThat(e.getMessage(),
+                    equalTo("{\"message\":\"Not Found\",\"documentation_url\":\"https://developer.github.com/v3/repos/contents/#get-contents\"}"));
         }
     }
 }
diff --git a/src/test/java/org/kohsuke/github/GHRepositoryTest.java b/src/test/java/org/kohsuke/github/GHRepositoryTest.java
@@ -65,7 +65,7 @@ public void getBranchNonExistentBut200Status() throws Exception {
             assertThat(e.getMessage(),
                     equalTo("Server returned HTTP response code: 200, message: 'OK' for URL: "
                             + mockGitHub.apiServer().baseUrl()
-                            + "/repos/github-api-test-org/github-api/branches/test%2FNonExistent"));
+                            + "/repos/github-api-test-org/github-api/branches/test/NonExistent"));
         }
     }
 

diff --git a/...7364792e-6ad0-4252-bc13-30085789bd98.json → ...e5080341-7377-4f3f-9427-388a105a91a7.json b/...7364792e-6ad0-4252-bc13-30085789bd98.json → ...e5080341-7377-4f3f-9427-388a105a91a7.json
@@ -65,14 +65,14 @@
   "releases_url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/releases{/id}",
   "deployments_url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/deployments",
   "created_at": "2015-08-15T14:14:57Z",
-  "updated_at": "2019-11-25T23:03:31Z",
-  "pushed_at": "2019-11-25T23:03:30Z",
+  "updated_at": "2019-11-26T01:09:15Z",
+  "pushed_at": "2019-11-26T01:09:14Z",
   "git_url": "git://github.com/github-api-test-org/GHContentIntegrationTest.git",
   "ssh_url": "[email protected]:github-api-test-org/GHContentIntegrationTest.git",
   "clone_url": "https://github.com/github-api-test-org/GHContentIntegrationTest.git",
   "svn_url": "https://github.com/github-api-test-org/GHContentIntegrationTest",
   "homepage": null,
-  "size": 47,
+  "size": 45,
   "stargazers_count": 1,
   "watchers_count": 1,
   "language": null,

diff --git a/...4122f675-92a6-4334-b47d-d09aebf2ec74.json → ...3cdae02b-77ed-4fb7-96fa-fee8aab5e88d.json b/...4122f675-92a6-4334-b47d-d09aebf2ec74.json → ...3cdae02b-77ed-4fb7-96fa-fee8aab5e88d.json
@@ -1,18 +1,18 @@
 [
   {
-    "name": "test+file-to-create-#1.txt",
-    "path": "test+directory+#50/test+file-to-create-#1.txt",
+    "name": "test file-to+create-#1.txt",
+    "path": "test+directory #50/test file-to+create-#1.txt",
     "sha": "8db9c31d79dfb9d0411c7af11b7ec7fabc72c5b1",
     "size": 34,
-    "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%2B%2350/test%2Bfile-to-create-%231.txt?ref=master",
-    "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%2B%2350/test%2Bfile-to-create-%231.txt",
+    "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt?ref=master",
+    "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt",
     "git_url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/blobs/8db9c31d79dfb9d0411c7af11b7ec7fabc72c5b1",
-    "download_url": "https://raw.githubusercontent.com/github-api-test-org/GHContentIntegrationTest/master/test%2Bdirectory%2B%2350/test%2Bfile-to-create-%231.txt",
+    "download_url": "https://raw.githubusercontent.com/github-api-test-org/GHContentIntegrationTest/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt",
     "type": "file",
     "_links": {
-      "self": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%2B%2350/test%2Bfile-to-create-%231.txt?ref=master",
+      "self": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt?ref=master",
       "git": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/blobs/8db9c31d79dfb9d0411c7af11b7ec7fabc72c5b1",
-      "html": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%2B%2350/test%2Bfile-to-create-%231.txt"
+      "html": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt"
     }
   }
 ]
diff --git a/...f7e75a2a-262e-4729-a4fb-139e88f4a5f6.json → ...04f85e50-583c-48ad-8d45-55368c542148.json b/...f7e75a2a-262e-4729-a4fb-139e88f4a5f6.json → ...04f85e50-583c-48ad-8d45-55368c542148.json
@@ -1,45 +1,45 @@
 {
   "content": {
-    "name": "test+file-to-create-#1.txt",
-    "path": "test+directory+#50/test+file-to-create-#1.txt",
+    "name": "test file-to+create-#1.txt",
+    "path": "test+directory #50/test file-to+create-#1.txt",
     "sha": "da2d3cc78776aec68881668775c46a53f0ee2288",
     "size": 25,
-    "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%2B%2350/test%2Bfile-to-create-%231.txt?ref=master",
-    "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%2B%2350/test%2Bfile-to-create-%231.txt",
+    "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt?ref=master",
+    "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt",
     "git_url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/blobs/da2d3cc78776aec68881668775c46a53f0ee2288",
-    "download_url": "https://raw.githubusercontent.com/github-api-test-org/GHContentIntegrationTest/master/test%2Bdirectory%2B%2350/test%2Bfile-to-create-%231.txt",
+    "download_url": "https://raw.githubusercontent.com/github-api-test-org/GHContentIntegrationTest/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt",
     "type": "file",
     "_links": {
-      "self": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%2B%2350/test%2Bfile-to-create-%231.txt?ref=master",
+      "self": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt?ref=master",
       "git": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/blobs/da2d3cc78776aec68881668775c46a53f0ee2288",
-      "html": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%2B%2350/test%2Bfile-to-create-%231.txt"
+      "html": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt"
     }
   },
   "commit": {
-    "sha": "d5c866c6fc1897a9cdb1b11a3c9ab6505884a442",
-    "node_id": "MDY6Q29tbWl0NDA3NjM1Nzc6ZDVjODY2YzZmYzE4OTdhOWNkYjFiMTFhM2M5YWI2NTA1ODg0YTQ0Mg==",
-    "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/commits/d5c866c6fc1897a9cdb1b11a3c9ab6505884a442",
-    "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/commit/d5c866c6fc1897a9cdb1b11a3c9ab6505884a442",
+    "sha": "00c7107a9d00cfd9116747ce61c91f2fd9a7acbc",
+    "node_id": "MDY6Q29tbWl0NDA3NjM1Nzc6MDBjNzEwN2E5ZDAwY2ZkOTExNjc0N2NlNjFjOTFmMmZkOWE3YWNiYw==",
+    "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/commits/00c7107a9d00cfd9116747ce61c91f2fd9a7acbc",
+    "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/commit/00c7107a9d00cfd9116747ce61c91f2fd9a7acbc",
     "author": {
       "name": "Liam Newman",
       "email": "[email protected]",
-      "date": "2019-11-25T23:03:56Z"
+      "date": "2019-11-26T01:09:46Z"
     },
     "committer": {
       "name": "Liam Newman",
       "email": "[email protected]",
-      "date": "2019-11-25T23:03:56Z"
+      "date": "2019-11-26T01:09:46Z"
     },
     "tree": {
-      "sha": "8269cd7cca651b2d0f1c649a16cbd3e67809075a",
-      "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/trees/8269cd7cca651b2d0f1c649a16cbd3e67809075a"
+      "sha": "50e945108f168faeb2eaf15855904e65e70b9336",
+      "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/trees/50e945108f168faeb2eaf15855904e65e70b9336"
     },
     "message": "Updated file for integration tests.",
     "parents": [
       {
-        "sha": "c9f5bfb199be85c604bfecbbf8f252b41bdd4a59",
-        "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/commits/c9f5bfb199be85c604bfecbbf8f252b41bdd4a59",
-        "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/commit/c9f5bfb199be85c604bfecbbf8f252b41bdd4a59"
+        "sha": "ce407cde9c6e4c970e33a955197512a2c1922ecf",
+        "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/commits/ce407cde9c6e4c970e33a955197512a2c1922ecf",
+        "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/commit/ce407cde9c6e4c970e33a955197512a2c1922ecf"
       }
     ],
     "verification": {