compare_double mishandles infinities: equal infinities compare unequal, opposite infinities may compare equal
compare_double() does not handle non-finite values correctly.
In particular:
- two separately allocated numbers containing
+INFINITYcompare unequal; - two separately allocated numbers containing
-INFINITYcompare unequal; +INFINITYand-INFINITYcompare equal incJSON_Compare().
The issue comes from applying the relative-error comparison to infinities without handling non-finite values first.
Affected code
In cJSON.c, compare_double() is currently equivalent to:
static cJSON_bool compare_double(double a, double b)
{
double maxVal = fabs(a) > fabs(b) ? fabs(a) : fabs(b);
return (fabs(a - b) <= maxVal * DBL_EPSILON) ? true : false;
}For infinities this gives unexpected results.
For equal infinities:
a = +Inf
b = +Inf
a - b = NaN
fabs(a - b) = NaN
maxVal * epsilon = Inf
NaN <= Inf = falseso two distinct +Inf values compare unequal. The same happens for -Inf.
For opposite infinities:
a = +Inf
b = -Inf
a - b = +Inf
fabs(a - b) = +Inf
maxVal * epsilon = +Inf
Inf <= Inf = trueso +Inf and -Inf compare equal.
cJSON_Compare() uses this function for cJSON_Number, so these results are observable through the public API.
A similar compare_double() implementation also exists in cJSON_Utils.c.
Reproduction
#include <stdio.h>
#include <math.h>
#include "cJSON.h"
int main(void)
{
cJSON *p1 = cJSON_CreateNumber(INFINITY);
cJSON *p2 = cJSON_CreateNumber(INFINITY);
cJSON *n1 = cJSON_CreateNumber(-INFINITY);
cJSON *n2 = cJSON_CreateNumber(-INFINITY);
if (!p1 || !p2 || !n1 || !n2)
{
return 1;
}
printf("+Inf vs +Inf: %d\n", cJSON_Compare(p1, p2, 1));
printf("-Inf vs -Inf: %d\n", cJSON_Compare(n1, n2, 1));
printf("+Inf vs -Inf: %d\n", cJSON_Compare(p1, n1, 1));
printf("-Inf vs +Inf: %d\n", cJSON_Compare(n1, p1, 1));
cJSON_Delete(p1);
cJSON_Delete(p2);
cJSON_Delete(n1);
cJSON_Delete(n2);
return 0;
}Observed result:
+Inf vs +Inf: 0
-Inf vs -Inf: 0
+Inf vs -Inf: 1
-Inf vs +Inf: 1Expected behavior would be:
+Inf vs +Inf: 1
-Inf vs -Inf: 1
+Inf vs -Inf: 0
-Inf vs +Inf: 0Why this is reachable
Although JSON itself has no NaN or Infinity literals, non-finite values are reachable through the cJSON C API:
cJSON_CreateNumber(INFINITY);
cJSON_SetNumberValue(item, INFINITY);Infinity can also arise while parsing a syntactically valid JSON number whose magnitude exceeds the finite range of double, depending on the strtod() implementation, for example:
cJSON_Parse("1e999");So the comparison code should not assume that every stored double is finite.
Suggested fix
Handle non-finite values before applying the relative-error calculation.
For example, exact equality can be checked first:
static cJSON_bool compare_double(double a, double b)
{
double maxVal;
if (a == b)
{
return true;
}
if (!isfinite(a) || !isfinite(b))
{
return false;
}
maxVal = fabs(a) > fabs(b) ? fabs(a) : fabs(b);
return (fabs(a - b) <= maxVal * DBL_EPSILON) ? true : false;
}This gives the expected infinity behavior while preserving the existing approximate comparison for finite values.
The equivalent implementation in cJSON_Utils.c should be updated as well.
Related NaN-to-integer issue
While reviewing the same non-finite-number paths, cJSON_CreateNumber() / cJSON_SetNumberHelper() also reach:
valueint = (int)number;for NaN, because both comparisons against INT_MAX and INT_MIN are false for NaN. Converting NaN to an integer type this way is undefined behavior when the value cannot be represented.
However, that issue is already tracked by #999 and addressed by PR #1000, so I am not proposing to duplicate it here.
This issue is specifically about the incorrect infinity comparison behavior in compare_double().
Source: DaveGamble/cJSON