adixitconfluent commented on code in PR #20819:
URL: https://github.com/apache/kafka/pull/20819#discussion_r2494794976
##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -4137,6 +4148,11 @@ class KafkaApis(val requestChannel: RequestChannel,
config.shareGroupConfig.isShareGroupEnabled ||
shareVersion().supportsShareGroups
}
+ // Visible for testing.
+ def isMemberIdValid(memberId: String): Boolean = {
+ memberId.nonEmpty && memberId.length <= 36 &&
!memberId.equals(Uuid.ZERO_UUID.toString)
Review Comment:
>When we are checking for ZERO_UUID then should we also check that it's not
of any of the reserved Uuid
Why do we want to do that? Since ONE_UUID does not have any special meaning
to the broker, disallowing extra strings is a good idea?
>For my understanding: Also Uuid.ZERO_UUID.toString will generate
AAAAAAAAAAAAAAAAAAAAAAAA, by this check we want to restrict usage of AAAAA....
as member id?
Correct, and we want to restrict it because we use `EMPTY_MEMBER_ID` as
`ZERO_UUID` in SharePartition to denote that the member id of unacquired
records.
>Can memberId be " " i.e. just spaces, is it valid?
Non-Java clients can use it but Java clients cannot use it because in Java
we use `Uuid.randomUuid().toString()` to generate memberId and if you run
`Uuid.fromString(" ")`, it is going to give you an exception, so it cannot
be generated by `Uuid.randomUuid()`. However, this is an interesting question
if we should allow empty non-zero length strings such as `" "`. Wdyt
@AndrewJSchofield ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]