Migrate to httpclient5

[strangelookingnerd]

Replaces outdated httpclient4 with the successor httpclient5.

Iam well aware that this is a quite large changeset however I hope that there is still interest in this PR and it will be reviewed. If there are any questions, please do not hesitate to ping me.

Testing done

mvn clean verify and some manual testing.

Submitter checklist

• [x] Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• [x] Ensure that the pull request title represents the desired changelog entry
• [x] Please describe what you did
• [x] Link to relevant issues in GitHub or Jira
• [x] Link to relevant pull requests, esp. upstream and downstream changes
• [x] Ensure you have provided tests - that demonstrates feature works or fixes the issue

[strangelookingnerd]

Jenkins Security Scan alerts are unrelated to these changes and should be adressed in a separate PR.

[gounthar]

It looks like the build failure has nothing to do with your code modification: 10:19:45 ExecutionException The forked VM terminated without properly saying goodbye. VM crash or System.exit called?

[strangelookingnerd]

@jenkinsci/http-request-plugin-developers Kindly requesting a review.

[MarkEWaite]

@jenkinsci/http-request-plugin-developers Kindly requesting a review.

I'm the only active maintainer of this plugin and I'm deeply involved in other activities that are higher priority for me and for Jenkins. It will likely be several months before I'm able to review this.

[dgonz782]

In v1.20 of this plugin, I was able to use GitHub App credentials. With this change in v1.21, I'm no longer able to do so:

19:37:38  HttpMethod: GET
19:37:38  URL: https://github.<ent>.com/api/v3/repos/<org>/<repo>/pulls/2755
19:37:38  Using authentication: GHES-Jenkins-App
19:37:38  Response Code: 404
Found unhandled java.lang.IllegalStateException exception:
hudson.AbortException: Fail: Status code 404 is not in the accepted range: 100:399 while calling https://github.<ent>.com/api/v3/repos/<org>/<repo>/pulls/2755
	PluginClassLoader for http_request//jenkins.plugins.http_request.HttpRequestExecution.call(HttpRequestExecution.java:294)
	PluginClassLoader for http_request//jenkins.plugins.http_request.HttpRequestStep$Execution.run(HttpRequestStep.java:408)
	PluginClassLoader for http_request//jenkins.plugins.http_request.HttpRequestStep$Execution.run(HttpRequestStep.java:384)
	PluginClassLoader for workflow-step-api//org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution.lambda$start$0(SynchronousNonBlockingStepExecution.java:49)
	java.base/java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source)
	java.base/java.util.concurrent.FutureTask.run(Unknown Source)
	java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
	java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
	java.base/java.lang.Thread.run(Unknown Source)

[MarkEWaite]

In v1.20 of this plugin, I was able to use GitHub App credentials.

@strangelookingnerd would you be willing to investigate this? I'd rather not revert back to httpclient 4 after all your good work to make the switch.

[strangelookingnerd]

@strangelookingnerd would you be willing to investigate this? I'd rather not revert back to httpclient 4 after all your good work to make the switch.

I can give it a shot.

@dgonz782 Would you be able to share some more details so I could create a reproducible test case? The pipeline script you are using or similar would help solving this issue faster.

[dgonz782]

@strangelookingnerd - turns out the 404 err was due to an extra / in the constructed api url from clone url. v1.20 auto-fixed this, but v1.21 doesn't. After removing the extra /, GH App cred works with this plugin. My apologies for the confusion.

def prData = httpRequest (
   httpMode: 'GET',
   authentication: config.gitHttpsCredential,
-  url: getRepoApiUrl(config.repos.<repo>, "/pulls/${env.CHANGE_ID}")
+  url: getRepoApiUrl(config.repos.<repo>, "pulls/${env.CHANGE_ID}")
)

A url with // worked in v1.20, but doesn't in v1.21: url: https://HOSTNAME/api/v3/repos/OWNER/REPO//pulls/1234

[strangelookingnerd]

According to RFC2396 a double-slash is not a allowed in the path section of a URI. The //sequence has a defined meaning in the URI, it denotes the authority component (server). See RFC2396, sections 3.2, 3.4).

It appears httpclient4 has had a different default behavior when it comes to normalizing URI compared to httpclient5. While I assume that there are ways to workaround and restore the old behavior, I don't think that it would be correct from a technical standpoint.

@MarkEWaite WDYT?

[MarkEWaite]

there are ways to workaround and restore the old behavior, I don't think that it would be correct from a technical standpoint.

I agree. I think that we should accept that this new behavior is the more correct behavior.