Skip to content
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.

Sign up for GitHub

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

Fixed bug #50224 where float without decimals were converted to integer when encode to JSON #642

Closed
wants to merge 14 commits into from
Closed

Fixed bug #50224 where float without decimals were converted to integer when encode to JSON #642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
4c263b6
Fixed bug #50224 where float without decimals were converted to integer
jrbasso Mar 29, 2014
56b4ba5
Replacing use of strstr to strchr for performance reasons
jrbasso Apr 13, 2014
9bb2248
Performance updates
jrbasso Apr 20, 2014
cbe1360
Performance optimizations
jrbasso Apr 20, 2014
52a8fb0
Changed how float numbers are converted to JSON strings
jrbasso Apr 21, 2014
bb6697b
Fixing pointer address to avoid compiler warnings
jrbasso Apr 21, 2014
408b7f7
Reducing the size of string for float to string conversion
jrbasso May 23, 2014
a64736e
Fixed typo
jrbasso Jul 16, 2014
2468194
Using cast instead of getting the reference of first element
jrbasso Jul 16, 2014
dd52b7b
Using values from float.h and fallbacking to hardcoded value
jrbasso Jul 28, 2014
8c41b03
Added JSON_PRESERVE_FRACTIONAL_PART option
jrbasso Jul 30, 2014
4a53df1
Optimized the original code when the option JSON_PRESERVE_FRACTIONAL_…
jrbasso Jul 30, 2014
8ff879d
Added a condition to avoid overflow
jrbasso Jul 31, 2014
6c8d3dd
Renamed constant name after discussion on internals list
jrbasso Jan 11, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions ext/json/json.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@
#include "php_json.h"
#include <zend_exceptions.h>

#include <float.h>
#if defined(DBL_MANT_DIG) && defined(DBL_MIN_EXP)
#define NUM_BUF_SIZE (3 + DBL_MANT_DIG - DBL_MIN_EXP)
#else
#define NUM_BUF_SIZE 1080
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that not a tad, er, excessive?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TazeTSchnitzel according with http://tigcc.ticalc.org/doc/float.html the value if the constants are defined is 3 + 16 - -999 = 1018.
There is research from @bukka and he explains on this comment:

You might notice that the default value is 1080. Not sure where I got 1089 - I just tested the value and it's 1077 on my platform and max value that I googled was 1079 so it should never be higher than 1080. I'm almost sure that if it does, the constants will be defined float.h so it won't be a problem...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually allocate less space on the stack than it was before this patch - The spprintf (xbuf_format_converter) allocates 2048 + additional space for other variables and function stack . See http://lxr.php.net/xref/PHP_TRUNK/main/spprintf.c#xbuf_format_converter for more details. As I said 1080 won't be probably used in any case as DBL_MANT_DIG, DBL_MIN_EXP are almost always defined so it will be mostly 1079 :). You might think that it's not necessary as the EG(precision) will be always smaller. Unfortunately we don't know its value at the C compile time (the dynamic allocation leads to the worse perf so we need to allocate on the stack). The only better way might be using alloca. I plan to experiment with that later to see if there no perf penalty but we need to have fallback anyway for non-alloca platforms. That requires the max space for double value otherwise there would be chance of the stack overflow...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this was C99 we could do dynamic stack allocations. Alas.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, the constants are defined on windows, linux and mac. So just in rare cases it will fallback to the hardcoded value.

#endif


