[ML] Better handling of invalid JSON state documents by edsavage · Pull Request #2895 · elastic/ml-cpp

edsavage · 2026-01-26T03:32:16Z

Various changes to the handling of errors when parsing JSON state documents to improve consistency and provide better visibility

Relates #2875

Various changes to the handling of errors when parsing JSON state documents to improve consistency and provide better visibility

prodsecmachine · 2026-01-26T03:32:33Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

valeriy42

It looks mostly good. I have just a few minor comments.

valeriy42 · 2026-01-26T10:52:58Z

+                message = "Encountered NULL character in stream before parsing has started.";
                ret = false;
            }
+            if (m_Reader->handler().s_Type == SBoostJsonHandler::E_TokenObjectEnd) {


Since both if statements could be true, you would potentially reassign message. I guess it should be either else if here, or the messages should be concatenated.

valeriy42

LGTM

The isEof() implementation was changed from peek()-based to eof()-based in elastic#2895, but eof() is a lagging indicator on Windows - it is not set until a read past the end is attempted. This caused CFieldDataCategorizerTest/testRestoreFromBadState and testRestoreStateRecoversWithEmptyState to fail on Windows. Two fixes: 1. isEof() now falls back to peek() when eof() returns false, making the check portable across platforms. 2. The empty array detection in start() no longer depends on isEof() at all - the token sequence [,] is sufficient to identify an empty array without checking stream state. Tested on macOS ARM, Linux aarch64, and Windows x86_64 - all 800 tests pass on all three platforms. Co-authored-by: Cursor <cursoragent@cursor.com>

The isEof() implementation was changed from peek()-based to eof()-based in #2895, but eof() is a lagging indicator on Windows - it is not set until a read past the end is attempted. This caused CFieldDataCategorizerTest/testRestoreFromBadState and testRestoreStateRecoversWithEmptyState to fail on Windows. Two fixes: 1. isEof() now falls back to peek() when eof() returns false, making the check portable across platforms. 2. The empty array detection in start() no longer depends on isEof() at all - the token sequence [,] is sufficient to identify an empty array without checking stream state. Tested on macOS ARM, Linux aarch64, and Windows x86_64 - all tests pass on all three platforms. --------- Co-authored-by: Cursor <cursoragent@cursor.com>

…c#2898) The isEof() implementation was changed from peek()-based to eof()-based in elastic#2895, but eof() is a lagging indicator on Windows - it is not set until a read past the end is attempted. This caused CFieldDataCategorizerTest/testRestoreFromBadState and testRestoreStateRecoversWithEmptyState to fail on Windows. Two fixes: 1. isEof() now falls back to peek() when eof() returns false, making the check portable across platforms. 2. The empty array detection in start() no longer depends on isEof() at all - the token sequence [,] is sufficient to identify an empty array without checking stream state. Tested on macOS ARM, Linux aarch64, and Windows x86_64 - all tests pass on all three platforms. --------- Co-authored-by: Cursor <cursoragent@cursor.com>

…3082) The LOG_ERROR at readHeader() for missing or empty compressed state documents was never addressed in #2895 and is redundant with parseNext() diagnostics. Downgrade to INFO so routine categorizer state restore misses no longer trip the serverless ERROR log-rate promotion gate. Relates #2875

