Jettison
  1. Jettison
  2. JETTISON-61

String values converted to numeric literals are losing precision in Javascript due to it's 64 bit floating point representation

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.1
    • Fix Version/s: 1.2
    • Labels:
      None
    • Environment:
      When consuming JSON produced messages in a web browser through (firefox, IE)
    • Number of attachments :
      1

      Description

      I created an issue in the XStream project describing an issue with 64 bit Java long values losing precision when converting to javascript 64 bit float. I was able to work around it by changing the XStream API to accept the JETTISON SimpleConverter.
      http://jira.codehaus.org/browse/XSTR-541

      "The normal conversion of string values to JSON attempts to convert values to numeric values, but this is problematic for numeric values like 64 bit Long's when converted to Javascript, they are converted to 64 bit floating point values, so values larger than I believe 2^51 or something like that can lose precision.

      We can use the SimpleConverter and I put in an issue into XStream to ensure that users can configure the JettisonMappedXMLDriver, but even in the default behavior of Jettison we probably shouldn't allow numeric values that are larger than can be converted into 64 bit double precision floating point (although I guess that's a javascript specific limitation, and maybe Javascript can check that when it's evaluating and if the numeric value doesn't match what the string field was it would fail, but I'm not sure that's really easy to change).

      Here's the XStream patch with the issue, the solution right now is to simply use the SimpleConverter.
      http://jira.codehaus.org/browse/XSTR-540 "

      There was discussion on the mailing list to push this over to the Jettison developers:
      http://thread.gmane.org/gmane.comp.java.xstream.user/5495/focus=5500
      "Despite the fact that Jettison's SimpleConverter is not yet released, it's
      nothing we can really use by default. A lot of people would be surprised if
      any long/Long is suddenly provided as String. However, you might open an
      issue for Jettison, that they automatically keep the value as String if the
      long value exceed the JavaScript value range, because it is Jettison that
      internally transforms the input of XStream (resp. the StAX writer's output)
      into an integer value."

      Would it be possible for JETTISON to detect a value is larger than what can be represented in Javascript's 64-bit double value and convert it to a string? I guess the issue with that is Javascript is not the only consumer of JSON messages, and it would be strange to see some values as strings and some as numeric literals, so it might be best to leave up to configuration.

        Activity

        Hide
        Geoff Granum added a comment - - edited

        We have encountered this issue using RESTEasy, and I'd argue that it is a blocker, not minor. An example:

        @XmlRootElement
        @XmlAccessorType(XmlAccessType.PROPERTY) // leaving off getters/setters for clarity.
        public class DemoBean
        {
          @XmlElement(type = String.class ) // <-- just indicating that type doesn't help. ( Annotation would be on getter for AccessType.Property)
          private String value = "999999999999999999";
          private String name = "aName";
        }
        

        Marshalled value on first request:

        {"demoBean":{"name":"aName", "value":999999999999999999 } }
        

        Eval the bean:

        demoBean : {
         name : 'aName',
         value : 1000000000000000000
        }
        

        If the consumer then changes 'name' and saves the bean back to the server, value is modified as well.

        I can imagine a couple of potential fixes for this.

        Modify org.codehaus.jettison.mapped.DefaultConverter:

        • Argument in favor: It would better to return a string - which can be converted to a float - than to return a value which loses precision.
        • Argument against: changes existing behavior.
          Object primitive = null;
           // Attempt to convert to Integer
           try 
          {
             primitive = Long.valueOf(text);
             if( primitive > Integer.MAX_VALUE || primitive < Integer.MIN_VALUE )
             {
               return text;
             }
           } 
           catch (Exception e) {}
          

        Another possible fix:
        Check the System properties for a user-specified classpath to use for the TypeConverter

        • This in Configuration.java and MappedNamespaceConvention.java.
        • Since Jettison is embedded by RestEasy users will generally not be able to modify the TypeConverter to be used, at least without creating a vast amount of additional boilerplate.

        Thanks,
        Geoff Granum

        Show
        Geoff Granum added a comment - - edited We have encountered this issue using RESTEasy, and I'd argue that it is a blocker, not minor. An example: @XmlRootElement @XmlAccessorType(XmlAccessType.PROPERTY) // leaving off getters/setters for clarity. public class DemoBean { @XmlElement(type = String .class ) // <-- just indicating that type doesn't help. ( Annotation would be on getter for AccessType.Property) private String value = "999999999999999999" ; private String name = "aName" ; } Marshalled value on first request: {"demoBean":{"name":"aName", "value":999999999999999999 } } Eval the bean: demoBean : { name : 'aName', value : 1000000000000000000 } If the consumer then changes 'name' and saves the bean back to the server, value is modified as well. I can imagine a couple of potential fixes for this. Modify org.codehaus.jettison.mapped.DefaultConverter: Argument in favor: It would better to return a string - which can be converted to a float - than to return a value which loses precision. Argument against: changes existing behavior. Object primitive = null ; // Attempt to convert to Integer try { primitive = Long .valueOf(text); if ( primitive > Integer .MAX_VALUE || primitive < Integer .MIN_VALUE ) { return text; } } catch (Exception e) {} Another possible fix: Check the System properties for a user-specified classpath to use for the TypeConverter This in Configuration.java and MappedNamespaceConvention.java. Since Jettison is embedded by RestEasy users will generally not be able to modify the TypeConverter to be used, at least without creating a vast amount of additional boilerplate. Thanks, Geoff Granum
        Hide
        Geoff Granum added a comment -

        This patch also modifies the POM in order to enable per-test forking. Not sure how important that is to anyone.

        I went ahead and fixed the infinity and NaN bug (64) while I was in the code - I didn't notice the patch on the Jira post until just now. I do slightly favor the solution in this file, as it delegates the determination to the JVM rather than hard-coding the strings 'Infinity' and 'NaN'.

        This patch includes the option to set two System properties:
        #1: 'jettison.mapped.typeconverter.enforce_32bit_integer': Disable Long values.
        – Iff set to true, any integer value which exceeds 32bits will be returned as a String

        #2 'jettison.mapped.typeconverter.class': Specify a custom 'default' converter class (not to be confused with the current 'DefaultConverter').
        – Iff set to a valid class name before Configuration.java is accessed in the current Classloader, then the specified class will be instantiated via Class.forName(theValue) for all instances of Configuration.java in that Classloader. If not set, DefaultConverter will be used, as before.

        Test cases for all of the above.

        Show
        Geoff Granum added a comment - This patch also modifies the POM in order to enable per-test forking. Not sure how important that is to anyone. I went ahead and fixed the infinity and NaN bug (64) while I was in the code - I didn't notice the patch on the Jira post until just now. I do slightly favor the solution in this file, as it delegates the determination to the JVM rather than hard-coding the strings 'Infinity' and 'NaN'. This patch includes the option to set two System properties: #1: 'jettison.mapped.typeconverter.enforce_32bit_integer': Disable Long values. – Iff set to true, any integer value which exceeds 32bits will be returned as a String #2 'jettison.mapped.typeconverter.class': Specify a custom 'default' converter class (not to be confused with the current 'DefaultConverter'). – Iff set to a valid class name before Configuration.java is accessed in the current Classloader, then the specified class will be instantiated via Class.forName(theValue) for all instances of Configuration.java in that Classloader. If not set, DefaultConverter will be used, as before. Test cases for all of the above.
        Hide
        Dejan Bosanac added a comment -

        Hi Geoff, thanks for the patch. I'll see to commit it soon.

        Show
        Dejan Bosanac added a comment - Hi Geoff, thanks for the patch. I'll see to commit it soon.
        Hide
        Dejan Bosanac added a comment -

        Patch applied with svn revisions 83 and 84. Thanks.

        Show
        Dejan Bosanac added a comment - Patch applied with svn revisions 83 and 84. Thanks.

          People

          • Assignee:
            Dejan Bosanac
            Reporter:
            Doug Daniels
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: