fix(util): reject LiveDocs backing sets with bits beyond maxDoc

lucene/CHANGES.txt

@@ -229,6 +229,10 @@ Optimizations
 
 Bug Fixes
 ---------------------
+* GITHUB#16768: DenseLiveDocs and SparseLiveDocs now reject a backing bit set with bits set at or
+  beyond maxDoc. Such bits skewed deletedCount and made the live and deleted doc iterators return
+  doc ids past maxDoc. (Salvatore Campagna)
+
 * GITHUB#16657: Fix hashCode/equals contract violation in LRUQueryCache's QueryCacheKey and account a
   cached query's RAM once per distinct query. (Sagar Upadhyaya)
 

lucene/core/src/java/org/apache/lucene/util/DenseLiveDocs.java

@@ -58,8 +58,9 @@ public final class DenseLiveDocs implements LiveDocs {
   /**
    * Creates a builder for constructing DenseLiveDocs instances.
    *
-   * @param liveDocs bit set where set bits represent LIVE documents
-   * @param maxDoc the maximum document ID (exclusive)
+   * @param liveDocs bit set where set bits represent LIVE documents; may be longer than maxDoc, but
+   *     must have no bits set at or beyond maxDoc
+   * @param maxDoc the maximum document ID (exclusive), must not be negative
    * @return a new builder instance
    */
   public static Builder builder(FixedBitSet liveDocs, int maxDoc) {
@@ -92,9 +93,19 @@ public Builder withDeletedCount(int deletedCount) {
      * Builds the DenseLiveDocs instance.
      *
      * @return a new DenseLiveDocs instance
-     * @throws IllegalArgumentException if deletedCount is outside valid range [0, maxDoc]
+     * @throws IllegalArgumentException if maxDoc is negative, if liveDocs has bits set at or beyond
+     *     maxDoc, or if deletedCount is outside valid range [0, maxDoc]
      */
     public DenseLiveDocs build() {
+      if (maxDoc < 0) {
+        throw new IllegalArgumentException("maxDoc must not be negative: " + maxDoc);
+      }
+
+      if (liveDocs.length() > maxDoc
+          && liveDocs.nextSetBit(maxDoc) != DocIdSetIterator.NO_MORE_DOCS) {
+        throw new IllegalArgumentException("liveDocs has bits set at or beyond maxDoc=" + maxDoc);
+      }
+
       int count = deletedCount != null ? deletedCount : (maxDoc - liveDocs.cardinality());
 
       if (count < 0 || count > maxDoc) {

lucene/core/src/java/org/apache/lucene/util/SparseLiveDocs.java

@@ -57,8 +57,9 @@ public final class SparseLiveDocs implements LiveDocs {
   /**
    * Creates a builder for constructing SparseLiveDocs instances.
    *
-   * @param deletedDocs bit set where set bits represent DELETED documents
-   * @param maxDoc the maximum document ID (exclusive)
+   * @param deletedDocs bit set where set bits represent DELETED documents; may be longer than
+   *     maxDoc, but must have no bits set at or beyond maxDoc
+   * @param maxDoc the maximum document ID (exclusive), must not be negative
    * @return a new builder instance
    */
   public static Builder builder(SparseFixedBitSet deletedDocs, int maxDoc) {
@@ -91,9 +92,20 @@ public Builder withDeletedCount(int deletedCount) {
      * Builds the SparseLiveDocs instance.
      *
      * @return a new SparseLiveDocs instance
-     * @throws IllegalArgumentException if deletedCount is outside valid range [0, maxDoc]
+     * @throws IllegalArgumentException if maxDoc is negative, if deletedDocs has bits set at or
+     *     beyond maxDoc, or if deletedCount is outside valid range [0, maxDoc]
      */
     public SparseLiveDocs build() {
+      if (maxDoc < 0) {
+        throw new IllegalArgumentException("maxDoc must not be negative: " + maxDoc);
+      }
+
+      if (deletedDocs.length() > maxDoc
+          && deletedDocs.nextSetBit(maxDoc) != DocIdSetIterator.NO_MORE_DOCS) {
+        throw new IllegalArgumentException(
+            "deletedDocs has bits set at or beyond maxDoc=" + maxDoc);
+      }
+
       int count = deletedCount != null ? deletedCount : deletedDocs.cardinality();
 
       if (count < 0 || count > maxDoc) {

lucene/core/src/test/org/apache/lucene/util/TestLiveDocs.java

@@ -754,6 +754,92 @@ public void testApplyMaskOffsetBeyondMaxDoc() {
     }
   }
 
+  public void testPaddedBackingSetStaysWithinMaxDoc() throws IOException {
+    for (int iter = atLeast(5); iter > 0; iter--) {
+      for (int maxDoc : boundaryMaxDocs()) {
+        int padding = random().nextInt(256);
+        FixedBitSet deleted = randomBitSet(maxDoc, random().nextDouble());
+        List<Integer> expectedDeleted =
+            collectDocs(new BitSetIterator(deleted, deleted.cardinality()));
+        List<Integer> expectedLive = new ArrayList<>();
+        for (int doc = 0; doc < maxDoc; doc++) {
+          if (deleted.get(doc) == false) {
+            expectedLive.add(doc);
+          }
+        }
+
+        for (LiveDocs liveDocs : liveDocsViews(maxDoc, deleted, padding)) {
+          assertEquals(maxDoc, liveDocs.length());
+          assertEquals(expectedDeleted.size(), liveDocs.deletedCount());
+          assertEquals(expectedDeleted, collectDocs(liveDocs.deletedDocsIterator()));
+          assertEquals(expectedLive, collectDocs(liveDocs.liveDocsIterator()));
+        }
+      }
+    }
+  }
+
+  public void testBuilderRejectsBitsBeyondMaxDoc() {
+    for (int iter = atLeast(5); iter > 0; iter--) {
+      for (int maxDoc : boundaryMaxDocs()) {
+        for (int padding : new int[] {1, 2, TestUtil.nextInt(random(), 3, 256)}) {
+          int last = maxDoc + padding - 1;
+          assertRejectsBitBeyondMaxDoc(maxDoc, padding, maxDoc);
+          if (last > maxDoc) {
+            assertRejectsBitBeyondMaxDoc(maxDoc, padding, last);
+          }
+          if (last > maxDoc + 1) {
+            assertRejectsBitBeyondMaxDoc(
+                maxDoc, padding, TestUtil.nextInt(random(), maxDoc + 1, last - 1));
+          }
+        }
+      }
+    }
+  }
+
+  public void testBuilderRejectsNegativeMaxDoc() {
+    int maxDoc = -TestUtil.nextInt(random(), 1, 1000);
+    FixedBitSet live = new FixedBitSet(128);
+    SparseFixedBitSet deletedDocs = new SparseFixedBitSet(128);
+
+    assertEquals(
+        "maxDoc must not be negative: " + maxDoc,
+        expectThrows(
+                IllegalArgumentException.class, () -> DenseLiveDocs.builder(live, maxDoc).build())
+            .getMessage());
+    assertEquals(
+        "maxDoc must not be negative: " + maxDoc,
+        expectThrows(
+                IllegalArgumentException.class,
+                () -> SparseLiveDocs.builder(deletedDocs, maxDoc).build())
+            .getMessage());
+  }
+
+  private static void assertRejectsBitBeyondMaxDoc(int maxDoc, int padding, int beyond) {
+    FixedBitSet live = new FixedBitSet(maxDoc + padding);
+    live.set(0, maxDoc);
+    live.set(beyond);
+    assertEquals(
+        "liveDocs has bits set at or beyond maxDoc=" + maxDoc,
+        expectThrows(
+                IllegalArgumentException.class, () -> DenseLiveDocs.builder(live, maxDoc).build())
+            .getMessage());
+
+    SparseFixedBitSet deletedDocs = new SparseFixedBitSet(maxDoc + padding);
+    deletedDocs.set(beyond);
+    assertEquals(
+        "deletedDocs has bits set at or beyond maxDoc=" + maxDoc,
+        expectThrows(
+                IllegalArgumentException.class,
+                () -> SparseLiveDocs.builder(deletedDocs, maxDoc).build())
+            .getMessage());
+  }
+
+  private static int[] boundaryMaxDocs() {
+    return new int[] {
+      1, 2, 63, 64, 65, 127, 128, 129, 4095, 4096, 4097, TestUtil.nextInt(random(), 1, 8192)
+    };
+  }
+
   /**
    * Wraps a {@link Bits} instance so that {@link Bits#applyMask} resolves to the default
    * implementation, which is the specification that overrides must match.