[ 
https://issues.apache.org/jira/browse/KAFKA-20894?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18102268#comment-18102268
 ] 

GiminKim commented on KAFKA-20894:
----------------------------------

Thanks for asking. I could not find a Kafka-internal runtime path that
directly invokes StreamsGroupDescription.toString(); for example,
StreamsGroupCommand accesses the individual fields instead.

The failure is reachable by Admin API consumers, though. The convenience
describeStreamsGroups(Collection) method uses the default options, where
includeAuthorizedOperations is false. Therefore, the returned
StreamsGroupDescription can legitimately have authorizedOperations == null.

Any direct or implicit string conversion then reaches toString(), for example:

StreamsGroupDescription description =
    admin.describeStreamsGroups(List.of(groupId))
        .all().get().get(groupId);

System.out.println(description);
String text = "group=" + description;

Since null is a documented valid state and the public API object overrides
toString(), I think toString() should handle it safely. I agree that the
impact is limited to consumers that stringify the returned description,
which is why I classified it as Minor.

> StreamsGroupDescription.toString throws NullPointerException when authorized 
> operations are omitted
> ---------------------------------------------------------------------------------------------------
>
>                 Key: KAFKA-20894
>                 URL: https://issues.apache.org/jira/browse/KAFKA-20894
>             Project: Kafka
>          Issue Type: Bug
>          Components: clients, streams
>            Reporter: GiminKim
>            Assignee: GiminKim
>            Priority: Minor
>
> h2. Problem
> `DescribeStreamsGroupsOptions.includeAuthorizedOperations()` defaults to 
> `false`. In this normal request path, the broker omits authorized operations 
> and `AdminUtils.validAclOperations()` converts the omitted value to `null`. 
> `StreamsGroupDescription` and its Javadoc explicitly allow that nullable 
> state.
> However, `StreamsGroupDescription.toString()` unconditionally calls 
> `authorizedOperations.stream()`. As a result, merely logging or rendering a 
> successfully returned description can throw `NullPointerException`.
> h2. Reproduction
> {code:java}
> StreamsGroupDescription description = result.get();
> log.info("group={}", description);
> {code}
> The failure occurs when authorized operations were not requested:
> {code}
> includeAuthorizedOperations=false
>   -> server omits authorized operations
>   -> validAclOperations() returns null
>   -> StreamsGroupDescription stores null
>   -> toString() calls authorizedOperations.stream()
>   -> NullPointerException
> {code}
> h2. Expected behavior
> `toString()` should handle the documented omitted state and render 
> `authorizedOperations=null`. Formatting for populated authorized-operation 
> sets should remain unchanged.
> h2. Root cause
> The constructor and accessor accept and document `null`, but `toString()` 
> assumes the set is always non-null.
> h2. Proposed fix
> Add a null check in `StreamsGroupDescription.toString()` and extend the 
> existing AdminClient test for an omitted authorized-operations response to 
> assert that `toString()` does not throw and includes 
> `authorizedOperations=null`.
> h2. Testing
> * Targeted `KafkaAdminClientTest` and `DescribeStreamsGroupsHandlerTest`
> * Full `clients:test` suite
> * Checkstyle and SpotBugs main analysis
> * `spotlessCheck`
> All applicable local tests and quality checks pass.
> h2. Pull request
> [apache/kafka#23083|https://github.com/apache/kafka/pull/23083]



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to