feat: Add option to let `InitiatorSslFilter` bypass `InetSocketAddress`' reverse DNS lookup by t-fitchie-meur · Pull Request #1271 · quickfix-j/quickfixj

t-fitchie-meur · 2026-07-01T09:11:56Z

This is a feature to fix something that's not really a bug, but probably not an intentional feature either, and is definitely an edge case.

Summary

Currently, all of the host resolution code in quickfixj uses InetSocketAddress::getHostName, which performs a reverse DNS lookup that may block for several seconds in certain cases, such as if you are attempting to connect to an IP address that does not have a PTR DNS record. I have observed what I believe is the following race during setup of such a FIX connection:

Reverse DNS lookup pre-TCP connect in IoSessionInitiator (this is fine, but may elongate session start-up time)
Establish TCP connection
Another Reverse DNS lookup in InitiatorSslFilter whilst attempting to initialize the TLS handshake
In the time we're blocking waiting for the Reverse DNS lookup, the acceptor times out waiting for us to initialize the TLS handshake
When the Reverse DNS finally times out, the acceptor has closed the TCP connection, so we get END_OF_STREAM

In scenarios where we are connecting directly to an IP address, most of the time the reverse DNS lookup does not provide any functionality, so I propose that we add the option to bypass this.

Sample configuration in which the issue was discovered:

ConnectionType=initiator
SocketUseSSL=Y
EnabledProtocols=TLSv1.3
SocketConnectHost=1.2.3.4
SocketConnectPort=1234

JDK: 25
QuickFIX/J version: 3.0.1

Without the fix applied, I see the following pattern in the logs, repeating every retry interval:

MINA session created: local=/---.---.---.--/-----, remote=/---.---.---.--/-----
ERROR ... NullPointerException ... SslFilter.filterWrite ... sslHandler is null
Disconnecting: Encountered END_OF_STREAM

With the fix applied, things connect immediately, with no wait for the reverse DNS lookup.

Change safety

The reverse DNS setting defaults to enabled, so this should not introduce any regressions

chrjohn · 2026-07-01T11:00:47Z

@t-fitchie-meur thanks for the PR. 👍 I will take a look. First thing that came to my mind: could you please add the config setting to configuration.md also? I am also thinking about the naming of the setting, but that can be easily changed later on.
@the-thing any thoughts? Thanks. 😃

Copilot

Pull request overview

This PR introduces a configurable way for initiator-side networking/TLS code to avoid InetSocketAddress#getHostName() reverse-DNS lookups (which can block), by routing host selection through a HostResolutionStrategy and a new session setting (ReverseDNSEnabled, defaulting to enabled for backward compatibility).

Changes:

Added HostResolutionStrategy with WITH_REVERSE_DNS / WITHOUT_REVERSE_DNS options.
Threaded the strategy through initiator connection setup and InitiatorSslFilter engine creation to choose getHostName() vs getHostString().
Added unit coverage for InitiatorSslFilter peer-host selection behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
quickfixj-core/src/test/java/quickfix/mina/ssl/InitiatorSslFilterTest.java	Adds tests validating peer-host selection under both host-resolution strategies.
quickfixj-core/src/main/java/quickfix/mina/ssl/InitiatorSslFilter.java	Uses `HostResolutionStrategy` to select the host passed into `SSLContext#createSSLEngine`.
quickfixj-core/src/main/java/quickfix/mina/initiator/IoSessionInitiator.java	Threads host-resolution strategy into SSL filter creation and reconnect address recreation.
quickfixj-core/src/main/java/quickfix/mina/initiator/AbstractSocketInitiator.java	Reads the new `ReverseDNSEnabled` setting and selects the appropriate strategy.
quickfixj-core/src/main/java/quickfix/mina/HostResolutionStrategy.java	Introduces the functional interface and default strategy implementations.
quickfixj-core/src/main/java/quickfix/Initiator.java	Adds the `ReverseDNSEnabled` settings key with Javadoc.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

t-fitchie-meur · 2026-07-01T11:11:12Z

@chrjohn thanks for taking a look!

I've updated the configuration.md and the .html file. Let me know if there's anything else!

chrjohn · 2026-07-01T11:13:00Z

@t-fitchie-meur thanks. I've deleted the html doc in the meantime since it was outdated. Sorry :) I'll resolve the conflict.

…classes

chrjohn · 2026-07-01T11:41:27Z

@copilot resolve the merge conflicts in this pull request

Removed comment about public API status and adjusted constructor parameters.

the-thing · 2026-07-20T06:40:03Z

I had a brief look and the PR looks ok itself, but...

My concern is that the scenario that is being described shouldn't normally happen unless I missed something. The reverse DNS lookup is first performed in quickfix.mina.initiator.IoSessionInitiator.ConnectTask#getSniHostName to connect to the next host in the list and later when SSL filter is installed in quickfix.mina.ssl.InitiatorSslFilter#createEngine.

