On Tue, Oct 16, 2018 at 5:59 PM wjm wjm <zzz...@gmail.com> wrote:
>
> 在 2018年10月17日星期三 UTC+8上午12:54:52，Tatu Saloranta写道：
>>
>> On Tue, Oct 16, 2018 at 9:50 AM wjm wjm <zzz...@gmail.com> wrote:
>>>
>>>
>>> 1/2. "too big" or "too small" number, maybe should throw exception, just
>>> like Integer.parseInt(......);
>>
>>
>> yes, in cases where this occurs that makes sense.
>>
>>>
>>> 3.valid range of BigInteger/BigDecimal should be determined by different
>>> scenes
>>> but i'm not sure what is the "best" default configuration
>>> or just default to unlimit, it's develop team's duty to set a valid
>>> configuration for it, :)
>>
>>
>> The problem here is that if there is anywhere someone with legitimate use
>> and that breaks in a patch release,
>> dev team usually gets to hear about it. So it is important to balance quick
>> fix with some caution.
>
> my "develop team" means customer team, not jackson team, so default to
> unlimit will not break customer's logic
> but if they want to fix DoS attack problem of BigInteger/BigDecimal (not
> enum/int/long and so on), they must set a sensible range for their business
> logic.
Ideally the default would -- I think -- not be unlimited because doing
that will leave most users vulnerable.
So I agree that yes, if default was unlimited, nothing would break.
But it would be good to find defaults that
protect most users and are unlikely to break many.
>> But I think that one relative small fix to `_parseSlowInt()` (which I
>> checked in 2.9 branch)
>> might help quite a bit, if anyone wants to test. I will try to add tests
>> too, but this is tricky one
>> to test... not sure how to automate since it is very difficult to
>> automatically verify timing
>> problems due to usual performance measurement challenges (JVM startup
>> slowness etc).
>>
>> -+ Tatu +-
>
> As long as the numbers are large enough, then old parse speed is very slow
> eg:
> Strings.repeat("1", 100_0000)
> old logic will cost 17+ seconds in my machine
> but if fail quickly, only cost 10+ milliseconds
> so think about difference between machines, we can treat less than 1 second
> to be a fixed status?
My main concern is that manually testing this is easy enough but that
I am not sure logic is easy to automate in a way that
does not cause false test failures on some systems (when running tests
on cold JVM for example).
But maybe others can point to cool test extensions or libraries that
can handle this kind of testing -- reliably detecting
performance problems with some sort of statistical running of test for
a while, which is done for performance benchmarking.
-+ Tatu +-
>>
>>
>>>
>>>
>>> 在 2018年10月16日星期二 UTC+8下午11:08:11，Tatu Saloranta写道：
>>>>
>>>> On Tue, Oct 16, 2018 at 8:02 AM wjm wjm <zzz...@gmail.com> wrote:
>>>> >
>>>> > seems that jackson have some problems when parse super number
>>>> > Gson failed at once, but jackson block long times and then failed
>>>> > because jackson parse the super number to BigInteger first and then
>>>> > convert to target number type.
>>>> >
>>>> > i write a reproduce test case
>>>> > please help me to resolve the problem, thanks
>>>> >
>>>> > package test.jackson;
>>>> >
>>>> > import java.util.concurrent.Callable;
>>>> >
>>>> > import com.fasterxml.jackson.databind.ObjectMapper;
>>>> > import com.google.common.base.Strings;
>>>> > import com.google.gson.Gson;
>>>> > import com.google.gson.GsonBuilder;
>>>> >
>>>> > public class DoS {
>>>> > static String input = Strings.repeat("1", 100_0000);
>>>> >
>>>> > static Gson gson = new GsonBuilder().create();
>>>> >
>>>> > static ObjectMapper mapper = new ObjectMapper();
>>>> >
>>>> > public static enum Color {
>>>> > WHITE,
>>>> > BLACK
>>>> > }
>>>> >
>>>> > static void run(String name, Callable callable) {
>>>> > System.out.println(name);
>>>> >
>>>> > long start = System.currentTimeMillis();
>>>> > try {
>>>> > Object result = callable.call();
>>>> > System.out.println(" result:" + result);
>>>> > } catch (Throwable e) {
>>>> > //System.out.println(" failed: " + e.getMessage());
>>>> > System.out.println(" failed");
>>>> > }
>>>> > System.out.println(" time:" + (System.currentTimeMillis() - start)
>>>> > + " ms");
>>>> > }
>>>> >
>>>> > public static void main(String[] args) {
>>>> > run("Gson enum:", () -> gson.fromJson(input, Color.class));
>>>> > run("Jackson enum:", () -> mapper.readValue(input, Color.class));
>>>> > System.out.println();
>>>> >
>>>> > // byte/Byte/short/Short/int/Integer/long/Long
>>>> > // all these types, jackson is very slow
>>>> > run("Gson int:", () -> gson.fromJson(input, int.class));
>>>> > run("Jackson int:", () -> mapper.readValue(input, int.class));
>>>> > System.out.println();
>>>> >
>>>> > // float/Float/double/Double
>>>> > // all these types, jackson is very slow
>>>> > run("Gson double:", () -> gson.fromJson(input, double.class));
>>>> > run("Jackson double:", () -> mapper.readValue(input, double.class));
>>>> > System.out.println();
>>>> > }
>>>> > }
>>>> >
>>>> >
>>>> > the output is :
>>>> > Gson enum:
>>>> > result:null
>>>> > time:16 ms
>>>> > Jackson enum:
>>>> > failed
>>>> > time:17469 ms
>>>> >
>>>> > Gson int:
>>>> > failed
>>>> > time:32 ms
>>>> > Jackson int:
>>>> > failed
>>>> > time:17392 ms
>>>> >
>>>> > Gson double:
>>>> > result:Infinity
>>>> > time:8 ms
>>>> > Jackson double:
>>>> > result:Infinity
>>>> > time:18187 ms
>>>>
>>>> Ok, this is obviously a big problem. I filed:
>>>>
>>>> https://github.com/FasterXML/jackson-databind/issues/2157
>>>>
>>>> to cover it, and this is related to earlier
>>>>
>>>> https://github.com/FasterXML/jackson-databind/issues/2141
>>>>
>>>> Unfortunately problem is difficult to tackle. Jackson does figure out
>>>> magnitude part during decoding, and use "smallest possible" type
>>>> for integral numbers, from group of `int`, `long` and `BigInteger`
>>>> (for floating-point logic is bit different, but similar idea applies).
>>>> Problem occurs when target type needed by databinding is something
>>>> else: like very long integer number that is to be used for filling
>>>> `int` field. This operation is slow with JDK, and should be avoided
>>>> partly because overflow check would fail anyway -- but this is only
>>>> applied after very slow conversion, allowing DoS attacks.
>>>>
>>>> I have some questions on how this could and should be handled,
>>>> regarding things like:
>>>>
>>>> 1. On "too big" (big magnitude) number, should we typically truncate
>>>> to MAX_VALUE (for `double`), or throw exception (latter for integral I
>>>> think)
>>>> 2. On "too small" (small magnitude) number, should we truncate to zero
>>>> 3. If applying strict limits to longest BigInteger/BigDecimal, are
>>>> there safe length limits? Same use cases (like cryptography) use
>>>> relatively long numbers legitimately, for example
>>>>
>>>> But. Maybe I can figure out solution to bounds-check-fail case at least,
>>>> first.
>>>>
>>>> -+ Tatu +-
>>>>
>>>> ps. This is not limited to json either; almost certainly applicable to
>>>> XML, YAML, possibly CSV, Properties. So I think help would be useful
>>>> in figuring out extent of the problem.
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups
>>> "jackson-user" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an
>>> email to jackson-user...@googlegroups.com.
>>> To post to this group, send email to jackso...@googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.
>
> --
> You received this message because you are subscribed to the Google Groups
> "jackson-user" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to jackson-user+unsubscr...@googlegroups.com.
> To post to this group, send email to jackson-user@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups
"jackson-user" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to jackson-user+unsubscr...@googlegroups.com.
To post to this group, send email to jackson-user@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
