IndexWriter indexes a custom term frequency of 0 unchecked and commits a segment CheckIndex rejects

A field indexed with DOCS_AND_FREQS can take each token's frequency from TermFrequencyAttribute instead of counting occurrences. Lucene's two stock implementations of that attribute reject values below 1, but the interface states no range, and an application can plug in its own implementation through the Expert AttributeSource.addAttributeImpl or an AttributeFactory. IndexingChain.invertTokenStream checks every token's position increment and offsets before adding it, but checks the custom frequency only for overflow (IndexingChain.java:2053-2064 at 5adff78bb168); FreqProxTermsWriterPerField then stores the value as the posting's frequency (:125, :177, :196); on a DOCS_AND_FREQS field the only positivity guard is an assert (:143). With assertions off, a frequency of 0 is committed silently and CheckIndex rejects the segment (freq 0 is out of bounds). When a term's total frequency ends below its document count, the commit instead throws cannot write negative vLong (got: -1), which closes the writer and loses the uncommitted documents. Every release from 7.0.0 to 10.6.0 has it; stock analysis cannot trigger it.

1. Only the stock setters check the range. TermFrequencyAttribute (analysis/tokenattributes/TermFrequencyAttribute.java:23-35) states no range. TermFrequencyAttributeImpl.setTermFrequency (:30-36) and PackedTokenAttributeImpl.setTermFrequency (:157-164) throw "Term frequency must be 1 or greater". A custom class plugged in with addAttributeImpl ("Expert", util/AttributeSource.java:176-188) bypasses both; it has to be registered before the default packed implementation claims the interface, as the repro does.
2. The indexing chain checks the other token values, not this one. IndexingChain.invertTokenStream rejects a bad position increment or position overflow (index/IndexingChain.java:1996-2033) and bad offsets (:2035-2051) with non-aborting IllegalArgumentExceptions before the add. The custom frequency is only added to the field length with Math.addExact, "too many tokens" on overflow (:2053-2064): an upper bound, no lower bound. This is the fix site.
3. It is stored as is. termsHashPerField.add (:2075, inside the aborting catch (Throwable) at :2098-2100) reaches FreqProxTermsWriterPerField.newTerm / addTerm, which store getTermFreq() as the posting's frequency (index/FreqProxTermsWriterPerField.java:125, :177, :196). getTermFreq() (:208-219) returns termFreqAtt.getTermFrequency() and rejects only freq != 1 when positions are indexed.
4. On a DOCS_AND_FREQS field the only positivity guard is an assert. assert !hasFreq || postings.termFreqs[termID] > 0 (:143) runs only with -ea, and on the term's next occurrence, not on the document that carries the 0. Other index options do have checks, inside the add: with positions, getTermFreq() rejects any value other than 1 (:208-219); on a DOCS-only field addTerm rejects a custom frequency other than 1 (:147-152).
5. What the flush writes. The postings get freq 0 (term foo: docFreq 2, totalTermFreq 2, doc 0 freq 0, doc 1 freq 2) and CheckIndex treats freq <= 0 as corruption (index/CheckIndex.java:1735-1739). When a term's total frequency ends below its document count (one document with frequency 0: totalTermFreq 0, docFreq 1), the terms dictionary writes ttf - df as a vLong (codecs/lucene103/blocktree/Lucene103BlockTreeTermsWriter.java:528), which throws at store/DataOutput.java:238 during the commit; the exception is tragic and closes the writer.

With assertions on, an internal assert fires instead: FreqProxTermsWriterPerField.addTerm:143 when the frequency-0 document comes first, Lucene104PostingsWriter.startDoc:290 during flush when it comes second (the devil's-advocate verifier's test).

• Who. Code that writes its own TermFrequencyAttribute implementation (an Expert hook) without the stock range check, on a field indexed with frequencies. No public example was found: a GitHub code search for implements TermFrequencyAttribute (top 30 hits) found only copies of Lucene's own class, so the likelihood is rated contrived.
• Not hit. The two stock attributes throw for values below 1; DelimitedTermFrequencyTokenFilter calls the stock setter, so foo|0 is rejected; FeatureField rejects values below Float.MIN_NORMAL.
• Effect. Silent in production mode (assertions off): the bad value is committed and only CheckIndex notices. In the single-document case the commit fails and closes the writer: in the probe, after one committed document, a frequency-0 document and a valid one were added; the commit threw, the writer closed, and the index kept only the first document. How search and merge behave on a segment with freq 0 was not run.
• Default vs opt-in. Custom term frequencies are opt-in; nothing else is configured.
• Scale. Two one-token documents; deterministic, single thread.

