[Question] Concern about RkRecordHandler being unreachable #1252

Activity

1. Mikkey-f commented on Oct 22, 2026

   

   Mikkey-f

   on Oct 22, 2026

   Contributor

   More actions

   Hi @nkuprins — I dug into this on POI 5.5.1 and fesod's event path. Short answer: for every well-formed workbook, RkRecordHandler is unreachable; there is one narrow malformed-stream exception.

   Records are converted before any listener runs

   fesod's only XLS data path is XlsSaxAnalyser.execute(), which calls HSSFEventFactory.processWorkbookEvents() (XlsSaxAnalyser.java:190-195). That method goes through HSSFEventFactory.genericProcessEvents(), which wraps the stream in RecordFactoryInputStream (HSSFEventFactory.java:166). RecordFactoryInputStream.readNextRecord() converts:

   • RKRecord → NumberRecord (RecordFactoryInputStream.java:311-313)
   • MulRKRecord → NumberRecord[] (:315-321)

   before req.processRecord(r) is called. So RK cells in a real workbook are handled by NumberRecordHandler, which reads both the value and the format (NumberRecordHandler.java:52-69).

   The exception: records read during the encryption probe bypass conversion

   RecordFactoryInputStream's constructor runs StreamEncryptionInfo, which reads the BOF and then one or two more records to detect FILEPASS/WRITEPROTECT, and puts them in _unreadRecordBuffer (RecordFactoryInputStream.java:49-103). nextRecord() returns buffered records directly, without readNextRecord() (:245-260). I confirmed with a small POI-only probe:

   • BOF, RK(42), EOF → listener receives RKRecord (unconverted)
   • BOF, WRITEPROTECT, RK(42), EOF → listener receives RKRecord (the 3rd record is also buffered)
   • BOF, DATEMODE, RK(42), EOF → listener receives NumberRecord value=42.0

   In a valid workbook these early records are globals (INTERFACEHDR, WRITEPROTECT, DATEMODE, …), never cell records, so this window cannot hold an RK cell. A crafted/malformed stream could put one there. If it does, it arrives before any worksheet BOF — fesod has no sheet context yet — so the current EMPTY branch is not what a real read would produce. MulRKRecord is additionally expanded by MissingRecordAwareHSSFListener before the analyser sees it, so RK packs never reach the handler either.

   So

   • The "older BIFF files silently drop RK values" symptom is not reproducible on valid files — POI's own javadoc says the same ("HSSF converts these to NUMBER records").
   • RkRecordHandler is dead for valid input, but not strictly unreachable (the malformed 2nd/3rd-record window), so removing the registration is not a pure no-op for adversarial input.
   • Fix#1213 decode XLS RKRecord numeric cells instead of treating them as empty #1214's test calls the handler directly, so it does not demonstrate the reported data loss or exercise the read path. The decode change itself is harmless defensive coding.

   I'd suggest re-scoping #1213 to "harden the practically-unreachable handler" rather than data loss, or closing it if you prefer to remove the dead handler instead. Happy to be corrected if there's a read path I missed.