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

Apache MINA 2.2.X update #635

Merged
merged 9 commits into from
Dec 1, 2023

Conversation

the-thing
Copy link
Contributor

@the-thing the-thing commented May 13, 2023

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.

quickfixj-core/pom.xml Outdated Show resolved Hide resolved
@the-thing the-thing marked this pull request as draft May 13, 2023 08:49
@chrjohn
Copy link
Member

chrjohn commented May 13, 2023

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.

Copy link
Member

@chrjohn chrjohn left a 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.

@chrjohn chrjohn mentioned this pull request May 15, 2023
@the-thing the-thing marked this pull request as ready for review June 6, 2023 11:27
@the-thing the-thing requested a review from chrjohn June 6, 2023 11:27
Copy link
Member

@chrjohn chrjohn left a 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
Copy link

masonedmison commented Jun 9, 2023

@chrjohn Hi there, is this work slated to be released soon? I am facing a similar issue as described in this SO post - I'm currently on JDK11 and would rather not have to go back to JDK 8 if at all possible.

@chrjohn
Copy link
Member

chrjohn commented Jun 10, 2023

@masonedmison did you test that it works with this change.?

@chrjohn chrjohn merged commit f5447cf into quickfix-j:master Dec 1, 2023
11 checks passed
@the-thing the-thing deleted the 634_-_mina_2.2.X_update branch February 19, 2024 08:38
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.

Upgrade to latest MINA release
3 participants