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

ssl: introduce SSLEngineConfigurator#setSSLParameters #2054

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

kofemann
Copy link
Contributor

SSLParameters is a class introduced in java6 which can take over
SSLEngineConfigurator. As a first step, let allow applications to
use SSLParameters to configure SSLEngineConfigurator.

Copy link

@bshannon bshannon left a comment

Choose a reason for hiding this comment

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

I suppose this is fine, although it's interesting that SSLParameters supports a lot of
options that are ignored here, and SSLEngine can take an SSLParamaters object
directly, but that isn't done here. I suspect this code should be updated to handle
additions to SSLEngine in JDK 1.7 and 1.8.

@kofemann
Copy link
Contributor Author

As mentioned in the commit message, this can be the first step. The SSLEngineConfigurator can be updated to build SSLParamaters and then use it when SSLEngineConfigurator#configure method is called. This will make code much simple.

@@ -272,6 +273,21 @@ public SSLEngineConfigurator setWantClientAuth(boolean wantClientAuth) {
return this;
}

/**
* Apply {@link SSLParameters} to this SSLEngineConfigurator.
Copy link
Contributor

Choose a reason for hiding this comment

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

Apply {@link SSLParameters} to this SSLEngineConfigurator.

Might want to document that only the cipherSuites, protocols, needClientAuth and wantClientAuth properties are applied?

@kofemann
Copy link
Contributor Author

may be make sense to squash this two commits...

@mnriem
Copy link
Contributor

mnriem commented Aug 24, 2023

@kofemann Have you applied the requested changes?

@kofemann
Copy link
Contributor Author

Are people interested in this changes? In general, looks like grizzly project has very low activity since eclipse took over.

@arjantijms
Copy link
Contributor

Are people interested in this changes? In general, looks like grizzly project has very low activity since eclipse took over.

We definitely have a low amount of resources to work on Grizzly, which is really unfortunate :(

Copy link
Contributor

@arjantijms arjantijms left a comment

Choose a reason for hiding this comment

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

Let's forget about the requested changes

@kofemann
Copy link
Contributor Author

Ok. Let me rebase and retest. I will update the PR.

SSLParameters is a class introduced in java6 which can take over
SSLEngineConfigurator. As a first step, let allow applications to
use SSLParameters to configure SSLEngineConfigurator. Update
SSLEngineConfigurator to use it internally instead of bunch
of class fields.

Signed-off-by: Tigran Mkrtchyan <[email protected]>
@mnriem
Copy link
Contributor

mnriem commented Aug 30, 2023

@arjantijms Looks good to me

@arjantijms arjantijms added this to the 4.0.1 milestone Aug 30, 2023
@arjantijms arjantijms added the enhancement New feature or request label Aug 30, 2023
@arjantijms arjantijms merged commit a80c366 into eclipse-ee4j:master Aug 30, 2023
2 checks passed
@kofemann kofemann deleted the ssl-param branch August 30, 2023 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants