search

FasterXML / jackson-core

Core part of Jackson that defines Streaming API as well as basic shared abstractions
Apache License 2.0
2.27k stars 793 forks source link

Fix `NumberInput.looksLikeValidNumber()` implementation #1241

Closed pjfanning closed 7 months ago

pjfanning commented 7 months ago

See https://github.com/FasterXML/jackson-databind/issues/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 commented 7 months ago

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 commented 7 months ago

(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