fix: index underflow panic in TelepenReader::setCounters

[jmortlock]

Problem

TelepenReader::setCounters panics with:

index out of bounds: the len is 80 but the index is 18446744073709551615

(at telepen_reader.rs:311 in 0.9.2, attempt to subtract with overflow in debug builds).

When a row never records a counter — e.g. an entirely-black row, where the initial "move to first white pixel" scan consumes the whole row — counterLength is still 0 when the trailing noise-merge block runs. That block evaluates self.counters[self.counterLength - 1], and 0usize - 1 underflows to usize::MAX.

We hit this in production scanning ~20k real-world product photos through MultiUseMultiFormatReader: 170 images reliably panicked the decode thread here.

Fix

The in-loop noise-merge already guards this exact case with || self.counterLength == 0. This PR applies the same guard to the identical trailing block after the loop, so the final count is appended instead of merged into a nonexistent previous counter. Decoding then fails gracefully with NotFoundException.

Test

Adds a minimal regression test (all_black_row_returns_not_found_instead_of_panicking): a 2048-wide all-black BitArray (wide enough that minToleratedWidth >= 2, putting the trailing count of 1 into the noise branch). It panics before the fix and returns NotFoundException after.

cargo test --lib oned::telepen and cargo test --test telepen_blackbox_1_test_case --features image,oned,decoders,image_formats both pass.

[jmortlock]

Some field data that may be useful context, from running this patch's format restriction across ~20,000 real product photographs.

On the panic's frequency: the unrestricted multi-format scan hit this underflow on 170 of 19,804 images (0.86%). Not an exotic edge case — ordinary retail packaging shots, where the Telepen reader was speculatively tried on rows that had no recorded transitions.

On the Telepen reader's precision, separately from the panic: across that same estate the reader produced exactly one successful decode, and its payload was

Nm)f\x05\x1F\x05a89

Control characters. The image's actual barcode was the EAN-13 9300617323419, decoded correctly by the EAN reader from the same picture. So the single Telepen "success" in 19,804 images was a misread of a neighbouring symbol.

That is not an argument against the reader — Telepen is a real symbology and someone scanning genuine Telepen labels needs it. But it does suggest the reader is currently prone to firing on non-Telepen input, and that the underflow this PR fixes is the crash-shaped tip of that: setCounters reaching the trailing noise-merge with counterLength == 0 means it found no usable structure at all, which is exactly what you would expect when pointed at something that is not a Telepen code.

Happy to open a separate issue for the precision question if that is useful, or to add a stricter start/stop-pattern guard in this PR if you would prefer them together. Left as-is for now since a minimal, reviewable fix for the panic seemed the better first contribution.

[axxel]

@jmortlock I'm the maintainer of the zxing-cpp project and your 19k test image set caught my attention. Would you be in the position to do any/all of the following:

1. share your test image set
2. share the one image that triggered the Telepen false positive
3. run your test-set through https://crates.io/crates/zxing-cpp and see how it compares to rxing (see also this benchmark as an appetizer for the effort ;)).

[jmortlock]

Hi @axxel

I'm not in a position to share the entire set as its licensed data, I did run the failing image through your library and it worked fine.

             │ rxing 0.9.2              │ zxing-cpp 0.5.3          
─────────────┼──────────────────────────┼──────────────────────────
Telepen only │ Nm)f\x05\x1f\x05a89      │ 0 results                
All formats  │ TELEPEN phantom + EAN_13 │ EAN-13 9300617323419 only

I did find a publicly available image which is the same image just lower resolution (I'm using an original 5025x5025 JPEG) and interestingly it works fine on both libraries.

https://assets.woolworths.com.au/images/1005/98361_2.jpg?impolicy=wowsmkqiema

[axxel]

I'm not in a position to share the entire set as its licensed data, I did run the failing image through your library and it worked fine.

Thanks for the feedback. If you have the time and assuming your image set is available in a folder of standard image files, could you do me the favor and run it through the above linked benchmark and share the results?

git clone https://github.com/axxel/zxing-bench.git
cd zxing-bench/rust
cargo run --release -- <image-folder>

I did find a publicly available image which is the same image just lower resolution (I'm using an original 5025x5025 JPEG) and interestingly it works fine on both libraries.

That file seems too low res for both rxing and zxing-cpp. Are you sure you used that one for your test?

[jmortlock]

@axxel You are right, and my earlier note was wrong. Apologies.

I re-checked that public file just now. It is 1200x1200, and it decodes in neither library:

                          │ rxing 0.9.2         │ zxing-cpp 0.5.3
──────────────────────────┼─────────────────────┼─────────────────
all formats, TryHarder    │ NotFoundException   │ 0 results
all formats, no TryHarder │ NotFoundException   │ —

(zxing-cpp built with features = ["bundled"], AllLinear + QRCode + DataMatrix, try_harder/try_rotate/try_invert all on.)

The comparison table I posted was produced against the 5025x5025 original, not that URL. I went looking for a public stand-in for an image I cannot share, found one of visibly the same product shot, and reported it as equivalent without re-running the test against it. That was sloppy of me. 1200x1200 also appears to be the maximum that host will serve publicly — the other impolicy values I tried return 424-byte Access Denied pages — so I do not currently have a shareable file that reproduces the phantom.

So the accurate statement is narrower than what I wrote: at 5025x5025, rxing 0.9.2 returns a phantom TELEPEN result alongside the correct EAN-13, and zxing-cpp 0.5.3 returns only the EAN-13. At 1200x1200 neither library finds anything, so that file says nothing either way.

On the benchmark: I would like to run it, and there is a constraint I should be upfront about rather than quietly not replying. The set is ~19,800 licensed supplier photographs. I cannot share the images, and they are not sitting in a local folder — they live behind a licensor CDN, so a full pass means ~20k third-party fetches, which is not my call alone to spend.

What I can realistically offer, if useful to you:

1. Aggregate numbers only — zxing-bench timings plus per-format detection counts for both libraries over the full set, with no images leaving our side.
2. A focused subset rather than all 19.8k, if that is enough for what you want to learn.

I will check whether the re-fetch is acceptable on our side and come back to you either way. If the aggregate output would still be worth having under those terms, say which of the two is more useful and I will aim for it.

[axxel]

@jmortlock Thanks for the detailed explanation. I simply assumed you had local access to those mentioned 19k images. I'd be happy to get some aggregate results on any subset you might still have or easily get access to.

[hschimke]

Sorry it took me so long to look at this!

I am probably the least familiar with how the Telepen reader works, as it was contributed by someone rather than me porting / building it.

Regarding your question, I think having a stricter start/stop in this PR would be a good bundle if you don't mind adding that. What you have in it as of today looks great and pending passing integration tests I'd call it approved.

[jmortlock]

Thanks @hschimke, and no problem at all. Stricter start/stop added in 9999bba.

It turned out to be more than tightening a threshold. There are three separate defects on the start side, each of which independently defeats the check:

1. The wide element was never checked. The start pattern is ten narrow elements followed by one wide, but the wide test sat in an else if j == 10 nested inside if j < 10 — unreachable. Only the ten narrow elements were ever tested, so any run of ten narrow elements matched a start pattern.

2. The threshold was minBar + maxBar / 2.0, which is the midpoint only when minBar is zero. With a narrow width of 2 and a wide of 6 that gives 5.0 rather than 4.0, so elements of width 5 passed as narrow.

3. A uniform window passed vacuously. With minBar == maxBar every element compares equal to the threshold and satisfies both the narrow and the wide test. A flat run of elements is precisely what noise looks like, and it matched.

The stop side had no structural check at all. findEndPattern finds the end by looking for a quiet zone — an element half again wider than anything nearby — which any sufficiently isolated dark run satisfies, and it accepts whatever precedes it. So on a picture with no Telepen symbol in it, the _ / z / checksum guards were the only thing between noise and a decode. checkStopPattern now requires the symbol to end in the run of narrow elements that the stop character ends with.

One deliberate limitation worth flagging for review, since it is a judgement call rather than a fact. I first implemented the stop as the exact mirror of the start (one wide, ten narrow). That rejected real symbols in test_resources/blackbox/telepen-1. Dumping the decoded element structure of that corpus shows the actual trailing run is two wide then nine narrow, and how many wide elements survive binarisation varies with the trailing quiet zone. So the check is on the trailing narrow run only, with the count taken from that corpus rather than assumed. If you know the symbology better than the corpus does, that constant is the thing to second-guess.

Everything is classified against the midpoint of the region being decoded — the same rule decode_row applies a few lines later — so none of this rejects anything the subsequent narrow/wide categorisation would have accepted anyway.

On evidence. Each of the four new tests fails on main and passes here, and genuine_start_pattern_is_found is included as a positive control that passes on both, so the suite is not trivially satisfied. I also ran every non-Telepen image in test_resources/blackbox (1,250 images, 61 directories) through the Telepen reader alone, before and after: zero false decodes either way. I am reporting that as a null result rather than omitting it — the checksum already catches this corpus end-to-end, so it does not demonstrate an improvement. The gain is at the structural gate, which is what the unit tests pin directly.

telepen_blackbox_1_test_case still passes for alpha and numeric at both 0 and 180 degrees. The full --features image,oned,decoders,image_formats suite passes: 659 lib tests plus every blackbox target. cargo clippy reports the same 18 pre-existing warnings as main, and cargo fmt --all is clean.

[hschimke]

The let median = minBar + maxBar / 2.0; is sending me, I can't believe I didn't catch that!