From 88756c04908d5137bf570e2c399587e222169f92 Mon Sep 17 00:00:00 2001 From: dieter Date: Wed, 27 May 2015 15:51:30 +0200 Subject: [PATCH 1/2] Hidden typecast for Float objects in JSONobject.increment(String key) --- JSONObjectTest.java | 60 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/JSONObjectTest.java b/JSONObjectTest.java index de36c01..c02ebd1 100644 --- a/JSONObjectTest.java +++ b/JSONObjectTest.java @@ -493,7 +493,7 @@ public class JSONObjectTest { "\"negativeHexFloat\":-0x1.fffp1,"+ "\"hexFloat\":0x1.0P-1074,"+ "\"floatIdentifier\":0.1f,"+ - "\"doubleIdentifier\":0.1d,"+ + "\"doubleIdentifier\":0.1d"+ "}"; JSONObject jsonObject = new JSONObject(str); Object obj; @@ -694,6 +694,17 @@ public class JSONObjectTest { "\"keyLong\":9999999993,"+ "\"keyDouble\":3.1,"+ // TODO: not sure if this will work on other platforms + + // Should work the same way on any platform, this the effect of a float to double conversion and happens because + // java type-casts float to double. A 32 bit float is type-casted to 64 bit double by simply appending zero-bits to the + // mantissa (and extended the signed exponent by 3 bits.) + + // Like 1/3 cannot be represented as base10 number because it is periodically, 1/5 cannot be represented as base2 number + // since it is periodically in base2 (take a look at http://www.h-schmidt.net/FloatConverter/) + // The same happens to 3.1, that decimal number (base10 representation) is periodic in base2 representation, therefore + // appending zero-bits is inaccurate only repeating the periodically repeating bits (0110) would be a proper conversion. + // However one cannot detect from a 32 bit IEE754 representation which bits would "repeat infinitely", since the missing + // bits would not fit into the 32 bit float, i.e. the information needed is not there! ;) "\"keyFloat\":3.0999999046325684,"+ "}"; JSONObject jsonObject = new JSONObject(str); @@ -709,6 +720,53 @@ public class JSONObjectTest { jsonObject.increment("keyFloat"); JSONObject expectedJsonObject = new JSONObject(expectedStr); Util.compareActualVsExpectedJsonObjects(jsonObject, expectedJsonObject); + + /* + float f = 3.1f; + double df = (double) f; + double d = 3.1d; + System.out.println(Integer.toBinaryString(Float.floatToRawIntBits(f))); + System.out.println(Long.toBinaryString(Double.doubleToRawLongBits(df))); + System.out.println(Long.toBinaryString(Double.doubleToRawLongBits(d))); + + - Float: + seeeeeeeemmmmmmmmmmmmmmmmmmmmmmm + 1000000010001100110011001100110 + - Double + seeeeeeeeeeemmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm + 10000000 10001100110011001100110 + 100000000001000110011001100110011000000000000000000000000000000 + 100000000001000110011001100110011001100110011001100110011001101 + */ + assertTrue( "Correctly converting float to double via base10 (string) representation!", new Double( 3.1d ).equals( new Double( new Float( 3.1f ).toString() ) ) ); + + // Pinpointing the not so obvious "buggy" conversion from float to double in JSONObject + JSONObject jo = new JSONObject(); + jo.put( "bug", 3.1f ); // will call put( String key, double value ) with implicit and "buggy" type-cast from float to double + assertFalse( "The java-compiler did add some zero bits for you (probably unexpected, but well documented)", jo.get( "bug" ).equals( new Double( 3.1d ) ) ); + + JSONObject inc = new JSONObject(); + inc.put( "bug", new Float( 3.1f ) ); // This will put in instance of Float into JSONObject, i.e. call put( String key, Object value ) + assertTrue( "Everything is ok here!", inc.get( "bug" ) instanceof Float ); + inc.increment( "bug" ); // after adding 1, increment will call put( String key, double value ) with implicit and "buggy" type-cast from float to double! + // this.put(key, (Float) value + 1); + // 1. The (Object)value will be typecasted to (Float)value since it is an instanceof Float actually nothing is done. + // 2. Float instance will be autoboxed into float because the + operator will work on primitives not Objects! + // 3. A float+float operation will be performed and results into a float primitive. + // 4. There is no method that matches the signature put( String key, float value), java-compiler will choose the method + // put( String key, double value) and does an implicit type-cast(!) by appending zero-bits to the mantissa + assertTrue( "JSONObject increment unexpected behaviour, Float will not stay Float!", jo.get( "bug" ) instanceof Float ); + // correct implementation (with change of behaviour) would be: + // this.put(key, new Float((Float) value + 1)); + // Probably it would be better to deprecate the method and remove some day, while convenient processing the "payload" is not + // really in the the scope of a JSON-library (IMHO.) + + // Some more examples of well documented unexpeced "numbercrunching" ;) + assertTrue("Stumbled over explicitly type-casting float as double!", (double)0.2f == 0.2d ); + assertTrue("Stumbled over comparing float with double any implicit type-cast!", 0.2f == 0.2d ); + Double d1 = new Double( 1.1f ); + Double d2 = new Double( "1.1f" ); + assertTrue( "Stumbled again over implicit type cast from float to double before calling Double(double d) constructor", d1.equals( d2 ) ); } @Test From fa79826f0c429cdfc3d060697c6790b7d4b31ee6 Mon Sep 17 00:00:00 2001 From: dieter Date: Wed, 27 May 2015 16:33:42 +0200 Subject: [PATCH 2/2] Better show what has to be expected and what goes wrong --- JSONObjectTest.java | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/JSONObjectTest.java b/JSONObjectTest.java index c02ebd1..06f96e5 100644 --- a/JSONObjectTest.java +++ b/JSONObjectTest.java @@ -695,16 +695,19 @@ public class JSONObjectTest { "\"keyDouble\":3.1,"+ // TODO: not sure if this will work on other platforms - // Should work the same way on any platform, this the effect of a float to double conversion and happens because - // java type-casts float to double. A 32 bit float is type-casted to 64 bit double by simply appending zero-bits to the - // mantissa (and extended the signed exponent by 3 bits.) + // Should work the same way on any platform! @see https://docs.oracle.com/javase/specs/jls/se7/html/jls-4.html#jls-4.2.3 + // This is the effect of a float to double conversion and is inherent to the shortcomings of the IEEE 754 format, when + // converting 32-bit into double-precision 64-bit. + // Java type-casts float to double. A 32 bit float is type-casted to 64 bit double by simply appending zero-bits to the + // mantissa (and extended the signed exponent by 3 bits.) and there is no way to obtain more information than it is + // stored in the 32-bits float. - // Like 1/3 cannot be represented as base10 number because it is periodically, 1/5 cannot be represented as base2 number - // since it is periodically in base2 (take a look at http://www.h-schmidt.net/FloatConverter/) + // Like 1/3 cannot be represented as base10 number because it is periodically, 1/5 (for example) cannot be represented + // as base2 number since it is periodically in base2 (take a look at http://www.h-schmidt.net/FloatConverter/) // The same happens to 3.1, that decimal number (base10 representation) is periodic in base2 representation, therefore - // appending zero-bits is inaccurate only repeating the periodically repeating bits (0110) would be a proper conversion. + // appending zero-bits is inaccurate. Only repeating the periodically occuring bits (0110) would be a proper conversion. // However one cannot detect from a 32 bit IEE754 representation which bits would "repeat infinitely", since the missing - // bits would not fit into the 32 bit float, i.e. the information needed is not there! ;) + // bits would not fit into the 32 bit float, i.e. the information needed simply is not there! "\"keyFloat\":3.0999999046325684,"+ "}"; JSONObject jsonObject = new JSONObject(str); @@ -738,12 +741,19 @@ public class JSONObjectTest { 100000000001000110011001100110011000000000000000000000000000000 100000000001000110011001100110011001100110011001100110011001101 */ + // Examples of well documented but probably unexpected behavior in java / with 32-bit float to 64-bit float conversion. + assertFalse("Document unexpected behaviour with explicit type-casting float as double!", (double)0.2f == 0.2d ); + assertFalse("Document unexpected behaviour with implicit type-cast!", 0.2f == 0.2d ); + Double d1 = new Double( 1.1f ); + Double d2 = new Double( "1.1f" ); + assertFalse( "Document implicit type cast from float to double before calling Double(double d) constructor", d1.equals( d2 ) ); + assertTrue( "Correctly converting float to double via base10 (string) representation!", new Double( 3.1d ).equals( new Double( new Float( 3.1f ).toString() ) ) ); // Pinpointing the not so obvious "buggy" conversion from float to double in JSONObject JSONObject jo = new JSONObject(); jo.put( "bug", 3.1f ); // will call put( String key, double value ) with implicit and "buggy" type-cast from float to double - assertFalse( "The java-compiler did add some zero bits for you (probably unexpected, but well documented)", jo.get( "bug" ).equals( new Double( 3.1d ) ) ); + assertFalse( "The java-compiler did add some zero bits for you to the mantissa (unexpected, but well documented)", jo.get( "bug" ).equals( new Double( 3.1d ) ) ); JSONObject inc = new JSONObject(); inc.put( "bug", new Float( 3.1f ) ); // This will put in instance of Float into JSONObject, i.e. call put( String key, Object value ) @@ -755,18 +765,12 @@ public class JSONObjectTest { // 3. A float+float operation will be performed and results into a float primitive. // 4. There is no method that matches the signature put( String key, float value), java-compiler will choose the method // put( String key, double value) and does an implicit type-cast(!) by appending zero-bits to the mantissa - assertTrue( "JSONObject increment unexpected behaviour, Float will not stay Float!", jo.get( "bug" ) instanceof Float ); - // correct implementation (with change of behaviour) would be: + assertTrue( "JSONObject increment unexpected behavior, Float will not stay Float!", jo.get( "bug" ) instanceof Float ); + // correct implementation (with change of behavior) would be: // this.put(key, new Float((Float) value + 1)); // Probably it would be better to deprecate the method and remove some day, while convenient processing the "payload" is not // really in the the scope of a JSON-library (IMHO.) - // Some more examples of well documented unexpeced "numbercrunching" ;) - assertTrue("Stumbled over explicitly type-casting float as double!", (double)0.2f == 0.2d ); - assertTrue("Stumbled over comparing float with double any implicit type-cast!", 0.2f == 0.2d ); - Double d1 = new Double( 1.1f ); - Double d2 = new Double( "1.1f" ); - assertTrue( "Stumbled again over implicit type cast from float to double before calling Double(double d) constructor", d1.equals( d2 ) ); } @Test