-
-
Notifications
You must be signed in to change notification settings - Fork 795
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OutOfMemoryError
when writing BigDecimal
#315
Comments
Note: for earlier discussion, have a look at FasterXML/jackson-databind#1316 Basically there are differing views of how big is "too big", and what to do about that. The first step that I will do is to simply throw an exception if attempts is made with |
I think a shorter limit makes sense, maybe 100 digits max, but I think the only way to make everyone happy is to have minimum and maximum scale settings or something like that. Let's say we set the limit to 9999 as you suggest. That means each number can occupy a maximum of (slightly less than) 10KB of space. If I had an array with a lot of these I could still easily see it causing a similar error. I suspect most users of the Actually, I can't think of any other valid reason to use that setting other than making numbers more human-readable. (If your app requires JSON numbers to be formatted a particular way to actually function, then it's not using JSON properly.) |
I think any limit below millions should suffice to solve the reported problem of running out of memory accidentally. I agree with your guess on usage; I don't use this feature myself nor think it generally makes sense. So: let's start with 9999 as the limit and see where that takes us. |
Then what was the original purpose for adding that feature? I think 10K is a bit much to represent a number but you're right it probably won't cause issues in all but the most extreme cases. You know your users much more than I do so I'm happy to trust your judgment there. I can't think of a good argument for any specific number. |
@gmethvin It was added by someone requesting it (and/or providing a PR); I don't recall exact reasons but usually it's either compatibility or visual aesthetics. At this point 95%+ of features are due to someone else needing the feature. Just because I don't need or use something does not mean others don't. If Jackson was limited to my needs, version 1.1 would have done close to everything. :) |
@cowtowncoder I think you missed my point. I cannot find much discussion on this and the only issue I found was FasterXML/jackson-databind#251. I was hoping to find some to support the idea that someone would want a very long number to be written out, but pretty much everything I can find suggests that option is being used to fix annoyances with shorter numbers. I'm not saying you should remove the option necessarily, but maybe we need something that more directly addresses their problem without causing these potentially unwanted side effects. |
At this point I think we are all set, from my point, and see no reason to make further changes. |
(note: moved from FasterXML/jackson-databind#1316 reported by @gmethvin)
When I've enabled the
WRITE_BIGDECIMAL_AS_PLAIN
setting on Jackson 2.7.5, Jackson will attempt to write out the whole number, no matter how large the exponent.For example, the following code:
triggers the exception:
I know technically Jackson is doing what you're telling it to do (so if you don't feel this is an issue feel free to close it). But it would be nice if
WRITE_BIGDECIMAL_AS_PLAIN
set a reasonable length on the number, so as not to leave users open to denial of service vulnerabilities.(Actually, I think this might technically be an issue in jackson-core; let me know if I should resubmit.)
The text was updated successfully, but these errors were encountered: