Skip to content

Commit

Permalink
Fixed GHContent to allow spaces in path
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bitwiseman committed Nov 26, 2019
1 parent 7d4f194 commit 05cbf36
Show file tree
Hide file tree
Showing 60 changed files with 1,376 additions and 1,075 deletions.
8 changes: 4 additions & 4 deletions src/main/java/org/kohsuke/github/GHContent.java
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ public GHContentUpdateResponse update(byte[] newContentBytes, String commitMessa
requester.with("branch", branch);
}

GHContentUpdateResponse response = requester.to(getApiRoute(), GHContentUpdateResponse.class);
GHContentUpdateResponse response = requester.to(getApiRoute(repository, path), GHContentUpdateResponse.class);

response.getContent().wrap(repository);
response.getCommit().wrapUp(repository);
Expand Down Expand Up @@ -359,14 +359,14 @@ public GHContentUpdateResponse delete(String commitMessage, String branch) throw
requester.with("branch", branch);
}

GHContentUpdateResponse response = requester.to(getApiRoute(), GHContentUpdateResponse.class);
GHContentUpdateResponse response = requester.to(getApiRoute(repository, path), GHContentUpdateResponse.class);

response.getCommit().wrapUp(repository);
return response;
}

private String getApiRoute() {
return "/repos/" + repository.getOwnerName() + "/" + repository.getName() + "/contents/" + path;
static String getApiRoute(GHRepository repository, String path) {
return repository.getApiTailUrl("contents/" + path);
}

GHContent wrap(GHRepository owner) {
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/org/kohsuke/github/GHContentBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,7 @@ public GHContentBuilder message(String commitMessage) {
* the io exception
*/
public GHContentUpdateResponse commit() throws IOException {
GHContentUpdateResponse response = req.to(repo.getApiTailUrl("contents/" + path),
GHContentUpdateResponse.class);
GHContentUpdateResponse response = req.to(GHContent.getApiRoute(repo, path), GHContentUpdateResponse.class);

response.getContent().wrap(repo);
response.getCommit().wrapUp(repo);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/kohsuke/github/GHRelease.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
36 changes: 3 additions & 33 deletions src/main/java/org/kohsuke/github/GHRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.*;
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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.
*
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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);
}

/**
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/org/kohsuke/github/Requester.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -942,6 +944,21 @@ static String transformEnum(Enum en) {
return en.toString().toLowerCase(Locale.ENGLISH).replace('_', '-');
}

/**
* Encode the path to url safe string.
*
* @param value
* string to be path encoded.
* @return The encoded string.
*/
public static String urlPathEncode(String value) {
try {
return new URI(null, null, value, null, null).toString();
} catch (URISyntaxException ex) {
throw new AssertionError(ex);
}
}

private static final List<String> METHODS_WITHOUT_BODY = asList("GET", "DELETE");
private static final Logger LOGGER = Logger.getLogger(Requester.class.getName());
}
30 changes: 29 additions & 1 deletion src/test/java/org/kohsuke/github/GHContentIntegrationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,18 @@
import java.io.InputStreamReader;
import java.util.List;

import static org.hamcrest.CoreMatchers.*;

/**
* Integration test for {@link GHContent}.
*/
public class GHContentIntegrationTest extends AbstractGitHubWireMockTest {

private GHRepository repo;
private String createdFilename = "test-file-to-create.txt";

// 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";

@Before
@After
Expand Down Expand Up @@ -79,8 +84,23 @@ public void testCRUDContent() throws Exception {
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(), 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.");
GHContent updatedContent = updatedContentResponse.getContent();
Expand All @@ -96,5 +116,13 @@ public void testCRUDContent() throws Exception {

assertNotNull(deleteResponse.getCommit());
assertNull(deleteResponse.getContent());

try {
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\"}"));
}
}
}
2 changes: 1 addition & 1 deletion src/test/java/org/kohsuke/github/GHRepositoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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-10-11T22:17:07Z",
"pushed_at": "2019-10-11T22:17:06Z",
"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": 38,
"size": 45,
"stargazers_count": 1,
"watchers_count": 1,
"language": null,
Expand All @@ -81,13 +81,13 @@
"has_downloads": true,
"has_wiki": true,
"has_pages": false,
"forks_count": 40,
"forks_count": 41,
"mirror_url": null,
"archived": false,
"disabled": false,
"open_issues_count": 0,
"license": null,
"forks": 40,
"forks": 41,
"open_issues": 0,
"watchers": 1,
"default_branch": "master",
Expand Down Expand Up @@ -296,17 +296,17 @@
"has_downloads": true,
"has_wiki": true,
"has_pages": false,
"forks_count": 59,
"forks_count": 60,
"mirror_url": null,
"archived": false,
"disabled": false,
"open_issues_count": 0,
"license": null,
"forks": 59,
"forks": 60,
"open_issues": 0,
"watchers": 0,
"default_branch": "master"
},
"network_count": 59,
"network_count": 60,
"subscribers_count": 1
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
{
"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%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%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%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%20%2350/test%20file-to%2Bcreate-%231.txt"
}
}
]
Original file line number Diff line number Diff line change
@@ -1,45 +1,45 @@
{
"content": {
"name": "test-file-to-create.txt",
"path": "test-file-to-create.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-file-to-create.txt?ref=master",
"html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test-file-to-create.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-file-to-create.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-file-to-create.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-file-to-create.txt"
"html": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt"
}
},
"commit": {
"sha": "bc9cda986e02ae2677358a28f9cb6097865b3536",
"node_id": "MDY6Q29tbWl0NDA3NjM1Nzc6YmM5Y2RhOTg2ZTAyYWUyNjc3MzU4YTI4ZjljYjYwOTc4NjViMzUzNg==",
"url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/commits/bc9cda986e02ae2677358a28f9cb6097865b3536",
"html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/commit/bc9cda986e02ae2677358a28f9cb6097865b3536",
"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-10-11T22:17:43Z"
"date": "2019-11-26T01:09:46Z"
},
"committer": {
"name": "Liam Newman",
"email": "[email protected]",
"date": "2019-10-11T22:17:43Z"
"date": "2019-11-26T01:09:46Z"
},
"tree": {
"sha": "7aaee6a6cddcf48f94a40093de05be83c580ce10",
"url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/trees/7aaee6a6cddcf48f94a40093de05be83c580ce10"
"sha": "50e945108f168faeb2eaf15855904e65e70b9336",
"url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/trees/50e945108f168faeb2eaf15855904e65e70b9336"
},
"message": "Updated file for integration tests.",
"parents": [
{
"sha": "88bd1f52fc1e8242758364cee7dce2c82790d272",
"url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/commits/88bd1f52fc1e8242758364cee7dce2c82790d272",
"html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/commit/88bd1f52fc1e8242758364cee7dce2c82790d272"
"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": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
{
"name": "test-file-to-create.txt",
"path": "test-file-to-create.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-file-to-create.txt?ref=master",
"html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test-file-to-create.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-file-to-create.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",
"content": "dGhpcyBpcyBzb21lIG5ldyBjb250ZW50Cg==\n",
"encoding": "base64",
"_links": {
"self": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test-file-to-create.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-file-to-create.txt"
"html": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt"
}
}
Loading

0 comments on commit 05cbf36

Please sign in to comment.