You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "dajac (via GitHub)" <gi...@apache.org> on 2023/02/23 13:46:32 UTC
[GitHub] [kafka] dajac opened a new pull request, #13295: KAFKA-14744; NPE while converting OffsetFetch from version < 8 to version >= 8
dajac opened a new pull request, #13295:
URL: https://github.com/apache/kafka/pull/13295
While refactoring the OffsetFetch handling in KafkaApis, we introduced a NullPointerException (NPE). The NPE arises when the FetchOffset API is called with a client using a version older than version 8 and using null for the topics to signal that all topic-partition offsets must be returned. This means that this bug mainly impacts admin tools. The consumer does not use null.
This NPE is here: https://github.com/apache/kafka/commit/24a86423e9907b751d98fddc7196332feea2b48d#diff-0f2f19fd03e2fc5aa9618c607b432ea72e5aaa53866f07444269f38cb537f3feR237.
We missed this during the refactor because we had no tests in place to test this mode.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dajac commented on a diff in pull request #13295: KAFKA-14744; NPE while converting OffsetFetch from version < 8 to version >= 8
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #13295:
URL: https://github.com/apache/kafka/pull/13295#discussion_r1115721868
##########
core/src/test/scala/unit/kafka/server/OffsetFetchRequestTest.scala:
##########
@@ -126,6 +126,56 @@ class OffsetFetchRequestTest extends BaseRequestTest {
}
}
+ @Test
+ def testOffsetFetchRequestAllOffsetsSingleGroup(): Unit = {
Review Comment:
This one is based on `testOffsetFetchRequestSingleGroup`. I kept the exact same assertions.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dajac commented on a diff in pull request #13295: KAFKA-14744; NPE while converting OffsetFetch from version < 8 to version >= 8
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #13295:
URL: https://github.com/apache/kafka/pull/13295#discussion_r1115722278
##########
core/src/test/scala/unit/kafka/server/KafkaApisTest.scala:
##########
@@ -4208,6 +4208,77 @@ class KafkaApisTest {
assertEquals(expectedOffsetFetchResponse, response.data)
}
+ @ParameterizedTest
+ @ApiKeyVersionsSource(apiKey = ApiKeys.OFFSET_FETCH)
+ def testHandleOffsetFetchAllOffsetsWithSingleGroup(version: Short): Unit = {
+ // Version 0 gets offsets from Zookeeper. Version 1 does not support fetching all
+ // offsets request. We are not interested in testing these here.
+ if (version < 2) return
+
+ def makeRequest(version: Short): RequestChannel.Request = {
+ buildRequest(new OffsetFetchRequest.Builder(
+ "group-1",
+ false,
+ null, // all offsets.
Review Comment:
This is the important bit. We need to test with `null`.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dajac merged pull request #13295: KAFKA-14744; NPE while converting OffsetFetch from version < 8 to version >= 8
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac merged PR #13295:
URL: https://github.com/apache/kafka/pull/13295
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #13295: KAFKA-14744; NPE while converting OffsetFetch from version < 8 to version >= 8
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #13295:
URL: https://github.com/apache/kafka/pull/13295#issuecomment-1442014099
https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/requests/OffsetFetchRequest.java#L297
Could it produce NPE too? For example, server receives request from older client but the request fails due to authorization ( https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/server/KafkaApis.scala#L1362 )
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dajac commented on pull request #13295: KAFKA-14744; NPE while converting OffsetFetch from version < 8 to version >= 8
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #13295:
URL: https://github.com/apache/kafka/pull/13295#issuecomment-1442017344
> https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/requests/OffsetFetchRequest.java#L297
Topics is only nullable since version 2: https://github.com/apache/kafka/blob/trunk/clients/src/main/resources/common/message/OffsetFetchRequest.json#L41. So it should not happen there.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org