Merge pull request #82 from chondent/fix-invalid-github-url

Parse and fix broken github api urls

Author: chondent

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

@@ -34,7 +34,7 @@
 @Value.Immutable
 @GithubStyle
 @JsonSerialize(as = ImmutableBranch.class)
-@JsonDeserialize(as = ImmutableBranch.class)
+@JsonDeserialize(as = ImmutableBranch.class, using = BranchDeserializer.class)
 public interface Branch {
 
   /** Branch name */
@@ -52,3 +52,4 @@ public interface Branch {
   /** Branch protection API URL */
   Optional<URI> protectionUrl();
 }
+

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

@@ -0,0 +1,76 @@
+/*-
+ * -\-\-
+ * github-api
+ * --
+ * Copyright (C) 2016 - 2020 Spotify AB
+ * --
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * -/-/-
+ */
+
+package com.spotify.github.v3.repos;
+
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JsonDeserializer;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.spotify.github.v3.git.ImmutableShaLink;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
+
+public class BranchDeserializer extends JsonDeserializer<ImmutableBranch> {
+
+  private URI fixInvalidGithubUrl(final String invalidUrl) throws IOException {
+    // There's a bug in github where it gives you back non-url-encoded characters
+    // in the protection_url field. For example if your branch has a single "%" in its name.
+    // As of this writing, the protection URL looks something like this
+    // https://github-server.tld/api/v3/repos/owner/repo-name/branches/branch-name/protection
+    final String[] schemaAndPath = invalidUrl.split("//");
+    String[] pathParts = schemaAndPath[1].split("/");
+    for (int i = 0; i < pathParts.length; i++) {
+      pathParts[i] = URLEncoder.encode(pathParts[i], StandardCharsets.UTF_8);
+    }
+    String fixedUrlString = schemaAndPath[0] + "//" + String.join("/", pathParts);
+    return URI.create(fixedUrlString);
+  }
+
+  @Override
+  public ImmutableBranch deserialize(final JsonParser jsonParser,
+      final DeserializationContext deserializationContext)
+      throws IOException {
+    JsonNode node = jsonParser.getCodec().readTree(jsonParser);
+    URI protectionUrl;
+    var immutableBranchBuilder = ImmutableBranch.builder();
+    if (node.has("protected")) {
+      immutableBranchBuilder.isProtected(node.get("protected").asBoolean());
+      String protectionUrlString = node.get("protection_url").asText();
+      try {
+        protectionUrl = new URI(protectionUrlString);
+      } catch (URISyntaxException e) {
+        protectionUrl = fixInvalidGithubUrl(protectionUrlString);
+      }
+      immutableBranchBuilder.protectionUrl(protectionUrl);
+    }
+    ImmutableShaLink shaLink = ImmutableShaLink.builder()
+        .sha(node.get("commit").get("sha").asText())
+        .url(URI.create(node.at("/commit/url").asText()))
+        .build();
+    return immutableBranchBuilder
+        .name(node.get("name").textValue())
+        .commit(shaLink)
+        .build();
+  }
+}

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

@@ -249,7 +249,52 @@ public void getBranch() throws Exception {
     when(github.request("/repos/someowner/somerepo/branches/somebranch", Branch.class))
         .thenReturn(fixture);
     final Branch branch = repoClient.getBranch("somebranch").get();
+    assertThat(branch.isProtected().orElse(false), is(true));
     assertThat(branch.commit().sha(), is("6dcb09b5b57875f334f61aebed695e2e4193db5e"));
+    assertThat(
+        branch.commit().url().toString(),
+        is("https://api.github.com/repos/octocat/Hello-World/commits/c5b97d5ae6c19d5c5df71a34c7fbeeda2479ccbc"));
+  }
+
+  @Test
+  public void getBranchWithoutProtection() throws Exception {
+    // Make sure the custom deserialiser correctly handles the optional protected fields
+    final CompletableFuture<Branch> fixture =
+        completedFuture(json.fromJson(getFixture("branch-not-protected.json"), Branch.class));
+    when(github.request("/repos/someowner/somerepo/branches/somebranch", Branch.class))
+        .thenReturn(fixture);
+    final Branch branch = repoClient.getBranch("somebranch").get();
+    assertThat(branch.isProtected().orElse(false), is(false));
+    assertThat(branch.commit().sha(), is("6dcb09b5b57875f334f61aebed695e2e4193db5e"));
+    assertThat(
+        branch.commit().url().toString(),
+        is("https://api.github.com/repos/octocat/Hello-World/commits/c5b97d5ae6c19d5c5df71a34c7fbeeda2479ccbc"));
+  }
+
+  @Test
+  public void getBranchWithCharactersIncorrectlyUnescapedByTheGithubApi() throws Exception {
+    final CompletableFuture<Branch> fixture =
+        completedFuture(json.fromJson(getFixture("branch-escape-chars.json"), Branch.class));
+    when(github.request("/repos/someowner/somerepo/branches/unescaped-percent-sign-%", Branch.class))
+        .thenReturn(fixture);
+    final Branch branch = repoClient.getBranch("unescaped-percent-sign-%").get();
+    assertThat(branch.commit().sha(), is("6dcb09b5b57875f334f61aebed695e2e4193db5e"));
+    assertThat(
+        branch.protectionUrl().get().toString(),
+        is("https://api.github.com/repos/octocat/Hello-World/branches/unescaped-percent-sign-%25/protection"));
+  }
+
+  @Test
+  public void getBranchWithCharactersIncorrectlyUnescapedByTheGithubApi_uriVariationTwo() throws Exception {
+    final CompletableFuture<Branch> fixture =
+        completedFuture(json.fromJson(getFixture("branch-escape-chars-url-variation-two.json"), Branch.class));
+    when(github.request("/repos/someowner/somerepo/branches/unescaped-percent-sign-%", Branch.class))
+        .thenReturn(fixture);
+    final Branch branch = repoClient.getBranch("unescaped-percent-sign-%").get();
+    assertThat(branch.commit().sha(), is("6dcb09b5b57875f334f61aebed695e2e4193db5e"));
+    assertThat(
+        branch.protectionUrl().get().toString(),
+        is("https://api.github.com/api/v3/repos/octocat/Hello-World/branches/branch-name-with-slashes/unescaped-percent-sign-%25/protection"));
   }
 
   @Test

src/test/resources/com/spotify/github/v3/repos/branch-escape-chars-url-variation-two.json

@@ -0,0 +1,9 @@
+{
+  "name": "unescaped-percent-sign-%",
+  "commit": {
+    "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e",
+    "url": "https://api.github.com/repos/octocat/Hello-World/commits/c5b97d5ae6c19d5c5df71a34c7fbeeda2479ccbc"
+  },
+  "protected": true,
+  "protection_url": "https://api.github.com/api/v3/repos/octocat/Hello-World/branches/branch-name-with-slashes/unescaped-percent-sign-%/protection"
+}

src/test/resources/com/spotify/github/v3/repos/branch-escape-chars.json

@@ -0,0 +1,9 @@
+{
+  "name": "unescaped-percent-sign-%",
+  "commit": {
+    "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e",
+    "url": "https://api.github.com/repos/octocat/Hello-World/commits/c5b97d5ae6c19d5c5df71a34c7fbeeda2479ccbc"
+  },
+  "protected": true,
+  "protection_url": "https://api.github.com/repos/octocat/Hello-World/branches/unescaped-percent-sign-%/protection"
+}

src/test/resources/com/spotify/github/v3/repos/branch-not-protected.json

@@ -0,0 +1,7 @@
+{
+  "name": "master",
+  "commit": {
+    "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e",
+    "url": "https://api.github.com/repos/octocat/Hello-World/commits/c5b97d5ae6c19d5c5df71a34c7fbeeda2479ccbc"
+  }
+}