Add Number size limits (max 50 characters), enforcement #432
Conversation
|
Hmmmh. Looks like I found a Record-deserialization regression b/w Jackson 2.14 and 2.15. Yikes. Need to troubleshoot.... |
|
@tatu-at-datastax Guess not needed anymore with #439 ? |
|
@ivansenic We still need 2.15.x (likely 2.15.2) to get Number length limits. But I need to get 2.15.2 released first, to resolve issue uncovered by my "interesting" usage of setter method on |
|
Now that Jackson 2.15.2 is out, with relevant fix, can unblock this. Will first get ITs to run to make sure everything still works, then add tighter number length limits and matching tests. |
tatu-at-datastax
changed the title
tatu-at-datastax
changed the title
| @JsonTypeInfo( | ||
| use = JsonTypeInfo.Id.NAME, | ||
| include = JsonTypeInfo.As.WRAPPER_OBJECT, | ||
| property = "commandName") |
There was a problem hiding this comment.
"property" only used with As.PROPERTY, not with WRAPPER_OBJECT.
| @@ -100,16 +100,16 @@ public void insertDocument() { | |||
| public void insertDocumentWithDateValue() { | |||
| String json = | |||
| """ | |||
| { | |||
There was a problem hiding this comment.
Not done on purpose but ./mvnw fmt:format sometimes/somehow seems to decide it wants to reformat. :-(
I am not sure it would be possible to change these during runtime (to have effect dynamically), as they are typically injected into other handlers. I guess I haven't tried that, and in kubernetes environment I was assuming change to pod would need to restart it anyway (to pass system properties or env variables). But I'll read bit more about this annotation to understand it better. |
tatu-at-datastax
changed the title
|
Since injection of |
What this PR does:
Will impose limit to maximum length of JSON Number values we accept in JSON Documents (based on textual length): initial value configured to 50 characters.
In theory we could limit to lower (some documentation suggest maximum number of significant digits would be 21) but since the goal here is to reduce likelihood of DoS style resource consumption, we only need to prevent thousands of digits -- performance testing done by Jackson project suggested change not below 1000 digits.
Which issue(s) this PR fixes:
Fixes #172
Checklist