KAFKA-17164: Enforce 'application.server' <server>:<port> format at config level. by chickenchickenlove · Pull Request #22202 · apache/kafka

chickenchickenlove · 2026-05-04T04:10:15Z

Description

This PR implement
KIP-1245,
which enforces the application.server host:port format during
StreamsConfig construction.

Changes

Add a ConfigDef.Validator for application.server to fail fast on
invalid values in StreamsConfig instead of later in HostInfo.
Update an existing StreamsConfigTest case to use a valid
application.server value.
Add test coverage for valid and invalid application.server values.

Reviewers: Matthias J. Sax matthias@confluent.io

chickenchickenlove · 2026-05-04T04:12:32Z

Hi, @Nikita-Shupletsov !

It’s been a while since we last spoke.
I opened a PR for KIP-1245, which we discussed earlier!
When you get a chance, please take a look 🙇‍♂️

nileshkumar3

@chickenchickenlove thanks for the PR. I have a very minor comment.

nileshkumar3 · 2026-05-05T01:19:30Z

+
+        final String endPoint = (String) value;
+
+        if (endPoint.isEmpty()) {


This is redundant , Utils.isBlank(endPoint) covers this.

chickenchickenlove · 2026-05-05T02:50:36Z

@nileshkumar3 Thanks for the time to take a look!
I've addressed it based on your comments.
When you get a chance, please take another look.

Nikita-Shupletsov

Thank you for the PR!
left a few comments

Nikita-Shupletsov · 2026-05-05T03:03:14Z

+
+    private static final ConfigDef.Validator INSTANCE = new ApplicationServerConfigValidator();
+
+    public static ConfigDef.Validator getInstance() {


is it worth having it as a singleton?
I checked a few places(e.g. https://github.com/apache/kafka/blob/trunk/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/TopicCreationConfig.java#L128) and it looks like we don't really do that anywhere else

Nikita-Shupletsov · 2026-05-05T03:03:56Z

+import org.apache.kafka.common.config.ConfigException;
+import org.apache.kafka.common.utils.Utils;
+
+import static org.apache.kafka.common.utils.Utils.getHost;


nit: some methods from this class are imported statically, some are called via Utils.method

Nikita-Shupletsov · 2026-05-05T03:07:45Z

+        final Integer port;
+        try {
+            port = getPort(endPoint);
+        } catch (final NumberFormatException e) {


do we need to catch NumberFormatException? looks like HOST_PORT_PATTERN already checks that it's exclusively digits

The pattern only says the port looks like digits. Integer.parseInt still errors if that number is too big for a int.

When a very large number is provided as the port number, HOST_PORT_PATTERN.matcher(...) operates normally. However, when Integer.parseInt() is called on such a large number, a NumberFormatException is thrown.

While HostInfo would have previously thrown a NumberFormatException for large numbers as well, we now catch the NumberFormatException in order to throw a ConfigException, as described in the KIP.

it makes sense. would it make sense to tighten the regex instead?

It might be better, but I would prefer to do it in other PR.
This is because Utils.getPort(...) is not only used for HostInfo but also QuorumConfig, etc.

If we want to tighten the regex, I think another KIP is required.

I am not sure about a KIP for a change that, effectively, fixes a bug.
@mjsax could you please chime in? thank you

If we want to tighten the regex, I think another KIP is required.

I don't think so. I think the simplest fix would be, to push the try-catch into getPort() and let it return null if a NumberFormatException gets thrown? This would still align to the intended semantics of getPort() to return null if the port is not valid.

This would really just be a simple bug-fix (and messing with the regex-ex is always a mess, so maybe just catching the exception is simpler).

I see.
Thanks for your comments, Nikita and mjsax!
I'll keep it as it is.

chickenchickenlove · 2026-05-05T12:09:56Z

@Nikita-Shupletsov
Thanks for the time to take a look into this PR.
I've addressed it based on your comments!
When you get a chance, please take another look!

Nikita-Shupletsov · 2026-05-05T17:15:18Z

+import org.apache.kafka.common.config.ConfigException;
+import org.apache.kafka.common.utils.Utils;
+
+


nit: extra empty line

chickenchickenlove · 2026-05-06T11:57:10Z

@Nikita-Shupletsov
Thanks for your quick feedback!
I've removed empty line and left reply for your comments!
When you get a chance, please take a look! 🙇‍♂️

Nikita-Shupletsov

LGTM. thank you!

Nikita-Shupletsov · 2026-05-06T16:30:19Z

+        final Integer port;
+        try {
+            port = getPort(endPoint);
+        } catch (final NumberFormatException e) {


I am not sure about a KIP for a change that, effectively, fixes a bug.
@mjsax could you please chime in? thank you

nileshkumar3

LGTM, thanks.

chickenchickenlove · 2026-06-09T10:16:50Z

@mjsax
Sorry to bother you.
When you get a chance, could you take a look?

mjsax

Thanks for the PR. Overall LGTM.

Just wondering, if HostInfo#buildFromEndpoint() and the new validator (both duplicate the same code) could be unified?

mjsax · 2026-06-15T19:36:28Z

    }

+    @ParameterizedTest
+    @ValueSource(strings = {"dummy:host", "dummy:9999999999999999999999999", "dummy", "dummy:", ":port"})


Should we add ":8080" as one more test? Maybe also ":" ?

Thanks for your comments.
I've added them!

mjsax · 2026-06-15T19:44:43Z

+        final Integer port;
+        try {
+            port = getPort(endPoint);
+        } catch (final NumberFormatException e) {


If we want to tighten the regex, I think another KIP is required.

I don't think so. I think the simplest fix would be, to push the try-catch into getPort() and let it return null if a NumberFormatException gets thrown? This would still align to the intended semantics of getPort() to return null if the port is not valid.

This would really just be a simple bug-fix (and messing with the regex-ex is always a mess, so maybe just catching the exception is simpler).

chickenchickenlove · 2026-06-18T13:15:37Z

@mjsax
Thanks for the review!

Just wondering, if HostInfo#buildFromEndpoint() and the new validator (both duplicate the same code) could be unified?

I've addressed it!
I have a question.
Could you let me know which release this KIP is planned for?
I'd like to update docs/streams/upgrade-guide.md accordingly for that release.

mjsax

Thanks for the PR. LGTM.

As the PR is ready for merging, it will go into 4.4.0 release. Good callout to also update the upgrade guide section accordingly. Can merge after the docs update was added to this PR.

…onfig level.

chickenchickenlove · 2026-06-22T01:29:23Z

@mjsax
Thanks for your comments!
I've added it to docs sections.
Also, I've rebased on the latest trunk because CI was failed due to 1 password error.
When you get a chance, please take another look. 🙇‍♂️

github-actions Bot added streams triage PRs from the community small Small PRs labels May 4, 2026

nileshkumar3 reviewed May 5, 2026

View reviewed changes

Nikita-Shupletsov reviewed May 5, 2026

View reviewed changes

github-actions Bot removed the triage PRs from the community label May 5, 2026

chickenchickenlove requested review from Nikita-Shupletsov and nileshkumar3 May 5, 2026 12:09

Nikita-Shupletsov reviewed May 5, 2026

View reviewed changes

chickenchickenlove requested a review from Nikita-Shupletsov May 6, 2026 11:57

Nikita-Shupletsov approved these changes May 6, 2026

View reviewed changes

nileshkumar3 approved these changes May 9, 2026

View reviewed changes

mjsax reviewed Jun 15, 2026

View reviewed changes

mjsax approved these changes Jun 19, 2026

View reviewed changes

mjsax added the kip Requires or implements a KIP label Jun 19, 2026

chickenchickenlove added 6 commits June 22, 2026 10:13

KAFKA-17164: Enforce 'application.server' <server>:<port> format at c…

84bfdc3

…onfig level.

Addressing review.

aeb5e9b

Addressing review.

1cfc398

Remove extra empty line.

d8dbc98

Addressing Review.

855caca

Update upgrade docs.

62e9071

chickenchickenlove force-pushed the KAFKA-17164 branch from 4c42ecc to 62e9071 Compare June 22, 2026 01:20


		final String endPoint = (String) value;

		if (endPoint.isEmpty()) {


		private static final ConfigDef.Validator INSTANCE = new ApplicationServerConfigValidator();

		public static ConfigDef.Validator getInstance() {

		import org.apache.kafka.common.config.ConfigException;
		import org.apache.kafka.common.utils.Utils;

Conversation

chickenchickenlove commented May 4, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Uh oh!

chickenchickenlove commented May 4, 2026

Uh oh!

nileshkumar3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chickenchickenlove commented May 5, 2026

Uh oh!

Nikita-Shupletsov left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

chickenchickenlove commented May 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chickenchickenlove commented May 6, 2026

Uh oh!

Nikita-Shupletsov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nileshkumar3 left a comment

Choose a reason for hiding this comment

Uh oh!

chickenchickenlove commented Jun 9, 2026

Uh oh!

mjsax left a comment

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!

chickenchickenlove commented Jun 18, 2026

Uh oh!

mjsax left a comment

Choose a reason for hiding this comment

Uh oh!

chickenchickenlove commented Jun 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chickenchickenlove commented May 4, 2026 •

edited by github-actions Bot

Loading