…3082) (#3093) The LOG_ERROR at readHeader() for missing or empty compressed state documents was never addressed in #2895 and is redundant with parseNext() diagnostics. Downgrade to INFO so routine categorizer state restore misses no longer trip the serverless ERROR log-rate promotion gate. Relates #2875 (cherry picked from commit bfaa5cd) Co-authored-by: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com>

…3082) (#3092) The LOG_ERROR at readHeader() for missing or empty compressed state documents was never addressed in #2895 and is redundant with parseNext() diagnostics. Downgrade to INFO so routine categorizer state restore misses no longer trip the serverless ERROR log-rate promotion gate. Relates #2875 (cherry picked from commit bfaa5cd) Co-authored-by: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com>

…e token ID (#3143) An inconsistent or truncated categorizer state document can leave a restored category referencing a token ID at or beyond the end of the restored token ID lookup. That ID was later used to index the token ID lookup unchecked (for example when building a reverse search), which is an out-of-bounds access that can crash the autodetect process with a SIGSEGV inside libc rather than failing the restore. Validate, at the end of CTokenListDataCategorizerBase::acceptRestoreTraverser, that every token ID referenced by a restored category exists in the restored token ID lookup, and fail the restore gracefully if not. This is consistent with the graceful invalid-state handling added in #2895/#2898. Relates to #2875 Co-authored-by: Cursor <cursoragent@cursor.com>

…e token ID (#3143) (#3152) An inconsistent or truncated categorizer state document can leave a restored category referencing a token ID at or beyond the end of the restored token ID lookup. That ID was later used to index the token ID lookup unchecked (for example when building a reverse search), which is an out-of-bounds access that can crash the autodetect process with a SIGSEGV inside libc rather than failing the restore. Validate, at the end of CTokenListDataCategorizerBase::acceptRestoreTraverser, that every token ID referenced by a restored category exists in the restored token ID lookup, and fail the restore gracefully if not. This is consistent with the graceful invalid-state handling added in #2895/#2898. Relates to #2875 (cherry picked from commit 0f41e80) Co-authored-by: Ed Savage <ed.savage@elastic.co> Co-authored-by: Cursor <cursoragent@cursor.com>

…e token ID (#3143) (#3151) An inconsistent or truncated categorizer state document can leave a restored category referencing a token ID at or beyond the end of the restored token ID lookup. That ID was later used to index the token ID lookup unchecked (for example when building a reverse search), which is an out-of-bounds access that can crash the autodetect process with a SIGSEGV inside libc rather than failing the restore. Validate, at the end of CTokenListDataCategorizerBase::acceptRestoreTraverser, that every token ID referenced by a restored category exists in the restored token ID lookup, and fail the restore gracefully if not. This is consistent with the graceful invalid-state handling added in #2895/#2898. Relates to #2875 (cherry picked from commit 0f41e80) Co-authored-by: Ed Savage <ed.savage@elastic.co> Co-authored-by: Cursor <cursoragent@cursor.com>

…of-range token ID (#3143) (#3150) * [ML] Fail gracefully when restoring a categorizer with an out-of-range token ID (#3143) An inconsistent or truncated categorizer state document can leave a restored category referencing a token ID at or beyond the end of the restored token ID lookup. That ID was later used to index the token ID lookup unchecked (for example when building a reverse search), which is an out-of-bounds access that can crash the autodetect process with a SIGSEGV inside libc rather than failing the restore. Validate, at the end of CTokenListDataCategorizerBase::acceptRestoreTraverser, that every token ID referenced by a restored category exists in the restored token ID lookup, and fail the restore gracefully if not. This is consistent with the graceful invalid-state handling added in #2895/#2898. Relates to #2875 Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 0f41e80) * [ML] Add missing CJsonStateRestoreTraverser include on 8.19 backport The cherry-picked unit tests use JSON restore; 8.19's test file only included RapidXml headers, so all platform builds failed to compile. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Ed Savage <ed.savage@elastic.co> Co-authored-by: Cursor <cursoragent@cursor.com>

[ML] Better handling of invalid JSON state documents

e8498cb

Various changes to the handling of errors when parsing JSON state documents to improve consistency and provide better visibility

edsavage added >enhancement :ml v9.4.0 labels Jan 26, 2026

edsavage requested a review from valeriy42 January 26, 2026 03:33

edsavage added 2 commits January 26, 2026 16:44

Formatting

bff89ce

Update changelog

75c6624

valeriy42 reviewed Jan 26, 2026

View reviewed changes

edsavage added 3 commits January 27, 2026 14:26

Address code review comments

5a5964b

Formatting

ee61c6f

add missing file

587ec70

edsavage requested a review from valeriy42 January 28, 2026 01:18

edsavage mentioned this pull request Jan 28, 2026

autodetect fails to restore categorizer #2875

Open

valeriy42 approved these changes Jan 28, 2026

View reviewed changes

edsavage added 2 commits February 9, 2026 11:37

Downgrade log messages from ERROR to WARN

dcb09f7

Merge branch 'main' into improve_parsing_json_state

3bb13e3

edsavage merged commit 1ad9d71 into elastic:main Feb 9, 2026
3 checks passed

edsavage mentioned this pull request Feb 18, 2026

[ML] Fix cross-platform isEof() in CJsonStateRestoreTraverser #2898

Merged

4 tasks

valeriy42 mentioned this pull request Jul 21, 2026

[ML] Downgrade redundant CStateDecompressor empty-state log to INFO #3082

Merged

edsavage mentioned this pull request Aug 6, 2026

[ML] Fail gracefully when restoring a categorizer with an out-of-range token ID #3143

Merged

Navigation Menu

Uh oh!

[ML] Better handling of invalid JSON state documents - #2895

[ML] Better handling of invalid JSON state documents#2895
edsavage merged 8 commits into
elastic:mainfrom
edsavage:improve_parsing_json_state

edsavage commented Jan 26, 2026 •

edited

Loading

Uh oh!

prodsecmachine commented Jan 26, 2026 •

edited

Loading

Uh oh!

valeriy42 left a comment

Uh oh!

Uh oh!

valeriy42 Jan 26, 2026

Uh oh!

Uh oh!

Uh oh!

valeriy42 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

edsavage commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prodsecmachine commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

valeriy42 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

valeriy42 Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

valeriy42 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

edsavage commented Jan 26, 2026 •

edited

Loading

prodsecmachine commented Jan 26, 2026 •

edited

Loading