Skip to content

compare_double mishandles infinities: equal infinities compare unequal, opposite infinities may compare equal #1071

Description

@yayong3

compare_double() does not handle non-finite values correctly.

In particular:

  • two separately allocated numbers containing +INFINITY compare unequal;
  • two separately allocated numbers containing -INFINITY compare unequal;
  • +INFINITY and -INFINITY compare equal in cJSON_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        = false

so 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        = true

so +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: 1

Expected behavior would be:

+Inf vs +Inf: 1
-Inf vs -Inf: 1
+Inf vs -Inf: 0
-Inf vs +Inf: 0

Why 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().

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions