Skip to content

Conversation

@pjfanning
Copy link
Member

@pjfanning pjfanning commented Mar 16, 2024

See FasterXML/jackson-databind#4435

I'm not a regex expert and sort of hate them from a code readability and maintenance perspective.

This is my attempt at fixing the issue but the method is now so neutered, I really wonder if the method should be removed. It is also going to cause perf issues as the check is non-trivial.

@EAlf91
Copy link

EAlf91 commented Mar 16, 2024

Thanks for dealing with this topic :D
IMHO it can be removed completely. According to RFC-8259 the rules for strings should be applied here for parsing from string inputs in jackson-databind and not number in this case. (see JsonTokenId.ID_STRING in NumberDeserializers.java)

It seems that we're encountering this error because the prevalidation method looksLikeValidNumber is misplaced for the given input, which is JSON-Type string and not number.

@cowtowncoder cowtowncoder changed the title fix regression caused by looksLikeValidNumber Fix NumberInput.looksLikeValidNumber() implementation Mar 23, 2024
@cowtowncoder cowtowncoder added the 2.17 Issues planned (at earliest) for 2.17 label Mar 23, 2024
@cowtowncoder cowtowncoder added this to the 2.17.1 milestone Mar 23, 2024
@cowtowncoder
Copy link
Member

(I thought I had added a comment here).

No. this cannot and should not be removed: while the location of method may be misplaced (it is really only needed by jackson-databind, not core streaming itself), it serves necessary function as can be seen from NumberDeserializers.
It has nothing to do wrt general validation of JSON Strings but needed for heuristics on when to apply maximum number length checks to avoid possibly DoS for decoding "too long" floating-point numbers.

Thank you @pjfanning for PR, will merge to get in 2.17.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.17 Issues planned (at earliest) for 2.17

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants