fairyhawk / protostuff

Automatically exported from code.google.com/p/protostuff
Apache License 2.0
0 stars 0 forks source link

Quoted Integer json values do not parse correctly #109

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Create message with optional int32 testInt =1;
2. Attempt to deserialize {"testInt":"1670"} using JsonIOUtil

What is the expected output? What do you see instead?
getTestInt should be 1670, it was a different numeric value (depending on other 
properties perhaps)

What version of the product are you using? On what operating system?
1.0.4

Please provide any additional information below.
Directly using Jackson ObjectMapper works correctly. 

Original issue reported on code.google.com by celan...@gmail.com on 2 Apr 2012 at 10:38

GoogleCodeExporter commented 8 years ago
For security and pluggability, it is best to ask this feature from jackson.

E.g JsonParser.Feature.ALLOW_QUOTED_NUMBERS.
So that parser.getIntValue (and all the others) would parse a string internally 
if the feature is explicitly enabled by the user.

Original comment by david.yu...@gmail.com on 3 Apr 2012 at 3:20

GoogleCodeExporter commented 8 years ago
Agree. A configuration flag to support this would support my needs. Apparently 
jackson will quote numbers occasionally see 
http://jersey.576304.n2.nabble.com/In-JSON-output-how-do-you-write-a-number-as-a
-number-and-not-as-a-string-td7292424.html

Original comment by celan...@gmail.com on 3 Apr 2012 at 8:05

GoogleCodeExporter commented 8 years ago
PS. Looked a bit more into this. Jackson does supports quoted numbers in 
ObjectMapper. It doesn't seem to directly support it in JsonParser.  I believe 
ObjectMapper is using StdDeserializer.NumberDeserializer.deserialize to support 
this it as it does support quoted strings.

This is a minor issue for us as we could store it as a String, however it not 
throwing an Exception and turning string 1670 to an integer 3 in our use case 
is puzzling.

Original comment by celan...@gmail.com on 3 Apr 2012 at 8:08

GoogleCodeExporter commented 8 years ago
Which version of protostuff are you using?  Can you attach a simple 
example/project which demostrates the behavior? (turning a string 1670 to an 
integer 3)

I tested that with 1.0.4+ and you get a JsonParseException from jackson.
So it seems like you might a different version of jackson, which in turn 
contains the bug of incorrectly parsing 1670 to 3.

I've attached a JsonParser that transparently parses the quoted numbers.
You can simply provide this parse to JsonIOUtil.mergeFrom and you're good to go.

Original comment by david.yu...@gmail.com on 4 Apr 2012 at 5:22

Attachments:

GoogleCodeExporter commented 8 years ago
I can provide the full sample tomorrow after I scrub the data

Original comment by celan...@gmail.com on 4 Apr 2012 at 5:35

GoogleCodeExporter commented 8 years ago
It seems there might be a more important bug in the code. Its getting the most 
recent numeric field not the actual string value for some reason. No exception 
is thrown but both wantsChildren and heightInMM are 3 

message Profile {
optional int32 wantsChildren =1;
optional int32 heightInMM =2;
}

        String json = "{\"wantsChildren\":3,\"heightInMM\":\"1670\"}";
        StringReader reader = new StringReader(json);
        ProfileProtoBuffs.ProfileProto.Builder builder = ProfileProtoBuffs.ProfileProto.newBuilder();
        JsonIOUtil.mergeFrom(reader, builder, SchemaProfileProtoBuffs.ProfileProto.MERGE, false);
        ProfileProtoBuffs.ProfileProto profile = builder.build();
        assertEquals(1670,profile.getHeightInMM());

Original comment by celan...@gmail.com on 6 Apr 2012 at 5:46

GoogleCodeExporter commented 8 years ago
This throws a JsonInputException
    @Test
    public void testMinimal() throws IOException {
        String json = "{\"wantsChildren\":3,\"heightInMM\":\"1670\"}";
        StringReader reader = new StringReader(json);
        ProfileProtoBuffs.ProfileProto.Builder builder = ProfileProtoBuffs.ProfileProto.newBuilder();
        QuotedNumberJsonParser parser = new QuotedNumberJsonParser(JsonIOUtil.DEFAULT_JSON_FACTORY.createJsonParser(reader));
        JsonIOUtil.mergeFrom(parser, builder, SchemaProfileProtoBuffs.ProfileProto.MERGE, false);
        ProfileProtoBuffs.ProfileProto profile = builder.build();
        assertFalse(profile.hasApprovedPhoto());
        assertEquals(3,profile.getWantsChildren());
        assertEquals(1670,profile.getHeightInMM());
        assertEquals("Friends/family",profile.getCantLiveWithouts(0));

    }

com.dyuproject.protostuff.JsonInputException: Expected token: } but was null on 
messagetest.ProfileProtoBuffs$ProfileProto
    at com.dyuproject.protostuff.JsonIOUtil.mergeFrom(JsonIOUtil.java:344)
