Fixed SSL server_name extension value

[the-thing]

Fixes #1036

Two new properties added UseSNI and SNIHostName. When UseSNI=Y then either SNIHostName will be used or SocketConnectHost / SocketConnectHost<n>. When UseSNI=N the JVM will decide what to set which most likely be SocketConnectHost. Will work for proxies as well.

There is only one SNIHostName property even for multiple connect hosts, but it the future it might be required to add support for SNIHostName<n> similar to SocketConnectHost<n>. Lets see if there is a need in there future.

[chrjohn]

@the-thing thank you for the PR.
Just a question: to run all tests (it seems the ones that are using a proxy), I need to define a custom hostname localhost-quickfixj? Could this be achieved also programmatically? Because one could not run all tests on its machine without defining that extra hostname.

[the-thing]

The problem is that for certain test cases you can't use "localhost", because JVM will not populate it and you will not able to make certain assertions.

The tests will run on any environment, but the assumption assumeLocalhostAliasAvailable() will skip the test if the condition is not met - so no failures.

To be sure it runs on all the environments without skipping there would have to be some maven script that adds a custom host, which supports different operating systems and has sudo rights. This might be a problem. This is probably one of the few examples where conditional tests make sense.

Also maybe there is some custom Java SSL implementation that allows you mocking behaviors, but I wouldn't even go that way.

[the-thing]

I can't think of anything that is cleaner to what I did, but if you have a suggestion I will rewrite it.

As a last resort we could completely remove tests requiring this extra domain and replace it with a unit test for initiator ssl filter, but a lot of value in end to end test will be lost.

[chrjohn]

Would this work: https://stackoverflow.com/a/74250879/4962355 ?
But could be that I misunderstood what you are achieving with the additional host entry. If I misunderstood, please forgive me. :)

[the-thing]

@chrjohn

It works. Seems like caching DNS is easier than I thought. Thanks.

[chrjohn]

It works. Seems like caching DNS is easier than I thought. Thanks.

Cool, thanks for letting me know.

[chrjohn]

@the-thing , there are failures on Windows with JDK 11 and 21 in this test:

Failures: 
SSLCertificateTest.shouldAuthenticateServerAndClientCertificates:627 Session is not logged on: FIX.4.4:ZULU->ALFA[in:1,out:2]

Do you think this is related to your latest changes? Although the test was not changed AFAICT (the diff is not really helpful).

[the-thing]

Not sure yet. Checking on another branch. If so, then only related to this DNS cache.

[chrjohn]

Not sure yet. Checking on another branch. If so, then only related to this DNS cache.

Maybe it does help to uninstall() after every test, but only guessing.

[the-thing]

Doing install / uninstall, before and after every test. Lets see.

https://github.com/the-thing/quickfixj/actions/runs/17855159273

[chrjohn]

@the-thing thank you for your work on this, much appreciated 👍
Is this ready to merge?

[the-thing]

Yes. Looks solid now. Thanks.

[the-thing]

@herste

Any comments?

[herste]

@herste

Any comments?

Hi there - I'll give it a try in our environment and let you know asap... Thanks for all the efforts so far!

[herste]

Hello @the-thing,

successfully tested the patch and works as expected - thanks a lot for the efforts. We have back ported the fix to the current release version for testing. May I kindly ask whether you also plan to back port and release a 2.3.3 version?

[the-thing]

Thanks for checking. I wasn't planning to. It might be hard to cherry pick this for 2.3.3 due to MINA differences and most likely it will require to be applied manually again.

You can always try to test the nightly build after this change is merged.

[chrjohn]

@the-thing the 2.2.x MINA changes were part of the QFJ 2.3.2 release, so it should be possible to cherry-pick this without problems. I can do it if you want.
@herste sorry, cannot make any commitments on 2.3.3 release, still trying on 3.0.0. Would it help if this was cherry picked to 2.3.x branch? But you already did that by yourself it seems.

[ffeisst]

I have provided a backport for 2.3.x in #1049. It would be great, if this change could be part of future 2.3.x releases.