KAFKA-10713: (redux) Also allow underscore in protocol name

[tombentley]

No description provided.

[tombentley]

@mimaison @rayokota please take a look

[mimaison]

I don't think it's enough. The bootstrap.servers documentation says:

This list should be in the form host1:port1,host2:port2,....

But clients also accept protocol://host:port even if they don't use the protocol.
It looks like this was not really expected as there are no tests validating this. As it's unused the protocol could be anything and contains any characters. For example, a client with the following works:

config.put(CommonClientConfigs.BOOTSTRAP_SERVERS_CONFIG, "WH4T#$@VER¾://localhost:9092");

So if a protocol is specified, we should allow any characters.

[tombentley]

OK, but then we either:

1. Make it literally any character, and mark KAFKA-10713 as won't fix, or
2. Make it any character except ;, in order to prevent KAFKA-10713, or
3. Fix the parsing to be as documented, disallowing any protocol for the broker's bootstrap.servers, streams' application.server, and the raft quorum.voters.

It seems that the protocol was first added in 53f3143 (part of https://cwiki.apache.org/confluence/display/KAFKA/Multiple+Listeners+for+Kafka+Brokers), but it's not really clear why it was allowed for client configs.

While option 2 is easiest, I feel that 3 is closer to the right thing to do: Having documentation and implementation aligned will prevent other possible errors people might make, especially as this code is slowly being reused for other configs. Thoughts?

[rayokota]

Perhaps we should have a KIP and discussion for this since it changes existing public behavior? In the meantime, we should probably revert the original PR.

[ijuma]

If we want to make a change that is not backwards compatible, it would be best to do it in 3.0. Either way, a KIP is needed and it makes sense to revert/adjust so that compatibility is preserved for now.

[tombentley]

@ijuma or @mimaison could one of you revert 8a59a22 and I'll write a KIP.

[ijuma]

@tombentley Done.

[tombentley]

Thanks @ijuma, I've opened https://cwiki.apache.org/confluence/display/KAFKA/KIP-697%3A+Stricter+parsing+of+addresses+in+configs, targetting 3.0.