compare_double mishandles infinities: equal infinities compare unequal, opposite infinities may compare equal
I maintainer di solito rispondono entro 12 giorni
Nessuno ha ancora preso questa issue.
- #1075 di @afonsojanu — chiusa senza merge
Valutazione
- Difficoltà
- 2/5
- Tempo stimato
- 1-3 ore
- Idoneità per principianti
- 78/100
Direzione di ricerca
Inizia dalle implementazioni di compare_double() in cJSON.c e cJSON_Utils.c, quindi esegui la riproduzione C fornita usando cJSON_Compare(). Aggiorna il comportamento del confronto dei valori non finiti in entrambe le posizioni e verifica che le infinità uguali risultino uguali, che le infinità opposte risultino diverse e che i confronti approssimati finiti rimangano invariati.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
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 = 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().
- Lingua principale
- C
- Stelle
- 13k
- Fork
- 3.5k
- Metriche di merge delle PR
- Nessuna PR unita negli ultimi 30g
Preparare l'ambiente
- Nessun Dockerfile né file Docker Compose
- Nessun modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di DaveGamble/cJSON
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 68/100
DaveGamble/cJSON#1106 ·
I maintainer di solito rispondono entro 12 giorni
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 70/100
DaveGamble/cJSON#1100 · 1 reazione ·
I maintainer di solito rispondono entro 12 giorni
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 86/100
DaveGamble/cJSON#1094 · 9 commenti · 1 reazione ·
I maintainer di solito rispondono entro 12 giorni
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 68/100
DaveGamble/cJSON#1093 ·
I maintainer di solito rispondono entro 12 giorni
-
Use-after-free in cJSONUtils_ApplyPatches when a patch removes the patch arrayForse già presa @iliasabk l’ha presa 28 giorni fa. Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 82/100
DaveGamble/cJSON#1082 ·
I maintainer di solito rispondono entro 12 giorni
Tutte le issue di DaveGamble/cJSON
Issue simili
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 67/100
dosemu2/dosemu2#3178 · 1 commento · 1 reazione ·
I maintainer di solito rispondono entro 1 giorno
-
backlog bug
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
EchoTools/nevr-runtime#653 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
InfiniTimeOrg/InfiniTime#2492 ·
-
Build fails when `memset_s` is unavailable: `Undefined symbols for architecture ppc: "_memset_s"`Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 65/100
-
XML_GetCurrentByteIndex64() returns -1 instead of a meaningful offset when m_eventPtr is NULLAperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
I maintainer di solito rispondono entro 1 giorno