BytesRefArray.iterator(Comparator) throws NullPointerException for a null comparator instead of iterating in append order

BytesRefArray (core, @lucene.internal, @lucene.experimental) is an append-only list of byte strings. Its iterator(Comparator) javadoc says a non-null comparator gives sorted order and otherwise "the order is the same as the values were appended" (BytesRefArray.java:192-194), and iterator() is documented as "sugar for iterator(Comparator) with a null comparator" (:183). Since 8.5.0 the body is an unconditional sort, return iterator(sort(comp, false)); (:200). A null comparator is not a BytesRefComparator, so StringSorter takes its comparison sort, which calls cmp.compare at the first comparison (StringSorter.java:52-57, :102): with 2 or more values the call throws NullPointerException; with 0 or 1 values nothing is compared. Until 8.4.1 the body was comp == null ? null : sort(comp), and iterator() itself called iterator(null), so this path ran on every iterator() call for 8 years; LUCENE-9228 (a0b8f5c7c28, a doc-values update performance change) removed the check, moved iterator() to a new iterator((SortState) null) overload and left both javadocs unchanged. No Lucene code supplies a null comparator (OfflineSorter and InMemorySorter pass on the comparator they were built with, never null in the tree, and FieldUpdatesBuffer passes a SortState), so it went unnoticed; an application can still reach it by giving either sorter a null comparator. Executed: 8.0.0-8.4.1 iterate in append order, every release from 8.5.0 to 10.6.0 and main throws. A one-line null check restores the documented behaviour; correcting the javadoc is the alternative a maintainer may prefer.

