Skip to content

Commit 22bce81

Browse files
committed
Parse and fix broken github api urls
If a branch has a percent symbol in it, github will sometimes return a URI that java's URI object can't parse. This patch ensures that in such a case we url-escape the branch name.
1 parent dbabfc3 commit 22bce81

File tree

5 files changed

+123
-1
lines changed

5 files changed

+123
-1
lines changed

src/main/java/com/spotify/github/v3/repos/Branch.java

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,20 @@
2121
package com.spotify.github.v3.repos;
2222

2323
import com.fasterxml.jackson.annotation.JsonProperty;
24+
import com.fasterxml.jackson.core.JsonParser;
25+
import com.fasterxml.jackson.databind.DeserializationContext;
26+
import com.fasterxml.jackson.databind.JsonDeserializer;
27+
import com.fasterxml.jackson.databind.JsonNode;
2428
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
2529
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
2630
import com.spotify.github.GithubStyle;
31+
import com.spotify.github.v3.git.ImmutableShaLink;
2732
import com.spotify.github.v3.git.ShaLink;
33+
import java.io.IOException;
2834
import java.net.URI;
35+
import java.net.URISyntaxException;
36+
import java.net.URLEncoder;
37+
import java.nio.charset.StandardCharsets;
2938
import java.util.Optional;
3039
import javax.annotation.Nullable;
3140
import org.immutables.value.Value;
@@ -34,7 +43,7 @@
3443
@Value.Immutable
3544
@GithubStyle
3645
@JsonSerialize(as = ImmutableBranch.class)
37-
@JsonDeserialize(as = ImmutableBranch.class)
46+
@JsonDeserialize(as = ImmutableBranch.class, using = BranchDeserializer.class)
3847
public interface Branch {
3948

4049
/** Branch name */
@@ -52,3 +61,46 @@ public interface Branch {
5261
/** Branch protection API URL */
5362
Optional<URI> protectionUrl();
5463
}
64+
65+
class BranchDeserializer extends JsonDeserializer<ImmutableBranch> {
66+
67+
private URI fixInvalidGithubUrl(final String invalidUrl) throws IOException {
68+
// There's a bug in github where it gives you back non-url-encoded characters
69+
// in the protection_url field. For example if your branch has a single "%" in its name.
70+
// As of this writing, the protection URL looks something like this
71+
// https://github-server.tld/api/v3/repos/owner/repo-name/branches/branch-name/protection
72+
final String[] schemaAndPath = invalidUrl.split("//");
73+
String[] pathParts = schemaAndPath[1].split("/");
74+
for (int i = 0; i < pathParts.length; i++) {
75+
pathParts[i] = URLEncoder.encode(pathParts[i], StandardCharsets.UTF_8);
76+
}
77+
String fixedUrlString = schemaAndPath[0] + "//" + String.join("/", pathParts);
78+
return URI.create(fixedUrlString);
79+
}
80+
81+
@Override
82+
public ImmutableBranch deserialize(final JsonParser jsonParser, final DeserializationContext deserializationContext)
83+
throws IOException {
84+
JsonNode node = jsonParser.getCodec().readTree(jsonParser);
85+
URI protectionUrl;
86+
var immutableBranchBuilder = ImmutableBranch.builder();
87+
if (node.has("protected")) {
88+
immutableBranchBuilder.isProtected(node.get("protected").asBoolean());
89+
String protectionUrlString = node.get("protection_url").asText();
90+
try {
91+
protectionUrl = new URI(protectionUrlString);
92+
} catch (URISyntaxException e) {
93+
protectionUrl = fixInvalidGithubUrl(protectionUrlString);
94+
}
95+
immutableBranchBuilder.protectionUrl(protectionUrl);
96+
}
97+
ImmutableShaLink shaLink = ImmutableShaLink.builder()
98+
.sha(node.get("commit").get("sha").asText())
99+
.url(URI.create(node.at("/commit/url").asText()))
100+
.build();
101+
return immutableBranchBuilder
102+
.name(node.get("name").textValue())
103+
.commit(shaLink)
104+
.build();
105+
}
106+
}

src/test/java/com/spotify/github/v3/clients/RepositoryClientTest.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,52 @@ public void getBranch() throws Exception {
249249
when(github.request("/repos/someowner/somerepo/branches/somebranch", Branch.class))
250250
.thenReturn(fixture);
251251
final Branch branch = repoClient.getBranch("somebranch").get();
252+
assertThat(branch.isProtected().orElse(false), is(true));
252253
assertThat(branch.commit().sha(), is("6dcb09b5b57875f334f61aebed695e2e4193db5e"));
254+
assertThat(
255+
branch.commit().url().toString(),
256+
is("https://api.github.com/repos/octocat/Hello-World/commits/c5b97d5ae6c19d5c5df71a34c7fbeeda2479ccbc"));
257+
}
258+
259+
@Test
260+
public void getBranchWithoutProtection() throws Exception {
261+
// Make sure the custom deserialiser correctly handles the optional protected fields
262+
final CompletableFuture<Branch> fixture =
263+
completedFuture(json.fromJson(getFixture("branch-not-protected.json"), Branch.class));
264+
when(github.request("/repos/someowner/somerepo/branches/somebranch", Branch.class))
265+
.thenReturn(fixture);
266+
final Branch branch = repoClient.getBranch("somebranch").get();
267+
assertThat(branch.isProtected().orElse(false), is(false));
268+
assertThat(branch.commit().sha(), is("6dcb09b5b57875f334f61aebed695e2e4193db5e"));
269+
assertThat(
270+
branch.commit().url().toString(),
271+
is("https://api.github.com/repos/octocat/Hello-World/commits/c5b97d5ae6c19d5c5df71a34c7fbeeda2479ccbc"));
272+
}
273+
274+
@Test
275+
public void getBranchWithCharactersIncorrectlyUnescapedByTheGithubApi() throws Exception {
276+
final CompletableFuture<Branch> fixture =
277+
completedFuture(json.fromJson(getFixture("branch-escape-chars.json"), Branch.class));
278+
when(github.request("/repos/someowner/somerepo/branches/unescaped-percent-sign-%", Branch.class))
279+
.thenReturn(fixture);
280+
final Branch branch = repoClient.getBranch("unescaped-percent-sign-%").get();
281+
assertThat(branch.commit().sha(), is("6dcb09b5b57875f334f61aebed695e2e4193db5e"));
282+
assertThat(
283+
branch.protectionUrl().get().toString(),
284+
is("https://api.github.com/repos/octocat/Hello-World/branches/unescaped-percent-sign-%25/protection"));
285+
}
286+
287+
@Test
288+
public void getBranchWithCharactersIncorrectlyUnescapedByTheGithubApi_uriVariationTwo() throws Exception {
289+
final CompletableFuture<Branch> fixture =
290+
completedFuture(json.fromJson(getFixture("branch-escape-chars-url-variation-two.json"), Branch.class));
291+
when(github.request("/repos/someowner/somerepo/branches/unescaped-percent-sign-%", Branch.class))
292+
.thenReturn(fixture);
293+
final Branch branch = repoClient.getBranch("unescaped-percent-sign-%").get();
294+
assertThat(branch.commit().sha(), is("6dcb09b5b57875f334f61aebed695e2e4193db5e"));
295+
assertThat(
296+
branch.protectionUrl().get().toString(),
297+
is("https://api.github.com/api/v3/repos/octocat/Hello-World/branches/branch-name-with-slashes/unescaped-percent-sign-%25/protection"));
253298
}
254299

255300
@Test
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"name": "unescaped-percent-sign-%",
3+
"commit": {
4+
"sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e",
5+
"url": "https://api.github.com/repos/octocat/Hello-World/commits/c5b97d5ae6c19d5c5df71a34c7fbeeda2479ccbc"
6+
},
7+
"protected": true,
8+
"protection_url": "https://api.github.com/api/v3/repos/octocat/Hello-World/branches/branch-name-with-slashes/unescaped-percent-sign-%/protection"
9+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"name": "unescaped-percent-sign-%",
3+
"commit": {
4+
"sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e",
5+
"url": "https://api.github.com/repos/octocat/Hello-World/commits/c5b97d5ae6c19d5c5df71a34c7fbeeda2479ccbc"
6+
},
7+
"protected": true,
8+
"protection_url": "https://api.github.com/repos/octocat/Hello-World/branches/unescaped-percent-sign-%/protection"
9+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "master",
3+
"commit": {
4+
"sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e",
5+
"url": "https://api.github.com/repos/octocat/Hello-World/commits/c5b97d5ae6c19d5c5df71a34c7fbeeda2479ccbc"
6+
}
7+
}

0 commit comments

Comments
 (0)