TestCustomTermFreqBelowOne (one method, extends LuceneTestCase, package org.apache.lucene.index of the lucene/core tests, Apache license header). It passes checkGoogleJavaFormat. The test method draws no random values, though the test framework still picks locale, time zone and the default similarity from the seed; this test was run with seed DEADBEEF, and the hunt's longer version of it also failed with four other seeds (a separate re-run and the audit). Run it with -Ptests.asserts=false for the production-mode failure (CheckIndex); with assertions on, an internal AssertionError fires on the second addDocument instead. With fix A it passes unchanged, assertions on and off. As written it accepts any IllegalArgumentException from the first addDocument; a stricter version would also assert the message and that the index then holds 1 document (not run).

cp $REPRO/TestCustomTermFreqBelowOne.java $LUCENE/lucene/core/src/test/org/apache/lucene/index/
cd $LUCENE && ./gradlew :lucene:core:test --tests org.apache.lucene.index.TestCustomTermFreqBelowOne -Ptests.seed=DEADBEEF -Ptests.asserts=false

# gradle, scratch worktree at 5adff78bb168, -Ptests.seed=DEADBEEF -Ptests.asserts=false
TestCustomTermFreqBelowOne > testZeroCustomTermFrequency FAILED
    java.lang.RuntimeException: CheckIndex failed
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:BAD1EBD1119B1DB4]:0)
        at org.apache.lucene.tests.util.TestUtil.checkIndex(TestUtil.java:337)
        ...
  1>     test: terms, freq, prox...ERROR: org.apache.lucene.index.CheckIndex$CheckIndexException: term [66 6f 6f]: doc 0: freq 0 is out of bounds
  ...
  1> WARNING: 1 broken segments (containing 2 documents) detected

# gradle, same, assertions on
TestCustomTermFreqBelowOne > testZeroCustomTermFrequency FAILED
    java.lang.AssertionError
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:BAD1EBD1119B1DB4]:0)
        at org.apache.lucene.index.FreqProxTermsWriterPerField.addTerm(FreqProxTermsWriterPerField.java:143)
        at org.apache.lucene.index.TermsHashPerField.positionStreamSlice(TermsHashPerField.java:216)
        at org.apache.lucene.index.TermsHashPerField.add(TermsHashPerField.java:201)
        at org.apache.lucene.index.IndexingChain$PerField.invertTokenStream(IndexingChain.java:2075)
        ...

# with fix A: PASS with assertions on and off

# commit-failure probe (a separate main(), not the repro), assertions off, no fix:
# bar freq 1 committed; then foo freq 0 and baz freq 1 added; commit
second commit: java.lang.IllegalArgumentException: cannot write negative vLong (got: -1)
writer open: false
tragic exception: java.lang.IllegalArgumentException: cannot write negative vLong (got: -1)
documents in the index: 1
# with fix A: addDocument(foo, freq 0) throws IllegalArgumentException: term frequency must be 1 or greater (got 0) for field 'f'; commit ok; 2 documents

