Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ELY-2026] Use reflection to pass through the new methods if they are now available on the SSLEngine, SSLParameters, and SSLSocket APIs. #1448

Merged
merged 2 commits into from
Oct 7, 2020

Conversation

darranl
Copy link
Contributor

@darranl darranl commented Oct 7, 2020

https://issues.redhat.com/browse/ELY-2026

Under ELY-1979 we did attempt a number of different approaches to implement these methods directly and avoid the use of reflection, unfortunately as the project is built using Java 11 but targeting Java 8 the Java 11 compiler does not agree that these methods are present in Java 8.

We could consider a more complex build that compiles using two different JDKs but that seems overly complex, another option could be to split out an "accessor" class into another project which is built using the latest Java 8 (and has no plans to move to Java 11 soon) but that seems over complex.

Users of Java 9 or later will automatically get the JDKSpecific implementation that uses direct access.

… now available on the SSLEngine, SSLParameters, and SSLSocket APIs.
…cols to support a subset of the previously configured set.
@fjuma fjuma added the +1 FJ label Oct 7, 2020
throw new UnsupportedOperationException();
}

static void setApplicationProtocols(SSLParameters parameters, String[] protocols) {
if (SSLPARAMETERS_SET_APPLICATION_PROTOCOLS != null) {
try {
SSLPARAMETERS_SET_APPLICATION_PROTOCOLS.invoke(parameters, (Object) protocols);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work or should it be new Object[] {protocols}?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this works, I found the same in the Undertow code as well. The main point was to stop the protocols array itself being seen as an array of arguments - when I test this Undertow is setting an array of three Strings.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right yeah. It seemed like it may work, I've just never tried it this way is all :)

@dmlloyd
Copy link
Contributor

dmlloyd commented Oct 7, 2020

One thing I've done in the past is to generate "stub" classes to compile against. This basically amounts to creating a version of the class with the expected topography, where all the concrete methods are empty (or marked "native") and then adding that to the class path (e.g. as a provided dependency). This adds some complexity to the build, but usually makes up for it by dramatically simplifying the resultant code.

It might be worth exploring in the future if reflection proves too slow or hard to maintain.

@Skyllarr Skyllarr added the +1 DV label Oct 7, 2020
@fjuma fjuma merged commit 1bc17f3 into wildfly-security:1.x Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants