Skip to content

Commit 988f603

Browse files
committed
Fix HttpClientGitHubConnector to use redirects
Fixes hub4j#1305
1 parent 65d6de2 commit 988f603

File tree

3 files changed

+17
-19
lines changed

3 files changed

+17
-19
lines changed

src/main/java/org/kohsuke/github/GitHubClient.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@
2525

2626
import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY;
2727
import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE;
28-
import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
29-
import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED;
28+
import static java.net.HttpURLConnection.*;
3029
import static java.util.logging.Level.*;
3130
import static org.apache.commons.lang3.StringUtils.defaultString;
3231

@@ -413,6 +412,7 @@ private void detectKnownErrors(GitHubConnectorResponse connectorResponse,
413412
boolean detectStatusCodeError) throws IOException {
414413
detectOTPRequired(connectorResponse);
415414
detectInvalidCached404Response(connectorResponse, request);
415+
detectRedirect(connectorResponse);
416416
if (rateLimitHandler.isError(connectorResponse)) {
417417
rateLimitHandler.onError(connectorResponse);
418418
throw new RetryRequestException();
@@ -425,6 +425,19 @@ private void detectKnownErrors(GitHubConnectorResponse connectorResponse,
425425
}
426426
}
427427

428+
private void detectRedirect(GitHubConnectorResponse connectorResponse) throws IOException {
429+
if (connectorResponse.statusCode() == HTTP_MOVED_PERM || connectorResponse.statusCode() == HTTP_MOVED_TEMP) {
430+
// GitHubClient depends on GitHubConnector implementations to follow any redirects automatically
431+
// If this is not done and a redirect is requested, throw in order to maintain security and consistency
432+
throw new HttpException(
433+
"GitHubConnnector did not automatically follow redirect.\n"
434+
+ "Change your http client configuration to automatically follow redirects as appropriate.",
435+
connectorResponse.statusCode(),
436+
"Redirect",
437+
connectorResponse.request().url().toString());
438+
}
439+
}
440+
428441
private GitHubConnectorRequest prepareConnectorRequest(GitHubRequest request) throws IOException {
429442
GitHubRequest.Builder<?> builder = request.toBuilder();
430443
// if the authentication is needed but no credential is given, try it anyway (so that some calls

src/main/java11/org/kohsuke/github/extras/HttpClientGitHubConnector.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ public class HttpClientGitHubConnector implements GitHubConnector {
2828
private final HttpClient client;
2929

3030
/**
31-
* Instantiates a new HttpClientGitHubConnector with a defaut HttpClient.
31+
* Instantiates a new HttpClientGitHubConnector with a default HttpClient.
3232
*/
3333
public HttpClientGitHubConnector() {
34-
this(HttpClient.newHttpClient());
34+
this(HttpClient.newBuilder().followRedirects(HttpClient.Redirect.NORMAL).build());
3535
}
3636

3737
/**

src/test/java/org/kohsuke/github/GHWorkflowRunTest.java

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
11
package org.kohsuke.github;
22

33
import org.awaitility.Awaitility;
4-
import org.junit.Assume;
54
import org.junit.Before;
65
import org.junit.Test;
76
import org.kohsuke.github.GHWorkflowJob.Step;
87
import org.kohsuke.github.GHWorkflowRun.Conclusion;
98
import org.kohsuke.github.GHWorkflowRun.Status;
10-
import org.kohsuke.github.extras.HttpClientGitHubConnector;
119
import org.kohsuke.github.function.InputStreamFunction;
12-
import org.kohsuke.github.internal.DefaultGitHubConnector;
1310

1411
import java.io.IOException;
1512
import java.time.Duration;
@@ -201,10 +198,6 @@ public void testSearchOnBranch() throws IOException {
201198

202199
@Test
203200
public void testLogs() throws IOException {
204-
// This test fails for HttpClientGitHubConnector.
205-
// Not sure why, but it is better to move forward and come back to it later
206-
Assume.assumeFalse(DefaultGitHubConnector.create() instanceof HttpClientGitHubConnector);
207-
208201
GHWorkflow workflow = repo.getWorkflow(FAST_WORKFLOW_PATH);
209202

210203
long latestPreexistingWorkflowRunId = getLatestPreexistingWorkflowRunId();
@@ -243,10 +236,6 @@ public void testLogs() throws IOException {
243236
@SuppressWarnings("resource")
244237
@Test
245238
public void testArtifacts() throws IOException {
246-
// This test fails for HttpClientGitHubConnector.
247-
// Not sure why, but it is better to move forward and come back to it later
248-
Assume.assumeFalse(DefaultGitHubConnector.create() instanceof HttpClientGitHubConnector);
249-
250239
GHWorkflow workflow = repo.getWorkflow(ARTIFACTS_WORKFLOW_PATH);
251240

252241
long latestPreexistingWorkflowRunId = getLatestPreexistingWorkflowRunId();
@@ -327,10 +316,6 @@ public void testArtifacts() throws IOException {
327316

328317
@Test
329318
public void testJobs() throws IOException {
330-
// This test fails for HttpClientGitHubConnector.
331-
// Not sure why, but it is better to move forward and come back to it later
332-
Assume.assumeFalse(DefaultGitHubConnector.create() instanceof HttpClientGitHubConnector);
333-
334319
GHWorkflow workflow = repo.getWorkflow(MULTI_JOBS_WORKFLOW_PATH);
335320

336321
long latestPreexistingWorkflowRunId = getLatestPreexistingWorkflowRunId();

0 commit comments

Comments
 (0)