Copilot commented on code in PR #17077:
URL: https://github.com/apache/pinot/pull/17077#discussion_r2462032698
##########
pinot-plugins/pinot-input-format/pinot-avro/src/main/java/org/apache/pinot/plugin/inputformat/avro/SimpleAvroMessageDecoder.java:
##########
@@ -87,7 +101,14 @@ public GenericRow decode(byte[] payload, GenericRow
destination) {
*/
@Override
public GenericRow decode(byte[] payload, int offset, int length, GenericRow
destination) {
- _binaryDecoderToReuse = DecoderFactory.get().binaryDecoder(payload,
offset, length, _binaryDecoderToReuse);
+ int effectiveOffset = offset + _leadingBytesToStrip;
+ int effectiveLength = length - _leadingBytesToStrip;
+ if (effectiveLength < 0) {
+ throw new IllegalArgumentException("Configured '" +
LEADING_BYTES_TO_STRIP + "' (" + _leadingBytesToStrip
+ + ") exceeds available payload length (" + length + ")");
Review Comment:
The error message incorrectly states that the configured value exceeds the
payload length when it actually exceeds the available length after offset
adjustment. When offset is non-zero, a _leadingBytesToStrip value smaller than
the total payload length could still trigger this error. Consider clarifying:
'Configured leading.bytes.to.strip (%d) exceeds available length (%d) at offset
%d'.
```suggestion
throw new IllegalArgumentException(String.format(
"Configured leading.bytes.to.strip (%d) exceeds available length
(%d) at offset %d",
_leadingBytesToStrip, length, offset));
```
##########
pinot-plugins/pinot-input-format/pinot-avro/src/main/java/org/apache/pinot/plugin/inputformat/avro/SimpleAvroMessageDecoder.java:
##########
@@ -87,7 +101,14 @@ public GenericRow decode(byte[] payload, GenericRow
destination) {
*/
@Override
public GenericRow decode(byte[] payload, int offset, int length, GenericRow
destination) {
Review Comment:
Integer overflow risk: when offset is near Integer.MAX_VALUE, adding
_leadingBytesToStrip could overflow to a negative value, bypassing the
effectiveLength check. Validate that offset + _leadingBytesToStrip does not
overflow before computing effectiveOffset.
```suggestion
public GenericRow decode(byte[] payload, int offset, int length,
GenericRow destination) {
// Check for integer overflow when adding offset and _leadingBytesToStrip
if (_leadingBytesToStrip > 0 && offset > Integer.MAX_VALUE -
_leadingBytesToStrip) {
throw new IllegalArgumentException("Sum of offset (" + offset + ") and
'" + LEADING_BYTES_TO_STRIP + "' (" + _leadingBytesToStrip
+ ") causes integer overflow");
}
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
