You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/10/27 20:46:36 UTC
[GitHub] [kafka] mjsax opened a new pull request #9513: MINOR: improve `null` checks for headers
mjsax opened a new pull request #9513:
URL: https://github.com/apache/kafka/pull/9513
Headers are not allowed to be `null`, and thus the code does not check for `null` headers. However, we don't have proper guards in place and had NPE in the past if a header was `null`. This PR adds additional `null` checks to avoid that users create corrupted headers.
Cf https://issues.apache.org/jira/browse/KAFKA-8142 and https://issues.apache.org/jira/browse/KAFKA-10645
Call for review @mimaison
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on pull request #9513: MINOR: improve `null` checks for headers
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #9513:
URL: https://github.com/apache/kafka/pull/9513#issuecomment-718125704
@chia7712 Good call about `ConnectHeaders`. I am not sure what we would need to change in `ConsumerRecord` though? Can you elaborate?
@showuon: what PR are you talking about?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9513: MINOR: improve `null` checks for headers
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9513:
URL: https://github.com/apache/kafka/pull/9513#issuecomment-718309714
> I am not sure what we would need to change in ConsumerRecord though? Can you elaborate?
this line (https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerRecord.java#L173). However, it may be overkill since users don't create ```ConsumerRecord```.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax merged pull request #9513: MINOR: improve `null` checks for headers
Posted by GitBox <gi...@apache.org>.
mjsax merged pull request #9513:
URL: https://github.com/apache/kafka/pull/9513
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on pull request #9513: MINOR: improve `null` checks for headers
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #9513:
URL: https://github.com/apache/kafka/pull/9513#issuecomment-717804120
LGTM. I'll close my PR then.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 edited a comment on pull request #9513: MINOR: improve `null` checks for headers
Posted by GitBox <gi...@apache.org>.
chia7712 edited a comment on pull request #9513:
URL: https://github.com/apache/kafka/pull/9513#issuecomment-718309714
> I am not sure what we would need to change in ConsumerRecord though? Can you elaborate?
this line (https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerRecord.java#L173). However, it may be overkill since users don't create ```ConsumerRecord```.
> what PR are you talking about?
see #9518
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on pull request #9513: MINOR: improve `null` checks for headers
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #9513:
URL: https://github.com/apache/kafka/pull/9513#issuecomment-719085653
> However, it may be overkill since users don't create ConsumerRecord.
Yeah, I tend to agree. Would not hurt to add it, but should not be strictly necessary. Will merge this as-is.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org