• BytesRefArray (org.apache.lucene.util, core): an append-only list of BytesRef values stored in a ByteBlockPool. Marked @lucene.internal and @lucene.experimental (BytesRefArray.java:29-30).
• StringSorter (org.apache.lucene.util, core): the sorter BytesRefArray.sort uses since 9.9.0 (#12591). It radix-sorts when the comparator is a BytesRefComparator and otherwise falls back to an IntroSorter that calls the comparator.
• SortState: the result of BytesRefArray.sort(comp, stable), a permutation that iterator(SortState) replays.

1. iterator(Comparator) (BytesRefArray.java:198-201) is return iterator(sort(comp, false)); (:200). Nothing checks for null.
2. sort(comp, stable) (:128-180) fills a local int[] orderedEntries = new int[size()] (:129), wraps it in new StringSorter(comp) (:164; StableStringSorter at :136 when stable) and calls sorter.sort(0, size()) (:179).
3. StringSorter.sort (StringSorter.java:52-57) radix-sorts only if (cmp instanceof BytesRefComparator bCmp); null is not an instance of anything, so it calls fallbackSorter(cmp).sort(from, to) (:56). The fallback IntroSorter's compare (:98-103) calls cmp.compare(scratchBytes1, scratchBytes2) (:102).
4. For 2 values IntroSorter goes straight to insertion sort (IntroSorter.java:128), whose first step compares values 0 and 1 (Sorter.java:244): NullPointerException. With 0 or 1 values no pair is compared, so no exception.
5. The append-order code is one overload away: iterator(SortState) (:209-211) uses indices = sortState == null ? null : sortState.indices, with the same null rule in its javadoc (:203-207), and iterator() calls it with (SortState) null (:185). iterator(Comparator) just never routes null there.
6. Since 8.5 a bare array.iterator(null) does not compile: javac reports "reference to iterator is ambiguous ... both method iterator(Comparator<BytesRef>) in BytesRefArray and method iterator(SortState) in BytesRefArray match". A caller reaches the bug with a typed null: a cast, or a Comparator variable that is null when no order is wanted.

Two values b, a: iterator((Comparator<BytesRef>) null) throws on main, 10.6.0, 10.5.2 and 9.12.3, with assertions on and off. The history probe (values c, a, b) prints cab on all 8 releases from 8.0.0 to 8.4.1 (and on 4.8.0 and 7.7.3) and throws on all 52 releases from 8.5.0 to 10.5.2. The no-arg iterator(), a null comparator on 0 or 1 values, and any non-null comparator work.

• Who is hit. Code that uses BytesRefArray directly and passes a null comparator to mean "append order", as the javadoc invites, on 2 or more values. For example (illustrative, not found in any project): a helper that takes an optional Comparator<BytesRef> order and calls array.iterator(order), or code written for 8.4 or earlier as array.iterator(null), which stopped compiling in 8.5 and was fixed with a (Comparator<BytesRef>) cast.
• Also hit, indirectly. Code that builds an InMemorySorter (suggest module) or an OfflineSorter with a null comparator. Both store it without a check and pass it on to iterator(Comparator) (InMemorySorter.java:38-39, :51; OfflineSorter.java:219-221, :268, :637), so new InMemorySorter(null) and new OfflineSorter(dir, "tmp", null).sort(...) on b, a throw the same NullPointerException (executed on main, assertions on and off). Both classes are @lucene.internal and @lucene.experimental, and their constructors are documented as "sorting entries by the provided comparator" and "with a custom comparator", so a null there is outside their own documented use.
• What happens. That one call fails at once with an internal NullPointerException from StringSorter, which points at the sorter, not at the argument.
• What does not happen. No wrong results, no hang, no lasting resource use: before it throws, the call allocates a temporary int[size()] and a sorter (:129, :164), which become garbage. sort only permutes that local orderedEntries array, so the array's pool, offsets and size are untouched and it stays usable: after the exception on c, a, b, size() is still 3, iterator() gives cab, a naturalOrder() iterator gives abc, and a further append works (executed, assertions on and off). No index is written from this path.
• No Lucene code supplies a null comparator. OfflineSorter passes its configured comparator (OfflineSorter.java:637), and every new OfflineSorter( in the tree gives it a non-null one (the default naturalOrder(), BytesRefComparator.NATURAL or a suggester comparator); InMemorySorter passes its constructor's comparator (InMemorySorter.java:51), and both in-tree constructions use Comparator.naturalOrder() (main source FSTCompletionBuilder.java:133, test TestBytesRefSorters.java:56); FieldUpdatesBuffer sorts with BytesRefComparator.NATURAL and iterates with iterator(SortState) or iterator() (FieldUpdatesBuffer.java:226, 234, 325-328). No test or main source in the tree calls iterator with a null comparator.
• Default vs opt-in, scale. Opt-in: the application itself supplies the null comparator, to this internal utility or to one of the two internal sorters above. Any 2 values trigger it; data size plays no part. Likelihood: rare. No report of it since 8.5.0 (2020) was found (see Prior reports).

TestBytesRefArrayIteratorNullComparator creates new BytesRefArray(Counter.newCounter()), appends b and a, calls iterator((Comparator<BytesRef>) null) and asserts b, a, then null. A NullPointerException is rethrown as an AssertionError that states expected and actual, with the original as its cause. Two values are the smallest input that fails (0 or 1 values make no comparison), and b, a is not already sorted, so a pass shows append order rather than an accidental sort. Public API only (it sits in org.apache.lucene.util next to BytesRefArray, by Lucene convention); no randomness of its own, so the seed does not matter (seeds DEADBEEF and 5EED1234 gave the same failure). It compiles on 9.1 and later (it needs the org.apache.lucene.tests.util test framework).

As the regression test: this same test, for example as a new method in TestBytesRefArray. With fix A it passes, and ./gradlew :lucene:core:check -x test passes with it as a new test file. If the maintainer picks option B (reject null), its assertion becomes expectThrows(NullPointerException.class, () -> array.iterator((Comparator<BytesRef>) null)).

cp TestBytesRefArrayIteratorNullComparator.java $LUCENE/lucene/core/src/test/org/apache/lucene/util/
cd $LUCENE && ./gradlew :lucene:core:test --tests org.apache.lucene.util.TestBytesRefArrayIteratorNullComparator -Ptests.seed=DEADBEEF
# assertions off: add -Ptests.asserts=false (same result)

TestBytesRefArrayIteratorNullComparator > testNullComparatorIteratesInAppendOrder FAILED
    java.lang.AssertionError: iterator(null): expected append order b, a, got java.lang.NullPointerException: Cannot invoke "java.util.Comparator.compare(Object, Object)" because "this.val$cmp" is null
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:73D58901F5D8243]:0)
        at org.apache.lucene.util.TestBytesRefArrayIteratorNullComparator.testNullComparatorIteratesInAppendOrder(TestBytesRefArrayIteratorNullComparator.java:33)

        Caused by:
        java.lang.NullPointerException: Cannot invoke "java.util.Comparator.compare(Object, Object)" because "this.val$cmp" is null
            at org.apache.lucene.util.StringSorter$1.compare(StringSorter.java:102)
            at org.apache.lucene.util.Sorter.insertionSort(Sorter.java:244)
            at org.apache.lucene.util.IntroSorter.sort(IntroSorter.java:128)
            at org.apache.lucene.util.IntroSorter.sort(IntroSorter.java:44)
            at org.apache.lucene.util.StringSorter.sort(StringSorter.java:56)
            at org.apache.lucene.util.BytesRefArray.sort(BytesRefArray.java:179)
            at org.apache.lucene.util.BytesRefArray.iterator(BytesRefArray.java:200)
            at org.apache.lucene.util.TestBytesRefArrayIteratorNullComparator.testNullComparatorIteratesInAppendOrder(TestBytesRefArrayIteratorNullComparator.java:31)

