-
Notifications
You must be signed in to change notification settings - Fork 627
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
Apache MINA 2.2.X update #635
Conversation
quickfixj-core/src/main/java/quickfix/mina/initiator/InitiatorProxyIoHandler.java
Outdated
Show resolved
Hide resolved
Thanks for the PR. I'm more comfortable merging your PR than mine since you have more experience in SSL and MINA internals than me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please also update https://github.com/quickfix-j/quickfixj/blob/master/quickfixj-core/src/main/doc/usermanual/usage/configuration.html with the new configuration and remove the old UseSNI
config? Thank you.
MINA 2.2.2 has been released.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @the-thing , looking good to me.
@masonedmison did you test that it works with this change.? |
Fixes #634
Attempt to update MINA
I had a look at the branch - #441, but it was slightly behind the master so I decided to do it from scratch on master. Feel free not to merge this pull requires if you want to apply similar changes to the other branch.
I was able to improve and remove some of the SSL code including cipher suites config and the temporary useSNI flag etc. Test cases for SNI were also added.
I'm not sure about the proxy code. The problem with proxy is that there are no tests for it so we don't know if we are breaking things or fixing them. I think it is crucial to add some embedded proxy server at some point and make sure that it works.