Repository navigation
Refuse Float and Double data values that overflow to infinity [patch] - #360
Open
matt-edmondson wants to merge 1 commit into
Open
matt-edmondson wants to merge 1 commit into
matt-edmondson wants to merge 1 commit into
Conversation
The validator checked a Float or Double value only for being a JSON number, so 1e39 in a Float (or 1e309 in a Double) validated clean and then read back from the generated record as Infinity. Float, Double, and every vector, colour, quantity and semantic component stored in one are now held to the storage type's finite range, and the message states that range. SchemaDataValues.GetFloat, GetDouble and the vector/colour readers now treat a non-finite result as malformed and throw, instead of returning it. Fixes #355 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW3VYgr52Mw1KS9otcE
Contributor
Author
|
CI is red on #353 has the fix and is mergeable. I haven't copied that refactor in here, since it would duplicate #353 across three open PRs and they would then conflict with it. Once #353 merges I'll bring Generated by Claude Code |
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
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.
Fixes #355
What changed
SchemaDataValidator:FloatandDoublevalues now go throughExpectNumber, which holds a number to its storage type's finite range.Floatmust satisfy|x| ≤ float.MaxValueandDoublemust parse to a finite double. Vector, colour, quantity and semantic-type components with Float/Double storage use the same path, so they get the same check. The message states the range, for exampleExpected a number within the range of Float (±3.4028235E+38) for type 'Float' but found 1e39.SchemaDataValues(the optional part of the issue):GetFloat,GetDoubleand the vector/colour component readers treat a non-finite result as malformed and throwInvalidOperationException, the same way they already handle a value that doesn't fit. They no longer hand back ±Infinity.The migrator's
Conformsgoes through the validator, so it no longer accepts such values either.Tests
SchemaDataValidatorTypeTests:±1e39in a Float is an error, whilefloat.MaxValue,1e38and0are accepted.1e309in a Double is an error and1e39is not. Also covered: a Float vector component (error atdata.Value[1]), a colour component, a Float-stored quantity, and a semantic type over Float.GeneratedDataEditorTests.TestANumberPastItsTypesRangeThrowsRatherThanReadingAsInfinity:GetFloat,GetDoubleandGetVector3throw. A Double read of1e39still succeeds.With the fix reverted, the 7 new tests fail. With it, the full
Schema.Testsuite passes on net10.0 (728/728).This touches
SchemaDataValidator.cslike #359 (for #349), but in different methods. It's branched frommainon its own, not stacked on that PR.Main currently fails CA1506 on
Schema/Models/Schema.cs(#353 fixes that). I ran the tests locally with that one rule suppressed, and the suppression isn't part of this PR. CI here will stay red on CA1506 until #353 lands.Generated by Claude Code