# gradle, main 645d4fe14a20, -Ptests.seed=DEADBEEF -Ptests.asserts=false; identical with assertions on

• Contract, this exact method. iterator(Comparator): "If a non null Comparator is provided the iterator will iterate the byte values in the order specified by the comparator. Otherwise the order is the same as the values were appended." (BytesRefArray.java:192-194). iterator(): "sugar for iterator(Comparator) with a null comparator" (:183). iterator(SortState) states the same null rule (:203-207). No @throws, nothing says null is rejected.
• History: the null path worked for 8 years; then LUCENE-9228 changed it without updating the javadoc. f303bcd4650 (LUCENE-3807, 2012-03-02) wrote comp == null ? null : sort(comp) together with this javadoc (then BytesRefList in the suggest module; in core as BytesRefArray since 4.8.0). Until 8.4.1, iterator() was literally return iterator(null);, so every iterator() call used the null path. a0b8f5c7c28 (LUCENE-9228, "Sort dvUpdates by terms before apply", 2020-02-20) replaced the body with an unconditional sort, sent iterator() to the new iterator((SortState) null) and left both javadocs unchanged. After that no in-tree caller or test passed null, which is why nothing caught it.
• No sign of intent. The review (apache/lucene-solr#1263), the issue (#10268) and the later rewrite #12591 do not mention null comparators; CHANGES.txt and MIGRATE.md say nothing about it. That shows nobody documented or discussed a change to the null contract; it does not prove the change was an accident.
• Tests. No upstream test passes a null comparator; TestBytesRefArray uses iterator(), a non-null comparator (TestBytesRefArray.java:93-95) and iterator(SortState).

The strongest case for "not a bug" (the intended-behaviour checker's and the cross-model refute reviewer's): BytesRefArray is @lucene.internal and @lucene.experimental; the package-private interface it implements only says "Sort all values by the provided comparator and return an iterator over the sorted values" (SortableBytesRefArray.java:32-33); the sibling FixedLengthBytesRefArray.iterator documents no null case (FixedLengthBytesRefArray.java:153-162) and also sorts unconditionally (:166), so a null comparator throws the same NullPointerException there on 2 values (executed); and no Lucene code supplies null. So the null sentence may be a stale leftover and this a documentation defect.

Why it fails: the public javadoc of this exact method, and of iterator(), state the null behaviour and its result; the behaviour was implemented and exercised from 2012 to 8.4.1; it changed in a performance commit that updated neither javadoc and recorded no contract change; and @lucene.internal means the API may change between releases, not that a method may contradict its current javadoc. The sibling's behaviour supports the stale-leftover reading, but it does not change what this method's own javadoc promises. Residual: a maintainer may still prefer to drop the null case and fix the javadoc (option B). That is a choice of fix, not "works as designed".

Fix A was validated through gradle in a scratch worktree of 645d4fe14a20. Option B is the alternative the adjudication named; it was not run.

Skip the sort when the comparator is null and pass a null SortState, which iterator(SortState) already turns into append order. This is the pre-8.5 behaviour in today's code.

--- a/lucene/core/src/java/org/apache/lucene/util/BytesRefArray.java
+++ b/lucene/core/src/java/org/apache/lucene/util/BytesRefArray.java
@@ -197,7 +197,7 @@
    */
   @Override
   public BytesRefIterator iterator(final Comparator<BytesRef> comp) {
-    return iterator(sort(comp, false));
+    return iterator(comp == null ? null : sort(comp, false));
   }
 
   /**

Trade-offs

• Restores the documented behaviour of 2012 to 8.4.1 and makes iterator() truly "sugar for iterator(Comparator) with a null comparator" again; no javadoc change needed.
• No change for any non-null comparator, which is every in-tree caller.
• Cost: one null check per call; a null call also skips the int[size()] allocation and the sort.
• It keeps a null-means-unsorted convention that the sibling FixedLengthBytesRefArray and the package-private SortableBytesRefArray interface do not have.
• Coverage: the minimal test checks 2 values; the fix covers every size by construction, because a null comparator skips sort and goes to iterator((SortState) null), which iterates in append order (BytesRefArray.java:209-211, code reading).

What the checks validated

• Gradle, scratch worktree of 645d4fe14a20, seed DEADBEEF, 2026-11-01: without the fix the repro fails with assertions on and off; with it, it passes with assertions on and off.
• Closest core suites with the fix: TestBytesRefArray, TestFixedLengthBytesRefArray, TestStringSorter, TestOfflineSorter, TestFieldUpdatesBuffer and the repro: 6 classes, 35 tests, 0 failures, 2 skipped (nightly).
• ./gradlew :lucene:suggest:test --tests 'org.apache.lucene.search.suggest.fst.*' (InMemorySorter calls iterator(Comparator)): 3 classes, 24 tests, 0 failures, 1 skipped (nightly).
• ./gradlew :lucene:core:check -x test with the fix and the repro as a new test file: passes (google-java-format, ecj lint, forbidden APIs; error-prone skipped). The full Lucene test suite was not run.
• Earlier, with the fix shadowed into the stock jars: the finder's repro, the upstream TestBytesRefArray and the independent reproducer's 3 tests pass; the minimal repro passes with assertions on and off.

Reword iterator(Comparator) to say the comparator must not be null and point append-order callers to iterator(); reword iterator() to describe append order directly; optionally add Objects.requireNonNull(comp, "comp") at the top of iterator(Comparator).

Trade-offs

• Matches what 8.5.0 to 10.6.0 already do with 2 or more values, and the comparator-only wording of SortableBytesRefArray and FixedLengthBytesRefArray (neither says null is allowed or rejected; FixedLengthBytesRefArray throws the same NullPointerException for it).
• Removes a documented feature (of an internal API, so allowed between releases) instead of restoring it.
• With requireNonNull, a null call on 0 or 1 values, which works today, would start to throw; without it, the failure stays an obscure NPE inside StringSorter.
• The regression test becomes an expectThrows(NullPointerException.class, ...).

• Method: code reading, the hunt's non-formal tier, in the hunt unit on indexing-time pools (BytesRefHash, byte pools, BytesRefArray). The unit wrote a javadoc-derived reference model for BytesRefArray and compared it with the real class. That differential only exercised non-null comparators, so it did not find this; reading iterator(Comparator) while writing the model's null case showed the unconditional sort.
• Witness: a JUnit test on the stock main jar (three values c, a, b, plus a no-arg iterator() control) failed with the NPE while the control passed; a shadowed one-line fix made it pass.
• Later: an independent reproducer, an intended-behaviour devil's advocate, cross-model reviews and an adjudication checked it (2026-10-26); the history was bisected by execution over 60 releases; the test was reduced to one test method on two values (2026-10-29) and re-verified on main 645d4fe14a20.