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

Make SSL private to prevent its usage before it is initialized. #630

Merged
merged 1 commit into from
Dec 13, 2019

Conversation

miklish
Copy link
Collaborator

@miklish miklish commented Dec 13, 2019

See issue #629 for full details.

@miklish
Copy link
Collaborator Author

miklish commented Dec 13, 2019

Found another usage of SSL in light-4j (OAuthHelper)… WIll fix and update PR branch

@stevehu stevehu merged commit b25df39 into 1.6.x Dec 13, 2019
@stevehu stevehu deleted the iss629/makeClientSSLprivate branch December 13, 2019 22:03
stevehu pushed a commit that referenced this pull request Dec 13, 2019
@miklish
Copy link
Collaborator Author

miklish commented Dec 13, 2019

Sorry folks... as mentioned, there are more cases where the variable SSL is used directly that weren't updated... updating now

@stevehu
Copy link
Contributor

stevehu commented Dec 13, 2019

I think this breaks a lot of existing applications. If we don't do anything, all existing applications should work.

@miklish
Copy link
Collaborator Author

miklish commented Dec 13, 2019

I fixed it so it builds now, but the Audit Handler unit tests are failing. I'm examining why this is. As for breaking existing applications, that is a good thing. The alternative is apps that build but fail when run.

@miklish
Copy link
Collaborator Author

miklish commented Dec 13, 2019

I figured out why some of the unit tests were failing. They use Http2Client.SSL in the test, but do not have any truststores configured. The solution is simply to delete the Http2Client.SSL argument.

The problem was that I had replaced the Http2Client.SSL argument with Http2Client.getInstance().getDefaultXnioSSL() which caused the client to look for truststores that were not configured.

E.g.: The unit tests were relying on the fact that Http2Client.SSL would be null

@miklish
Copy link
Collaborator Author

miklish commented Dec 13, 2019

Ok, as per our discussion, this change should be reverted, as it causes to many issues in other related repos. We will instead deprecate Http2Client.SSL

younggwon1 pushed a commit to younggwon1/light-4j that referenced this pull request Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants