Fix HttpClientGitHubConnector to use redirects

Fixes hub4j#1305
bitwiseman · Nov 21, 2021 · 0d59ea5 · 0d59ea5
1 parent 8aeacf3
commit 0d59ea5
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 21 deletions.
diff --git a/pom.xml b/pom.xml
@@ -287,8 +287,8 @@
         <artifactId>maven-compiler-plugin</artifactId>
         <version>3.8.1</version>
         <configuration>
-          <source>1.8</source>
-          <target>1.8</target>
+          <source>11</source>
+          <target>11</target>
           <annotationProcessorPaths>
             <annotationProcessorPath>
               <groupId>org.jenkins-ci</groupId>

diff --git a/src/main/java/org/kohsuke/github/GitHubClient.java b/src/main/java/org/kohsuke/github/GitHubClient.java
@@ -25,8 +25,7 @@
 
 import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY;
 import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE;
-import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
-import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED;
+import static java.net.HttpURLConnection.*;
 import static java.util.logging.Level.*;
 import static org.apache.commons.lang3.StringUtils.defaultString;
 
@@ -413,6 +412,7 @@ private void detectKnownErrors(GitHubConnectorResponse connectorResponse,
             boolean detectStatusCodeError) throws IOException {
         detectOTPRequired(connectorResponse);
         detectInvalidCached404Response(connectorResponse, request);
+        detectRedirect(connectorResponse);
         if (rateLimitHandler.isError(connectorResponse)) {
             rateLimitHandler.onError(connectorResponse);
             throw new RetryRequestException();
@@ -425,6 +425,19 @@ private void detectKnownErrors(GitHubConnectorResponse connectorResponse,
         }
     }
 
+    private void detectRedirect(GitHubConnectorResponse connectorResponse) throws IOException {
+        if (connectorResponse.statusCode() == HTTP_MOVED_PERM || connectorResponse.statusCode() == HTTP_MOVED_TEMP) {
+            // GitHubClient depends on GitHubConnector implementations to follow any redirects automatically
+            // If this is not done and a redirect is requested, throw in order to maintain security and consistency
+            throw new HttpException(
+                    "GitHubConnnector did not automatically follow redirect.\n"
+                            + "Change your http client configuration to automatically follow redirects as appropriate.",
+                    connectorResponse.statusCode(),
+                    "Redirect",
+                    connectorResponse.request().url().toString());
+        }
+    }
+
     private GitHubConnectorRequest prepareConnectorRequest(GitHubRequest request) throws IOException {
         GitHubRequest.Builder<?> builder = request.toBuilder();
         // if the authentication is needed but no credential is given, try it anyway (so that some calls

diff --git a/src/main/java11/org/kohsuke/github/extras/HttpClientGitHubConnector.java b/src/main/java11/org/kohsuke/github/extras/HttpClientGitHubConnector.java
@@ -28,10 +28,10 @@ public class HttpClientGitHubConnector implements GitHubConnector {
     private final HttpClient client;
 
     /**
-     * Instantiates a new HttpClientGitHubConnector with a defaut HttpClient.
+     * Instantiates a new HttpClientGitHubConnector with a default HttpClient.
      */
     public HttpClientGitHubConnector() {
-        this(HttpClient.newHttpClient());
+        this(HttpClient.newBuilder().followRedirects(HttpClient.Redirect.NORMAL).build());
     }
 
     /**

diff --git a/src/test/java/org/kohsuke/github/GHWorkflowRunTest.java b/src/test/java/org/kohsuke/github/GHWorkflowRunTest.java
@@ -1,15 +1,12 @@
 package org.kohsuke.github;
 
 import org.awaitility.Awaitility;
-import org.junit.Assume;
 import org.junit.Before;
 import org.junit.Test;
 import org.kohsuke.github.GHWorkflowJob.Step;
 import org.kohsuke.github.GHWorkflowRun.Conclusion;
 import org.kohsuke.github.GHWorkflowRun.Status;
-import org.kohsuke.github.extras.HttpClientGitHubConnector;
 import org.kohsuke.github.function.InputStreamFunction;
-import org.kohsuke.github.internal.DefaultGitHubConnector;
 
 import java.io.IOException;
 import java.time.Duration;
@@ -201,10 +198,6 @@ public void testSearchOnBranch() throws IOException {
 
     @Test
     public void testLogs() throws IOException {
-        // This test fails for HttpClientGitHubConnector.
-        // Not sure why, but it is better to move forward and come back to it later
-        Assume.assumeFalse(DefaultGitHubConnector.create() instanceof HttpClientGitHubConnector);
-
         GHWorkflow workflow = repo.getWorkflow(FAST_WORKFLOW_PATH);
 
         long latestPreexistingWorkflowRunId = getLatestPreexistingWorkflowRunId();
@@ -243,10 +236,6 @@ public void testLogs() throws IOException {
     @SuppressWarnings("resource")
     @Test
     public void testArtifacts() throws IOException {
-        // This test fails for HttpClientGitHubConnector.
-        // Not sure why, but it is better to move forward and come back to it later
-        Assume.assumeFalse(DefaultGitHubConnector.create() instanceof HttpClientGitHubConnector);
-
         GHWorkflow workflow = repo.getWorkflow(ARTIFACTS_WORKFLOW_PATH);
 
         long latestPreexistingWorkflowRunId = getLatestPreexistingWorkflowRunId();
@@ -327,10 +316,6 @@ public void testArtifacts() throws IOException {
 
     @Test
     public void testJobs() throws IOException {
-        // This test fails for HttpClientGitHubConnector.
-        // Not sure why, but it is better to move forward and come back to it later
-        Assume.assumeFalse(DefaultGitHubConnector.create() instanceof HttpClientGitHubConnector);
-
         GHWorkflow workflow = repo.getWorkflow(MULTI_JOBS_WORKFLOW_PATH);
 
         long latestPreexistingWorkflowRunId = getLatestPreexistingWorkflowRunId();