Harden error handling: eliminate silent failures in parser and schema validation#353
Open
holodorum wants to merge 8 commits intokson-org:mainfrom
Open
Harden error handling: eliminate silent failures in parser and schema validation#353holodorum wants to merge 8 commits intokson-org:mainfrom
holodorum wants to merge 8 commits intokson-org:mainfrom
Conversation
The idempotency assertion in `assertParsesTo` was comparing `ksonParseResult.kson` to itself, meaning it could never fail. The intent was to re-parse the compiled Kson output and verify it produces the same result — but the re-parse step was missing. Every test using `assertParsesTo` (hundreds across all KsonCoreTest* files) had its idempotency guarantee silently disabled.
A FatalParseException — meaning the parser violated its own invariants — was caught, printed to stdout, and silently swallowed. The caller got back a KsonRootError with no indication anything catastrophic happened. Now the error is properly reported through the messageSink via a new FATAL_PARSE_ERROR MessageType. This makes these errors visible to callers and test assertions. Two error-path tests (testIllegalCharacterError, testEmptyCommas) were updated to expect the now-visible FATAL_PARSE_ERROR messages that were previously swallowed by println.
Copy-paste error: the last parameter to mapToOriginal was subStartColumn instead of subEndColumn, meaning the end column of sub-locations within numbers was always mapped from the start column. KsonString at line 137 had this correct. Added a test that verifies the end position differs from start when different columns are requested — this test fails before the fix and passes after.
RefValidator created a local MessageSink for parsing the $ref target schema, then discarded it. If the target was malformed, validation silently accepted everything (fail-open). Now parse errors are captured alongside the schema result and forwarded to the caller's messageSink when parsing fails. This makes broken $ref targets visible rather than silently permissive. Three generated JSON Schema Test Suite tests that were passing only because of this fail-open behavior are now correctly excluded: they exercise complex relative URI / URN ref resolution that KSON does not yet support, and their "valid" results were false positives.
…xceptions PatternValidator and PropertiesValidator both called Regex() on user-provided schema patterns without catching the exception for invalid regex syntax. This caused uncaught PatternSyntaxExceptions during schema parsing and validation. - Add SCHEMA_INVALID_REGEX MessageType to report invalid patterns - Wrap PatternValidator creation in SchemaParser with try-catch - Validate patternProperties regex keys during schema parsing, filtering out invalid ones and reporting errors - Pre-compile pattern regexes in PropertiesValidator during construction instead of re-compiling on every validation call
NotValidator, IfValidator, PropertyNamesValidator, and DependencyValidatorSchema all accepted nullable schemas and silently skipped validation when the schema was null. This meant a malformed sub-schema silently became a permissive schema. The fix is two-fold: - In SchemaParser, skip creating validators when parseSchemaElement returns null (the parse error is already reported to messageSink) - Make the validator constructors require non-null schemas, enforced by the type system This makes the null-handling explicit at the construction site and eliminates four separate silent-accept code paths.
aochagavia
reviewed
Mar 25, 2026
src/commonMain/kotlin/org/kson/schema/validators/PropertiesValidator.kt
Outdated
Show resolved
Hide resolved
FatalParseException is meant to signal internal parser invariant
violations (i.e. bugs in KSON), but error-recovery paths were
producing marker trees that violated downstream structural invariants,
turning recoverable user errors into fatal internal errors.
1. Empty commas (`[1,,3]`, `{a: 1,, b: 2}`): processComma's error
marker was created inside the element/property mark, giving
LIST_ELEMENT and OBJECT_PROPERTY unexpected extra children. Fix:
close the element mark before reporting extra commas, via a new
consumeExtraCommas method that runs outside the element scope.
2. Illegal characters (`key: \value`, `[\value]`): delimitedValue's
error-recovery marker for illegal chars appeared as an additional
child in OBJECT_PROPERTY and LIST_ELEMENT, tripping their structural
guards. Fix: count only non-error children for the structural guard
while preserving the defensive check for genuinely unexpected
parsing changes.
Compile Regex objects at parse time in SchemaParser and pass them through to PatternValidator and PropertiesValidator, rather than accepting raw strings and re-compiling later. This uses the type system to enforce that only valid regex patterns reach the validators, eliminating the comment in PropertiesValidator that explained why Regex compilation was "guaranteed to succeed." PatternValidator now takes a Regex instead of a String. PropertiesValidator now takes List<CompiledPatternSchema> instead of Map<KsonString, JsonSchema?>, removing the compiledPatterns derivation that re-parsed already-validated strings. No new tests: this is a compile-time safety improvement exercised by the existing JSON Schema Test Suite and SchemaParserTest coverage for both pattern and patternProperties.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Route FatalParseException through messageSink — parser invariant violations were caught, printed to stdout, and silently swallowed. Now reported as FATAL_PARSE_ERROR messages visible to callers and tests.
Fix KsonNumber.subCoordinatesLocation — passed
subStartColumnwheresubEndColumnwas needed, making end-column mapping always wrong for number sub-locations.Surface schema parse errors from RefValidator — a local MessageSink captured $ref target parse errors then discarded them, causing malformed $ref targets to silently accept everything (fail-open). Three JSON Schema Test Suite tests that were false positives due to this are now correctly excluded.
Catch invalid regex patterns in schema —
Regex()on user-provided patterns threw uncaught exceptions (PatternSyntaxException on JVM, SyntaxError on JS). Now caught viarunCatchingand reported as SCHEMA_INVALID_REGEX errors. Pattern regexes in PropertiesValidator are also pre-compiled once during construction.Eliminate fail-open pattern in schema validators with null sub-schemas — NotValidator, IfValidator, PropertyNamesValidator, and DependencyValidatorSchema accepted nullable schemas and silently skipped validation when null. Now the type system enforces non-null schemas; null-handling is pushed to the construction site in SchemaParser.