• Contract. TermFrequencyAttribute states no range; both stock implementations reject values below 1 (TermFrequencyAttributeImpl.java:30-36, PackedTokenAttributeImpl.java:157-164); FreqProxTermsWriterPerField asserts termFreqs > 0 (:143); CheckIndex reports freq <= 0 as out of bounds (CheckIndex.java:1735-1739). Michael McCandless on dev@ (2020-11-06, "Payloads for each term"): "term frequency 0 is not allowed".
• IndexWriter guards the index against bad token values. PositionIncrementAttributeImpl (:32-35) and PackedTokenAttributeImpl (:57-62) reject a negative increment, and IndexingChain still checks "position increment must be >= 0" (:2002-2008). Precedents: LUCENE-3874 (#4947, "don't let bogus positions corrumpt [sic] the index"), LUCENE-4127 ("don't let bogus offsets corrupt the index"), LUCENE-5977 (#7039, token stream safety checks in IndexWriter). The custom frequency's upper bound is checked too (6c3ece2b9f9, 2017).
• Tests. TestCustomTermFreq (15 tests) covers positions, DOCS-only fields, overflow and term vectors, but not a value below 1, because its streams use the stock attribute.
• History. LUCENE-7854 (d276acfbbcb, 2017) put the >= 1 check only into the setters; none of the 34 comments on #8905 discusses values below 1. The assert at :143 dates from 2012, when a frequency could only be counted up from 1.

The strongest case for "not a bug": the attribute is an Expert hook, an implementation that accepts 0 is broken, and Robert Muir wrote on #11086 (2021): "If you use custom term frequencies (expert feature), then it is your responsibility to have reasonable values. Otherwise, things can easily overflow, break scoring, cause failures from checkindex."

Why it fails: the interface documents no range; IndexWriter already re-checks the negative position increment, which only the same kind of custom code can produce; and in the same thread (2021-08-13) they wrote "It isn't viable to have an option that says 'allow me to corrupt my index'". The fix is one comparison on a value the chain already reads, and it rejects only the bad document. The argument does bear on likelihood, hence priority Low.

Option A was built and checked through gradle on 5adff78bb168; B to D were not run. A and B change behaviour only for a custom frequency below 1, which every stock producer already rejects; A applies to every inverted field, including DOCS_AND_CUSTOM_FREQS (see its trade-offs). If LFV-281 is fixed at the same time, B is the natural form.

Right after the offsets check in IndexingChain.invertTokenStream (IndexingChain.java:2051), read the custom frequency and throw an IllegalArgumentException when it is below 1. Like the position and offset checks above it, this runs before termsHashPerField.add, so it is non-aborting: the document is marked deleted, as for any analysis error, and the writer stays open. The message mirrors the stock setters.

          final int termFreq = invertState.termFreqAttribute.getTermFrequency();
          if (termFreq < 1) {
            throw new IllegalArgumentException(
                "term frequency must be 1 or greater (got "
                    + termFreq
                    + ") for field '"
                    + field.name()
                    + "'");
          }

Trade-offs

One more comparison per token on the indexing hot path (Lucene's SECURITY.md asks that robustness changes not slow hot paths). The value is read again a few lines below (IndexingChain.java:2059, on the branch for fields other than DOCS_AND_CUSTOM_FREQS, which adds it to the field length), so a final version can reuse the local there (not run). No benchmark was run. Scope: as written the check runs before the index-option branches, so it applies to every inverted field, not only DOCS_AND_FREQS. On DOCS-only and position-indexed fields the existing checks inside the aborting add (FreqProxTermsWriterPerField.java:147-152, :208-219) reject a frequency other than 1; the fix would reject a 0 before them, non-aborting. On DOCS_AND_CUSTOM_FREQS (10.5.0) custom frequencies are opaque scores (IndexOptions.java:52-55, FieldInfo.java:717-723), so decide whether 0 is a valid score there before rejecting it. With the fix only DOCS_AND_FREQS was run with a 0, and no repro covers a negative value, so this finding is about 0.

What the checks validated

• Gradle in a scratch worktree at 5adff78bb168, seed DEADBEEF: the repro fails without the fix (assertions on and off) and passes with it (on and off).
• With the fix: the repro, TestCustomTermFreq (15), TestFieldInvertState (2), TestTermsHashPerField (3), TestIndexWriterExceptions (40, 3 @Nightly skipped), TestFeatureField (14), TestFeatureSort (6), TestFeatureDoubleValues (6), TestPackedTokenAttributeImpl (4), TestSimilarity (3), TestReqOptSumScorer (8) and analysis-common's TestDelimitedTermFrequencyTokenFilter (3): 105 tests, 0 failures, 3 skipped. :lucene:core:checkGoogleJavaFormat passes. The whole core module was not run.
• The commit-failure probe with the fix: the frequency-0 document is rejected, the commit succeeds, the writer stays open and the index has 2 documents; CheckIndex is clean.

LFV-281's proposed fix adds a validateCustomTermFrequency step before the add whenever the frequency is not 1. A < 1 check fits there as its first line.

Trade-offs

Same behaviour as A, and no extra work for the common frequency of 1; it depends on LFV-281's change landing first or together with it.

Replace assert !hasFreq || postings.termFreqs[termID] > 0 with an exception.

Trade-offs

It runs inside the aborting termsHashPerField.add, so a bad document would close the writer and roll back everything since the last commit (the LFV-281 problem), and it fires on the term's next occurrence, not on the bad document.

State in the interface javadoc that the frequency must be 1 or greater.

Trade-offs

Cheapest, but IndexWriter would still commit a segment CheckIndex rejects. Worth adding together with A.

Method: semi-formal (a proof-obligation audit of a class's contract, witnessed by an executable repro).

• The finder read FreqProxTermsWriterPerField's obligation that a stored frequency is above 0 (:143, and CheckIndex at CheckIndex.java:1735-1739) and traced each place the value is set (:125, :177, :196) back to TermFrequencyAttribute.getTermFrequency(). The indexing chain discharges the obligation for positions and offsets with explicit checks, but for the frequency it relies on the stock setters, which an application's own implementation does not run.
• The finder then wrote a public-API JUnit test that fails on main with assertions on and off, and on 10.6.0 and 9.12.3.
• An independent reproducer, two executed cross-model reviews, a devil's-advocate verifier and an adjudication reproduced it with their own tests ("Prior reports, history and checks").