java.net.InetSocketAddress#getHostName normally caches successfully resolved hostnames permanently so the resolution before connection should be instantaneous when creating SSL filter, unless DNS caching is disabled or DNS cache TTL is very small.

I'm not sure how hard is this to change, but I have a feeling that making sure that destination host is resolved only before connection is the probably the way to go.

t-fitchie-meur · 2026-07-20T07:19:47Z

Hey @the-thing, thanks for taking a look!

The idea here is to cover the case where the session you're trying to connect to, for whatever reason, might have reverse DNS deliberately configured not to work. When this happens, we don't have a result to cache so we just retry every time InetSocketAddress::getHostName is called.

I agree that making sure that the destination host resolution (or resolution failure) occurs pre-connection is a better solution, but this does change the behaviour of the library, and I wasn't sure how strict quickfix is with changes like this? Happy to refactor this PR in that direction if you think it's fine.

the-thing · 2026-07-20T07:58:37Z

No probs.

I agree that making sure that the destination host resolution (or resolution failure) occurs pre-connection is a better solution, but this does change the behaviour of the library, and I wasn't sure how strict quickfix is with changes like this? Happy to refactor this PR in that direction if you think it's fine.

@t-fitchie-meur

Now when I think about this. it doesn't require any change. All java.net.InetSocketAddress instances share the same cache, so successful connection to the end point will keep the successfully resolved hostname (ignoring some custom dns cache config).

The idea here is to cover the case where the session you're trying to connect to, for whatever reason, might have reverse DNS deliberately configured not to work. When this happens, we don't have a result to cache so we just retry every time InetSocketAddress::getHostName is called.

@chrjohn

The change looks good itself. This is an extra functionality, but also an additional complexity too.

the-thing

One suggestion would be to write an end to end test demonstrating functionality with both strategies.

Something similar to quickfix.mina.ssl.SSLCertificateTest, but it doesn't need to use SSL etc.

t-fitchie-meur · 2026-07-20T12:23:19Z

One suggestion would be to write an end to end test demonstrating functionality with both strategies.

Something similar to quickfix.mina.ssl.SSLCertificateTest, but it doesn't need to use SSL etc.

@the-thing

I've added some E2E tests. To do this it made sense to add the reverse dns enabled configuration option to the acceptor as well as the initiator. Let me know what you think!

the-thing · 2026-07-22T10:15:47Z

+        Map<String, String> hostAliases = new HashMap<>();
+        hostAliases.put(FAKE_HOSTNAME, LOOPBACK_IP);
+        reverseDnsResolver = new BlockingReverseDnsResolver(hostAliases);
+        HostResolutionRequestInterceptor.INSTANCE.install(reverseDnsResolver, DefaultHostResolver.INSTANCE);


This fails internally during first time static context construction, but this seems to have no effect as the exception is not propagated upstream.

java.lang.UnsupportedOperationException at java.base/java.util.ImmutableCollections.uoe(ImmutableCollections.java:159) at java.base/java.util.ImmutableCollections$AbstractImmutableMap.put(ImmutableCollections.java:1318) at org.burningwave.core.classes.Modules.exportToAll(Modules.java:182) at org.burningwave.core.classes.Modules.lambda$exportAllToAll$0(Modules.java:89) at java.base/java.lang.Iterable.forEach(Iterable.java:75) at org.burningwave.core.classes.Modules.lambda$exportAllToAll$1(Modules.java:87) at java.base/java.util.HashMap.forEach(HashMap.java:1430) at org.burningwave.core.classes.Modules.exportAllToAll(Modules.java:86) at org.burningwave.core.assembler.StaticComponentContainer.<clinit>(StaticComponentContainer.java:471) at org.burningwave.tools.net.DefaultHostResolver.<clinit>(DefaultHostResolver.java:68) at org.burningwave.tools.net.HostResolutionRequestInterceptor.<clinit>(HostResolutionRequestInterceptor.java:61) at quickfix.mina.initiator.HostResolutionStrategyTest.installBlockingResolver(HostResolutionStrategyTest.java:209) at quickfix.mina.initiator.HostResolutionStrategyTest.initiatorShouldAttemptToCallReverseDnsDuringLogonWhenReverseDnsIsEnabled(HostResolutionStrategyTest.java:123)

Where are you seeing this - I can't reproduce locally or find it when searching through the gh actions workflow logs?

When running locally on Windows 11. I was running OpenJDK 25.

mvn test -pl quickfixj-core -Dmaven.javadoc.skip=true -PskipBundlePlugin,minimal-fix-latest -Dtest=HostResolutionStrategyTest -Dsurefire.failIfNoSpecifiedTests=false

Interesting, I'm also running Java 25 (Oracle, 25.0.3) on windows 11. Your command doesn't work for me (I probably have the repo misconfigured locally) but running:

./mvnw test -am -pl quickfixj-core "-Dmaven.javadoc.skip=true" "-PskipBundlePlugin,minimal-fix-latest" "-Dtest=HostResolutionStrategyTest" "-Dsurefire.failIfNoSpecifiedTests=false"