static PHP_MINFO_FUNCTION(json);
static PHP_FUNCTION(json_encode);
static PHP_FUNCTION(json_decode);
Expand Down Expand Up @@ -103,6 +111,7 @@ static PHP_MINIT_FUNCTION(json)
REGISTER_LONG_CONSTANT("JSON_PRETTY_PRINT", PHP_JSON_PRETTY_PRINT, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("JSON_UNESCAPED_UNICODE", PHP_JSON_UNESCAPED_UNICODE, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("JSON_PARTIAL_OUTPUT_ON_ERROR", PHP_JSON_PARTIAL_OUTPUT_ON_ERROR, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("JSON_PRESERVE_ZERO_FRACTION", PHP_JSON_PRESERVE_ZERO_FRACTION, CONST_CS | CONST_PERSISTENT);

REGISTER_LONG_CONSTANT("JSON_ERROR_NONE", PHP_JSON_ERROR_NONE, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("JSON_ERROR_DEPTH", PHP_JSON_ERROR_DEPTH, CONST_CS | CONST_PERSISTENT);
Expand Down Expand Up @@ -420,10 +429,17 @@ static void json_escape_string(smart_str *buf, char *s, int len, int options TSR
smart_str_append_long(buf, p);
} else if (type == IS_DOUBLE) {
if (!zend_isinf(d) && !zend_isnan(d)) {
char *tmp;
int l = spprintf(&tmp, 0, "%.*k", (int) EG(precision), d);
smart_str_appendl(buf, tmp, l);
efree(tmp);
char num[NUM_BUF_SIZE];
int l;

php_gcvt(d, EG(precision), '.', 'e', (char *)num);
l = strlen(num);
if (options & PHP_JSON_PRESERVE_ZERO_FRACTION && strchr(num, '.') == NULL && l < NUM_BUF_SIZE - 2) {
num[l++] = '.';
num[l++] = '0';
num[l] = '\0';
}
smart_str_appendl(buf, num, l);
} else {
JSON_G(error_code) = PHP_JSON_ERROR_INF_OR_NAN;
smart_str_appendc(buf, '0');
Expand Down Expand Up @@ -624,14 +640,19 @@ PHP_JSON_API void php_json_encode(smart_str *buf, zval *val, int options TSRMLS_

case IS_DOUBLE:
{
char *d = NULL;
char num[NUM_BUF_SIZE];
int len;
double dbl = Z_DVAL_P(val);

if (!zend_isinf(dbl) && !zend_isnan(dbl)) {
len = spprintf(&d, 0, "%.*k", (int) EG(precision), dbl);
smart_str_appendl(buf, d, len);
efree(d);
php_gcvt(dbl, EG(precision), '.', 'e', (char *)num);
len = strlen(num);
if (options & PHP_JSON_PRESERVE_ZERO_FRACTION && strchr(num, '.') == NULL && len < NUM_BUF_SIZE - 2) {
num[len++] = '.';
num[len++] = '0';
num[len] = '\0';
}
smart_str_appendl(buf, num, len);
} else {
JSON_G(error_code) = PHP_JSON_ERROR_INF_OR_NAN;
smart_str_appendc(buf, '0');
Expand Down
1 change: 1 addition & 0 deletions ext/json/php_json.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ extern PHP_JSON_API zend_class_entry *php_json_serializable_ce;
#define PHP_JSON_PRETTY_PRINT (1<<7)
#define PHP_JSON_UNESCAPED_UNICODE (1<<8)
#define PHP_JSON_PARTIAL_OUTPUT_ON_ERROR (1<<9)
#define PHP_JSON_PRESERVE_ZERO_FRACTION (1<<10)

/* Internal flags */
#define PHP_JSON_OUTPUT_ARRAY 0
Expand Down
60 changes: 60 additions & 0 deletions ext/json/tests/bug50224.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
--TEST--
bug #50224 (json_encode() does not always encode a float as a float)
--SKIPIF--
<?php if (!extension_loaded("json")) print "skip"; ?>
--FILE--
<?php
echo "* Testing JSON output\n\n";
var_dump(json_encode(12.3, JSON_PRESERVE_ZERO_FRACTION));
var_dump(json_encode(12, JSON_PRESERVE_ZERO_FRACTION));
var_dump(json_encode(12.0, JSON_PRESERVE_ZERO_FRACTION));
var_dump(json_encode(0.0, JSON_PRESERVE_ZERO_FRACTION));
var_dump(json_encode(array(12, 12.0, 12.3), JSON_PRESERVE_ZERO_FRACTION));
var_dump(json_encode((object)array('float' => 12.0, 'integer' => 12), JSON_PRESERVE_ZERO_FRACTION));

echo "\n* Testing encode/decode symmetry\n\n";

var_dump(json_decode(json_encode(12.3, JSON_PRESERVE_ZERO_FRACTION)));
var_dump(json_decode(json_encode(12, JSON_PRESERVE_ZERO_FRACTION)));
var_dump(json_decode(json_encode(12.0, JSON_PRESERVE_ZERO_FRACTION)));
var_dump(json_decode(json_encode(0.0, JSON_PRESERVE_ZERO_FRACTION)));
var_dump(json_decode(json_encode(array(12, 12.0, 12.3), JSON_PRESERVE_ZERO_FRACTION)));
var_dump(json_decode(json_encode((object)array('float' => 12.0, 'integer' => 12), JSON_PRESERVE_ZERO_FRACTION)));
var_dump(json_decode(json_encode((object)array('float' => 12.0, 'integer' => 12), JSON_PRESERVE_ZERO_FRACTION), true));
?>
--EXPECTF--
* Testing JSON output

string(4) "12.3"
string(2) "12"
string(4) "12.0"
string(3) "0.0"
string(14) "[12,12.0,12.3]"
string(27) "{"float":12.0,"integer":12}"

* Testing encode/decode symmetry

float(12.3)
int(12)
float(12)
float(0)
array(3) {
[0]=>
int(12)
[1]=>
float(12)
[2]=>
float(12.3)
}
object(stdClass)#%d (2) {
["float"]=>
float(12)
["integer"]=>
int(12)
}
array(2) {
["float"]=>
float(12)
["integer"]=>
int(12)
}