Illegal Reflective Access Operation to field java.net.HttpURLConnection.method

[andrewlalis]

Describe the bug
I attempt to archive a repository using the following code:

GHRepository repo = this.org.getRepository("repo-name-here");
repo.archive();

When checking GitHub itself, the repository is in fact archived, however I receive a warning in the stderr stream consisting of the following:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.kohsuke.github.GitHubHttpUrlConnectionClient$HttpURLConnectionResponseInfo (file:/C:/Users/andrew/.m2/repository/org/kohsuke/github-api/1.108/github-api-1.108.jar) to field java.net.HttpURLConnection.method
WARNING: Please consider reporting this to the maintainers of org.kohsuke.github.GitHubHttpUrlConnectionClient$HttpURLConnectionResponseInfo
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

To Reproduce
Steps to reproduce the behavior:

1. Fetch a GHRepository that is not yet archived.
2. Call archive() on that repository.

Expected behavior
I expect that there should not be any illegal reflective access exceptions.

Desktop (please complete the following information):

• OS: Windows 10
• Browser Firefox
• Version 74

[bitwiseman]

This is not an exception, as you noted your action succeeded.

Unfortunately, this can't be directly fixed. If you use the JDK version of HTTPUrlConnection, it only allows a specific set of verbs, so we must use reflection to override that.
https://github.com/github-api/github-api/blob/4c30f94355dfca7034f6619f64543ee3e9fcafdf/src/main/java/org/kohsuke/github/GitHubHttpUrlConnectionClient.java#L162-L191

If you switch to another implementation such as OkHttp3, this should go away. Also, if you use Java 8 you will not see this warning.

"All illegal access operations will be denied in a future release", but as of Java 13 that has not happened yet.

[lotharschulz]

I experience the same behavior using kotlin with

repository.setDefaultBranch("some-branch")

---

"All illegal access operations will be denied in a future release", but as of Java 13 that has not happened yet.

I experience the same with Java 14 ( jvm target set in build.gradle.kts: kotlinOptions.jvmTarget = "14")

[bitwiseman]

@lotharschulz
There is nothing to be done about this as long as this library uses HttpURLConnection.
Have you tried using OkHttp3?

[lotharschulz]

Have you tried using OkHttp3?

Yes and wasn't successful so far. I use GitHub.connect to connect to GitHub.com with Environmental variables.

The code in OkHttpConnectorTest was promising in the beginning, but I could not find a hint that made my code work.

In case it exists, please point me to an example that explains how to use OkHttp3 with GitHub.connect & Environmental variables.

[bitwiseman]

but I could not find a hint that made my code work.

How about something like this:

import okhttp3.OkHttpClient;
import org.kohsuke.github.GitHub;
import org.kohsuke.github.GitHubBuilder;
import org.kohsuke.github.extras.okhttp3.OkHttpConnector;

// ... in some class
static OkHttpClient baseClient = new OkHttpClient();