Does run for me and I can't recreate the issue.

Does it occur consistently or is it a one-off for you?

I built the project with Maven install beforehand. I think with

mvnw clean install -Dmaven.javadoc.skip=true -DskipTests -PskipBundlePlugin,minimal-fix-latest

It occurs only one time, because it happens in static {} block in

org.burningwave.core.assembler.StaticComponentContainer

Again. I don't think it is a problem, but I didn't look why it is happening.

the-thing · 2026-07-22T10:29:29Z

            }

            if (socketAddresses[nextSocketAddressIndex] instanceof InetSocketAddress) {
                return ((InetSocketAddress) socketAddresses[nextSocketAddressIndex]).getHostName();


Not sure what to do with this one.

If SNI host name is not provided we might try to do DNS lookup on the endpoint provided. If this fails we will use original address representation. This happens during SSL filter construction, way before ssl engine is created.

I think we should leave it as you did.

I believe SNI host name is prohibited from being a literal IP address (https://datatracker.ietf.org/doc/html/rfc6066#section-3).

If we were to add the host resolution strategy into here, then it would be possible to construct a broken session by disabling reverse DNS, leading to an IP getting through to the SNI name.

If you're passing a literal IP through with SNI enabled that probably suggests some degree of misconfiguration, but the intended behaviour should be either to reverse DNS to get a host name or to return null (i.e. the current behaviour of ::getHostName is correct in this case)

Correct, should stay as is, but if not explicitly provided it will possibly try DNS resolve which might fail.

the-thing

LGTM, but very optional feature.

Also, changes to AcceptorSslFilter made me realize that probably lack of

org.apache.mina.filter.ssl.SslFilter#setEndpointIdentificationAlgorithm
org.apache.mina.filter.ssl.SslFilter#setWantClientAuth

for SSL acceptor should be addressed.

codecov · 2026-07-24T14:39:41Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.85%. Comparing base (9b58948) to head (fbfc444).

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1271   +/-   ##
=========================================
  Coverage     42.85%   42.85%           
  Complexity      684      684           
=========================================
  Files           125      125           
  Lines          3794     3794           
  Branches        359      359           
=========================================
  Hits           1626     1626           
  Misses         2021     2021           
  Partials        147      147

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Add reverse DNS lookup bypass

2d492e2

chrjohn requested a review from Copilot July 1, 2026 11:00

Copilot started reviewing on behalf of chrjohn July 1, 2026 11:00 View session

chrjohn changed the title ~~feat: Add option to let InitiatorSslFilter bypass InetSocketAddress' reverse DNS lookup~~ feat: Add option to let InitiatorSslFilter bypass InetSocketAddress' reverse DNS lookup Jul 1, 2026

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread quickfixj-core/src/main/java/quickfix/mina/ssl/InitiatorSslFilter.java

Comment thread quickfixj-core/src/main/java/quickfix/mina/initiator/IoSessionInitiator.java Outdated

Comment thread quickfixj-core/src/main/java/quickfix/mina/initiator/IoSessionInitiator.java

Update configuration

b1c14fc

document that initiatorSslFilter and ioSessionInitiator are internal …

e3f4826

…classes

t-fitchie-meur and others added 4 commits July 1, 2026 13:06

Tidy up comments, remove config html change

70da966

Merge branch 'master' into reverse-proxy-bypass

4389903

Merge branch 'master' into reverse-proxy-bypass

539b9a3

Reduced visibility of IoSessionInitiator constructor

fa1d094

Removed comment about public API status and adjusted constructor parameters.

chrjohn requested a review from the-thing July 19, 2026 16:05

the-thing suggested changes Jul 20, 2026

View reviewed changes

t-fitchie-meur added 2 commits July 20, 2026 13:07

Add E2E tests + related features for the acceptor

e9483ad

tidy up

e8d23d7

fix mistake

faf465c

the-thing suggested changes Jul 22, 2026

View reviewed changes

Add both enabled test

d3098da

the-thing reviewed Jul 23, 2026

View reviewed changes

Merge branch 'master' into reverse-proxy-bypass

fbfc444

Uh oh!

Conversation

t-fitchie-meur commented Jul 1, 2026

Summary

Sample configuration in which the issue was discovered:

Change safety

Uh oh!

chrjohn commented Jul 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

t-fitchie-meur commented Jul 1, 2026

Uh oh!

chrjohn commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrjohn commented Jul 1, 2026

Uh oh!

the-thing commented Jul 20, 2026

Uh oh!

t-fitchie-meur commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-thing commented Jul 20, 2026

Uh oh!

the-thing left a comment

Choose a reason for hiding this comment

Uh oh!

t-fitchie-meur commented Jul 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

the-thing left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jul 24, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chrjohn commented Jul 1, 2026 •

edited

Loading

t-fitchie-meur commented Jul 20, 2026 •

edited

Loading