test.ProfileProtoBuffsTest.testMinimal(ProfileProtoBuffsTest.java:51)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.junit.internal.runners.TestMethod.invoke(TestMethod.java:59)
    at org.junit.internal.runners.MethodRoadie.runTestMethod(MethodRoadie.java:98)
    at org.junit.internal.runners.MethodRoadie$2.run(MethodRoadie.java:79)
    at org.junit.internal.runners.MethodRoadie.runBeforesThenTestThenAfters(MethodRoadie.java:87)
    at org.junit.internal.runners.MethodRoadie.runTest(MethodRoadie.java:77)
    at org.junit.internal.runners.MethodRoadie.run(MethodRoadie.java:42)
    at org.junit.internal.runners.JUnit4ClassRunner.invokeTestMethod(JUnit4ClassRunner.java:88)
    at org.junit.internal.runners.JUnit4ClassRunner.runMethods(JUnit4ClassRunner.java:51)
    at org.junit.internal.runners.JUnit4ClassRunner$1.run(JUnit4ClassRunner.java:44)
    at org.junit.internal.runners.ClassRoadie.runUnprotected(ClassRoadie.java:27)
    at org.junit.internal.runners.ClassRoadie.runProtected(ClassRoadie.java:37)
    at org.junit.internal.runners.JUnit4ClassRunner.run(JUnit4ClassRunner.java:42)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)

Original comment by celan...@gmail.com on 6 Apr 2012 at 6:06

GoogleCodeExporter commented 8 years ago
Combined with the patch from 
http://code.google.com/p/protostuff/issues/detail?id=108 I'm getting a 
different exception
Let me put together a brand new test project and attach some test cases

com.dyuproject.protostuff.JsonInputException: Expected token: $field: but was 
null on message test.ProfileProtoBuffs$ProfileProto
    at com.dyuproject.protostuff.JsonInput.readFieldNumber(JsonInput.java:150)
    at com.dyuproject.protostuff.JsonInput.readFieldNumber(JsonInput.java:137)
    at test.SchemaProfileProtoBuffs$ProfileProto$BuilderSchema.mergeFrom(SchemaProfileProtoBuffs.java:1252)
    at test.SchemaProfileProtoBuffs$ProfileProto$BuilderSchema.mergeFrom(SchemaProfileProtoBuffs.java:1)
    at com.dyuproject.protostuff.JsonIOUtil.mergeFrom(JsonIOUtil.java:340)
    at test.ProfileProtoBuffsTest.testMinimal(ProfileProtoBuffsTest.java:51)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.junit.internal.runners.TestMethod.invoke(TestMethod.java:59)
    at org.junit.internal.runners.MethodRoadie.runTestMethod(MethodRoadie.java:98)
    at org.junit.internal.runners.MethodRoadie$2.run(MethodRoadie.java:79)
    at org.junit.internal.runners.MethodRoadie.runBeforesThenTestThenAfters(MethodRoadie.java:87)
    at org.junit.internal.runners.MethodRoadie.runTest(MethodRoadie.java:77)
    at org.junit.internal.runners.MethodRoadie.run(MethodRoadie.java:42)
    at org.junit.internal.runners.JUnit4ClassRunner.invokeTestMethod(JUnit4ClassRunner.java:88)
    at org.junit.internal.runners.JUnit4ClassRunner.runMethods(JUnit4ClassRunner.java:51)
    at org.junit.internal.runners.JUnit4ClassRunner$1.run(JUnit4ClassRunner.java:44)
    at org.junit.internal.runners.ClassRoadie.runUnprotected(ClassRoadie.java:27)
    at org.junit.internal.runners.ClassRoadie.runProtected(ClassRoadie.java:37)
    at org.junit.internal.runners.JUnit4ClassRunner.run(JUnit4ClassRunner.java:42)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)

Original comment by celan...@gmail.com on 6 Apr 2012 at 6:20

GoogleCodeExporter commented 8 years ago
Here you go

Original comment by celan...@gmail.com on 6 Apr 2012 at 6:42

Attachments:

GoogleCodeExporter commented 8 years ago
If I add this method to the end of QuotedNumberParser 2 more tests pass
    @Override
    public JsonToken getCurrentToken() {
        return parser.getCurrentToken();
    }

Original comment by celan...@gmail.com on 6 Apr 2012 at 6:55

GoogleCodeExporter commented 8 years ago
Got all tests to pass.

Note:
Update testProtoc() and testBean() that it actually uses QuotedNumberJsonParser.

Attached is the updated QuotedNumberJsonParser (I simply the extra override 
methods, which the parser base class needs for composition to work (I guess it 
prefers inheritance).

Original comment by david.yu...@gmail.com on 7 Apr 2012 at 4:27

Attachments:

GoogleCodeExporter commented 8 years ago
The other tests specifically did not use QuotedNumberJsonParser so that you 
could see that it gives invalid values and throws no exceptions.

Original comment by celan...@gmail.com on 7 Apr 2012 at 6:21