// ... in some method
GitHub gitHub = GitHubBuilder.
    .fromEnvironment()
    .withConnector(new OkHttpConnector(baseClient.newBuilder().build())
    .build(); 

[lotharschulz]

Thanks @bitwiseman! These are the relevant kotlin changes I was able to do based on your input:

additional dependency using kotlin dsl (build.gradle.kts)

    implementation("com.squareup.okhttp3:okhttp:4.9.0")

some kt file

import okhttp3.OkHttpClient
import org.kohsuke.github.GitHubBuilder.fromEnvironment
import org.kohsuke.github.extras.okhttp3.OkHttpConnector
...
// within an existing class
    companion object {
        private val baseClient = OkHttpClient()
    }
...
// within an existing fun
                fromEnvironment().
                withConnector(
                    OkHttpConnector(baseClient)
                ).build()

[aalmiray]

I just hit the same issue when updating a the state of a Milestone.

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.kohsuke.github.GitHubHttpUrlConnectionClient$HttpURLConnectionResponseInfo (file:/private/tmp/jreleaser/jreleaser-0.1.0-SNAPSHOT/lib/github-api-1.125.jar) to field java.net.HttpURLConnection.method
WARNING: Please consider reporting this to the maintainers of org.kohsuke.github.GitHubHttpUrlConnectionClient$HttpURLConnectionResponseInfo
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

As previously stated running the code in Java 8 does not produce the warning.
Running the code with Java 9 up to 15 will produce the warning.
Running the code with JAva 16+ will make the program fail as I believe illegal-access is no longer allowed.

Moving to OkHttp is not an option (at least not for me).

The OpenFeign project has the same issue with their default HttpClient (inability to support more HTTP verbs such as PATCH). They did not use reflection to fix this. Instead they offer other Http clients such as Apache HTTP Client.

Would it be possible for this project to provide another HttpConnector based on Apache Http Client?

[bitwiseman]

@aalmiray
You are welcome to implement one. The HttpConnector interface depends on HttpURLConnection interface and is very minimal.

As an alternative it looks like you can allow reflective access in various ways:

https://stackoverflow.com/questions/46454995/how-to-hide-warning-illegal-reflective-access-in-java-9-without-jvm-argument
https://nipafx.dev/five-command-line-options-hack-java-module-system/
https://dzone.com/articles/migrating-your-project-to-jigsaw-step-by-step

At some point we'll remove the dependency on HttpURLConnection, but that is a much larger effort.

[aalmiray]

I might actually, considering my tool has to run with Java 8 for a loooong time.

Coincidentally, the links you posted don't work in this situation: running in classpath mode in Java 9+

The 1st link simply disables the warning. OK-ish in 9 - 15 but will break in 16+.
The other 2 links assume the application is modular, which again is not the case as the application is designed to run in the classpath.,

Hence why an alternate client is needed.

[bitwiseman]

@aalmiray
Hmm, take a look at this:
https://stackoverflow.com/questions/25163131/httpurlconnection-invalid-http-method-patch

We might be able to avoid reflection by using X-HTTP-Method-Override headers assuming GitHub recognizes it. Before you go implement an apache connector, could you give this a try? It would be a nice improvement to the project.

[aalmiray]

Well I could use the header but hub4j would still use the reflective call to set the HTTP verb. I don't think there's a way from my POV as a consumer to tell if the header works or not. The real test is to remove the reflective call and use the override header.

[bitwiseman]

@aalmiray I meant, you could try updating hub4j to use the header and remove the reflective access.

Replacing this insanity:

github-api/src/main/java/org/kohsuke/github/GitHubHttpUrlConnectionClient.java

Lines 165 to 189 in 7c8a7ff

	// JDK only allows one of the fixed set of verbs. Try to override that
	try {
	Field $method = HttpURLConnection.class.getDeclaredField("method");
	$method.setAccessible(true);
	$method.set(connection, method);
	} catch (Exception x) {
	throw (IOException) new IOException("Failed to set the custom verb").initCause(x);
	}
	// sun.net.www.protocol.https.DelegatingHttpsURLConnection delegates to another HttpURLConnection
	try {
	Field $delegate = connection.getClass().getDeclaredField("delegate");
	$delegate.setAccessible(true);
	Object delegate = $delegate.get(connection);
	if (delegate instanceof HttpURLConnection) {
	HttpURLConnection nested = (HttpURLConnection) delegate;
	setRequestMethod(method, nested);
	}
	} catch (NoSuchFieldException x) {
	// no problem
	} catch (IllegalAccessException x) {
	throw (IOException) new IOException("Failed to set the custom verb").initCause(x);
	}
	}
	if (!connection.getRequestMethod().equals(method))
	throw new IllegalStateException("Failed to set the request method to " + method);

[bitwiseman]

@aalmiray
Okay, you got me excited about removing this hacky reflection code. I think it PR #1083 fixes this issue, replacing reflection with X-HTTP-Method-Override where needed.

If you could build this locally and give it try, that would a huge help. I will probably not include this in the next release, but the release after it (a few weeks from now) since this is in some respects a huge change.

All this said, it would still be great if you wanted to implement a connector for Apache client - that client and okhttp3 offer a number of advanced features including response caching that radically improve performance and stability of this library. It would be nice to be able to give people Apache as an drop-in option similar to OkHttp3.

[aalmiray]

Thank you @bitwiseman 😄 !
I'll give it a try tomorrow, it's close to midnight where I am right now.
Will report back as soon as I have something conclusive.

[aalmiray]

@bitwiseman I can confirm the override method works and no reflection calls occur. Thank you!

[bitwiseman]

Unfortunately, HTTP-Method-Override only works in most cases. It turns out some endpoints do not honor it. Reverted the change.

We're back to the original guidance: use another client library such as OkHttp.
If someone wants to implement a connector for Apache client that would be great.
The eventual fix will be to move off of URLConnection.

[aalmiray]

Any chance of this being revisited? Sadly OkHttp is not an option for me. Apache Client would be much better.

Closing a milestone when running on JDK17 will trigger an error because of reflective access.

[gsmet]

@aalmiray are you sure? I think this has been fixed with the latest versions and the switch to the JDK11+ client.

[aalmiray]

Yes, I'm sure as I can't use the JDK11+ client just yet. The code that applies PATCH relies on reflection and that crashes under JDK17+

[gsmet]

Well, sure, if you’re not using the JDK 11 one, it will fail. I can understand the Okhttp client might not be ideal given you end up with Kotlin around but not sure why the JDK 11 client wouldn’t be.
Your requirements look extremely specific so maybe you can use a tweaked client layer. I would have thought it would be possible given the infra work that was done.

[aalmiray]

Correct. JReleaser's Maven and Gradle plugins are still bound to JDK8 for the foreseeable future. This problem becomes moot the project upgrades to JDK11.

I suppose staying with hub4j 1.129 is the current workaround.