jpountz commented on code in PR #13376:
URL: https://github.com/apache/lucene/pull/13376#discussion_r1604452451
##########
lucene/test-framework/src/java/org/apache/lucene/tests/store/BaseDirectoryTestCase.java:
##########
@@ -1442,6 +1442,19 @@ public void testListAllIsSorted() throws IOException {
}
}
+ public void testGroupVIntOverflow() throws IOException {
+ try (Directory dir = getDirectory(createTempDir("testGroupVIntOverflow")))
{
+ final int v = 1 << 30;
+ final long[] values = new long[4];
+ values[0] = v;
+ values[0] <<= 1; // values[0] = 2147483648 as long, but as int it is
-2147483648
Review Comment:
why not do `values[0] = 1L << 31` directly?
##########
lucene/core/src/java/org/apache/lucene/util/GroupVIntUtil.java:
##########
@@ -118,6 +120,13 @@ private static int numBytes(int v) {
return Integer.BYTES - (Integer.numberOfLeadingZeros(v | 1) >> 3);
}
+ private static int toInt(long value) {
+ if (value < 0 || value > 0xFFFFFFFFL) {
+ throw new ArithmeticException("integer overflow");
+ }
Review Comment:
Maybe use `Long.compareUnsigned`? (`if (Long.compareUnsigned(value,
0xFFFFFFFFL) > 0)`)
##########
lucene/core/src/java/org/apache/lucene/store/DataOutput.java:
##########
@@ -328,9 +328,12 @@ public void writeSetOfStrings(Set<String> set) throws
IOException {
/**
* Encode integers using group-varint. It uses {@link DataOutput#writeVInt
VInt} to encode tail
* values that are not enough for a group. we need a long[] because this is
what postings are
- * using, all longs are actually required to be integers.
+ * using, all longs are actually required to be integers. Negative numbers
are supported, but
+ * should be avoided.
*
- * @param values the values to write
+ * @param values the values to write. Note: if original integer is negative,
it should also be
+ * negative as long, not positive which is greater than
Integer.MAX_VALUE, that will cause
+ * integer overflow exception in {@link Math#toIntExact(long)}.
Review Comment:
Let's not mention this implementation detail.
```suggestion
* integer overflow exception.
```
--
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: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
