showuon commented on code in PR #13100:
URL: https://github.com/apache/kafka/pull/13100#discussion_r1105435108
##########
clients/src/main/java/org/apache/kafka/common/protocol/types/TaggedFields.java:
##########
@@ -100,6 +100,13 @@ public NavigableMap<Integer, Object> read(ByteBuffer
buffer) {
}
prevTag = tag;
int size = ByteUtils.readUnsignedVarint(buffer);
+ if (size < 0 && isNullable())
+ return null;
+ else if (size < 0)
+ throw new SchemaException("field size " + size + " cannot be
negative");
+ if (size > buffer.remaining())
+ throw new SchemaException("Error reading field of size " +
size + ", only " + buffer.remaining() + " bytes available");
Review Comment:
@divijvaidya , I was following your suggestion to do the documentType
refactor like this
https://github.com/apache/kafka/compare/trunk...showuon:taggedFields_temp?expand=1
, and then, I found in `taggedFields`, the size validation is in nested way.
So, that makes the validation framework doesn't be that useful. Besides, after
the change, the child of `DocumentType` can still override the `read` method if
it wants (inherited from parent `Type` class), which makes it not useful. So I
reverted back to the original change, and add a comment to the `read` method.
Does that make sense?
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
