Fix negative mantissa bug for decimal fractions and big floats (#265)
QCBORDecode_GetBigFloatBig() and QCBORDecode_GetDecimalFractionBig() return negative big number mantissas correctly now. They were off by one for not correctly taking the CBOR offset of 1 for negative numbers in some cases
QCBORDecode_DoubleConvertAll() was off by 1 when the input to decode was a decimal fraction with a negative big number mantissa.
Behavior of expAndMantissa in QCBORItem documented better.
* Fix bug negative mantissa bug for decimal fractions and big floats
* Fix neg big num off by 1 for decimal fraction conversion to float
---------
Co-authored-by: Laurence Lundblade <lgl@securitytheory.com>
diff --git a/inc/qcbor/qcbor_decode.h b/inc/qcbor/qcbor_decode.h
index 7df9094..f832fd1 100644
--- a/inc/qcbor/qcbor_decode.h
+++ b/inc/qcbor/qcbor_decode.h
@@ -380,6 +380,49 @@
/**
+ * @anchor expAndMantissa
+ *
+ * This holds the value for big floats and decimal fractions, as an
+ * exponent and mantissa. For big floats the base for exponentiation
+ * is 2. For decimal fractions it is 10. Whether an instance is a big
+ * float or decimal fraction is known by context, usually by @c uDataType
+ * in @ref QCBORItem which might be @ref QCBOR_TYPE_DECIMAL_FRACTION,
+ * @ref QCBOR_TYPE_BIGFLOAT, ...
+ *
+ * The mantissa may be an @c int64_t or a big number. This is again
+ * determined by context, usually @c uDataType in @ref QCBORItem which
+ * might be @ref QCBOR_TYPE_DECIMAL_FRACTION,
+ * @ref QCBOR_TYPE_DECIMAL_FRACTION_POS_BIGNUM, ... The sign of the
+ * big number also comes from the context
+ * (@ref QCBOR_TYPE_DECIMAL_FRACTION_POS_BIGNUM,
+ * @ref QCBOR_TYPE_DECIMAL_FRACTION_NEG_BIGNUM,...).
+ *
+ * @c bigNum is big endian or network byte order. The most significant
+ * byte is first.
+ *
+ * When @c Mantissa is @c int64_t, it represents the true value of the
+ * mantissa with the offset of 1 for CBOR negative values
+ * applied. When it is a negative big number
+ * (@ref QCBOR_TYPE_DECIMAL_FRACTION_NEG_BIGNUM or
+ * @ref QCBOR_TYPE_BIGFLOAT_NEG_BIGNUM), the offset of 1 has NOT been
+ * applied (doing so requires somewhat complex big number arithmetic
+ * and may increase the length of the big number). To get the correct
+ * value @c bigNum must be incremented by one before use.
+ *
+ * Also see QCBOREncode_AddTDecimalFraction(),
+ * QCBOREncode_AddTBigFloat(), QCBOREncode_AddTDecimalFractionBigNum()
+ * and QCBOREncode_AddTBigFloatBigNum().
+ */
+typedef struct {
+ int64_t nExponent;
+ union {
+ int64_t nInt;
+ UsefulBufC bigNum;
+ } Mantissa;
+} QCBORExpAndMantissa;
+
+
+/**
* This holds a decoded data item. It is returned by the
* QCBORDecode_GetNext(), the principle decoding function.
* It holds the type, value, label, tags and other details
@@ -472,40 +515,8 @@
/** See @ref QCBOR_TYPE_UKNOWN_SIMPLE */
uint8_t uSimple;
#ifndef QCBOR_DISABLE_EXP_AND_MANTISSA
- /**
- * @anchor expAndMantissa
- *
- * This holds the value for big floats and decimal fractions.
- * The use of the fields in this structure depends on @c
- * uDataType.
- *
- * When @c uDataType indicates a decimal fraction, the
- * @c nExponent is base 10. When it indicates a big float, it
- * is base 2.
- *
- * When @c uDataType indicates a big number, then the @c bigNum
- * member of @c Mantissa is valid. Otherwise the @c nInt member
- * of @c Mantissa is valid.
- *
- * See @ref QCBOR_TYPE_DECIMAL_FRACTION,
- * @ref QCBOR_TYPE_DECIMAL_FRACTION_POS_BIGNUM,
- * @ref QCBOR_TYPE_DECIMAL_FRACTION_NEG_BIGNUM,
- * @ref QCBOR_TYPE_BIGFLOAT, @ref QCBOR_TYPE_BIGFLOAT_POS_BIGNUM,
- * and @ref QCBOR_TYPE_BIGFLOAT_NEG_BIGNUM.
- *
- * Also see QCBOREncode_AddTDecimalFraction(),
- * QCBOREncode_AddTBigFloat(),
- * QCBOREncode_AddTDecimalFractionBigNum() and
- * QCBOREncode_AddTBigFloatBigNum().
- */
- struct {
- int64_t nExponent;
- union {
- int64_t nInt;
- UsefulBufC bigNum;
- } Mantissa;
- } expAndMantissa;
-#endif /* QCBOR_DISABLE_EXP_AND_MANTISSA */
+ QCBORExpAndMantissa expAndMantissa;
+#endif /* ! QCBOR_DISABLE_EXP_AND_MANTISSA */
uint64_t uTagV; /* Used internally during decoding */
} val;
diff --git a/inc/qcbor/qcbor_spiffy_decode.h b/inc/qcbor/qcbor_spiffy_decode.h
index 2b2311a..dcb03f3 100644
--- a/inc/qcbor/qcbor_spiffy_decode.h
+++ b/inc/qcbor/qcbor_spiffy_decode.h
@@ -1549,6 +1549,22 @@
* will convert all these to a big number. The limit to this
* conversion is the size of @c MantissaBuffer.
*
+ * The mantissa returned does NOT have the offset
+ * of one applied when it is negative. To get the true value
+ * one must be added to @c *pMantissa (which requires
+ * some big number arithmetic and may increase the length
+ * of it by one).
+ *
+ * For QCBOR before v1.5, this function had a bug where
+ * by the negative mantissa sometimes had the offset of
+ * one applied, making this function somewhat usless for
+ * negative mantissas. Specifically if the to-be-decode CBOR
+ * was a type 1 integer the offset was applied and when it
+ * was a tag 3, the offset was not applied. It is possible
+ * that a tag 3 could contain a value in the range of a type 1
+ * integer. @ref QCBORExpAndMantissa is
+ * correct and can be used instead of this.
+ *
* See also QCBORDecode_GetInt64ConvertAll(),
* QCBORDecode_GetUInt64ConvertAll() and
* QCBORDecode_GetDoubleConvertAll() which can convert decimal
@@ -1648,6 +1664,9 @@
*
* mantissa * ( 2 ** exponent )
*
+ * This has the same issue with negative mantissas as
+ * QCBORDecode_GetDecimalFractionBig().
+ *
* See also QCBORDecode_GetInt64ConvertAll(),
* QCBORDecode_GetUInt64ConvertAll() and
* QCBORDecode_GetDoubleConvertAll() which can convert big floats.
diff --git a/src/qcbor_decode.c b/src/qcbor_decode.c
index 1d20077..1433082 100644
--- a/src/qcbor_decode.c
+++ b/src/qcbor_decode.c
@@ -6469,7 +6469,7 @@
return QCBOR_ERR_UNEXPECTED_TYPE;
}
break;
-#endif /* ndef QCBOR_DISABLE_EXP_AND_MANTISSA */
+#endif /* ! QCBOR_DISABLE_EXP_AND_MANTISSA */
case QCBOR_TYPE_POSBIGNUM:
if(uConvertTypes & QCBOR_CONVERT_TYPE_BIG_NUM) {
@@ -6498,32 +6498,33 @@
break;
case QCBOR_TYPE_DECIMAL_FRACTION_NEG_BIGNUM:
- if(uConvertTypes & QCBOR_CONVERT_TYPE_DECIMAL_FRACTION) {
- double dMantissa = -QCBOR_Private_ConvertBigNumToDouble(pItem->val.expAndMantissa.Mantissa.bigNum);
- *pdValue = dMantissa * pow(10, (double)pItem->val.expAndMantissa.nExponent);
+ if(uConvertTypes & QCBOR_CONVERT_TYPE_DECIMAL_FRACTION) {
+ /* Must subtract 1 for CBOR negative integer offset */
+ double dMantissa = -1-QCBOR_Private_ConvertBigNumToDouble(pItem->val.expAndMantissa.Mantissa.bigNum);
+ *pdValue = dMantissa * pow(10, (double)pItem->val.expAndMantissa.nExponent);
} else {
return QCBOR_ERR_UNEXPECTED_TYPE;
}
break;
case QCBOR_TYPE_BIGFLOAT_POS_BIGNUM:
- if(uConvertTypes & QCBOR_CONVERT_TYPE_BIGFLOAT) {
- double dMantissa = QCBOR_Private_ConvertBigNumToDouble(pItem->val.expAndMantissa.Mantissa.bigNum);
- *pdValue = dMantissa * exp2((double)pItem->val.expAndMantissa.nExponent);
+ if(uConvertTypes & QCBOR_CONVERT_TYPE_BIGFLOAT) {
+ double dMantissa = QCBOR_Private_ConvertBigNumToDouble(pItem->val.expAndMantissa.Mantissa.bigNum);
+ *pdValue = dMantissa * exp2((double)pItem->val.expAndMantissa.nExponent);
} else {
return QCBOR_ERR_UNEXPECTED_TYPE;
}
break;
case QCBOR_TYPE_BIGFLOAT_NEG_BIGNUM:
- if(uConvertTypes & QCBOR_CONVERT_TYPE_BIGFLOAT) {
- double dMantissa = -1-QCBOR_Private_ConvertBigNumToDouble(pItem->val.expAndMantissa.Mantissa.bigNum);
- *pdValue = dMantissa * exp2((double)pItem->val.expAndMantissa.nExponent);
+ if(uConvertTypes & QCBOR_CONVERT_TYPE_BIGFLOAT) {
+ double dMantissa = -1-QCBOR_Private_ConvertBigNumToDouble(pItem->val.expAndMantissa.Mantissa.bigNum);
+ *pdValue = dMantissa * exp2((double)pItem->val.expAndMantissa.nExponent);
} else {
return QCBOR_ERR_UNEXPECTED_TYPE;
}
break;
-#endif /* ndef QCBOR_DISABLE_EXP_AND_MANTISSA */
+#endif /* ! QCBOR_DISABLE_EXP_AND_MANTISSA */
default:
return QCBOR_ERR_UNEXPECTED_TYPE;
@@ -6859,6 +6860,7 @@
int64_t *pnExponent)
{
QCBORError uErr;
+ uint64_t uMantissa;
if(pMe->uLastError != QCBOR_SUCCESS) {
return;
@@ -6869,25 +6871,29 @@
goto Done;
}
- uint64_t uMantissa;
-
switch (pItem->uDataType) {
case QCBOR_TYPE_DECIMAL_FRACTION:
case QCBOR_TYPE_BIGFLOAT:
- /* See comments in ExponentiateNN() on handling INT64_MIN */
if(pItem->val.expAndMantissa.Mantissa.nInt >= 0) {
uMantissa = (uint64_t)pItem->val.expAndMantissa.Mantissa.nInt;
*pbIsNegative = false;
- } else if(pItem->val.expAndMantissa.Mantissa.nInt != INT64_MIN) {
- uMantissa = (uint64_t)-pItem->val.expAndMantissa.Mantissa.nInt;
- *pbIsNegative = true;
} else {
- uMantissa = (uint64_t)INT64_MAX+1;
+ if(pItem->val.expAndMantissa.Mantissa.nInt != INT64_MIN) {
+ uMantissa = (uint64_t)-pItem->val.expAndMantissa.Mantissa.nInt;
+ } else {
+ /* Can't negate like above when int64_t is INT64_MIN because it
+ * will overflow. See ExponentNN() */
+ uMantissa = (uint64_t)INT64_MAX+1;
+ }
*pbIsNegative = true;
}
- *pMantissa = QCBOR_Private_ConvertIntToBigNum(uMantissa,
- BufferForMantissa);
+ /* Reverse the offset by 1 for type 1 negative value to be consistent
+ * with big num case below which don't offset because it requires
+ * big number arithmetic. This is a bug fix for QCBOR v1.5.
+ */
+ uMantissa--;
+ *pMantissa = QCBOR_Private_ConvertIntToBigNum(uMantissa, BufferForMantissa);
*pnExponent = pItem->val.expAndMantissa.nExponent;
break;
diff --git a/test/qcbor_decode_tests.c b/test/qcbor_decode_tests.c
index d7ef447..910d43a 100644
--- a/test/qcbor_decode_tests.c
+++ b/test/qcbor_decode_tests.c
@@ -5832,7 +5832,7 @@
QCBOR_SUCCESS, /* for GetDecimalFractionBig */
-1,
- {(const uint8_t []){0x03}, 1},
+ {(const uint8_t []){0x02}, 1},
false,
QCBOR_SUCCESS, /* for GetBigFloat */
@@ -5841,7 +5841,7 @@
QCBOR_SUCCESS, /* for GetBigFloatBig */
-1,
- {(const uint8_t []){0x03}, 1},
+ {(const uint8_t []){0x02}, 1},
false
},
@@ -5896,7 +5896,7 @@
QCBOR_SUCCESS, /* for GetDecimalFractionBig */
-1,
- {(const uint8_t []){0x03}, 1},
+ {(const uint8_t []){0x02}, 1},
false,
QCBOR_ERR_UNEXPECTED_TYPE, /* for GetBigFloat */
@@ -5927,7 +5927,7 @@
QCBOR_ERR_UNEXPECTED_TYPE, /* for GetDecimalFractionBig */
0,
- {(const uint8_t []){0x03}, 1},
+ {(const uint8_t []){0x02}, 1},
false,
QCBOR_SUCCESS, /* for GetBigFloat */
@@ -5936,7 +5936,7 @@
QCBOR_SUCCESS, /* for GetBigFloatBig */
300,
- {(const uint8_t []){0x64}, 1},
+ {(const uint8_t []){0x63}, 1},
false
},
@@ -7555,6 +7555,26 @@
FLOAT_ERR_CODE_NO_FLOAT_HW(EXP_AND_MANTISSA_ERROR(QCBOR_SUCCESS))
},
{
+ "Decimal fraction -3/10",
+ {(uint8_t[]){0xC4, 0x82, 0x20, 0x22}, 4},
+ 0,
+ EXP_AND_MANTISSA_ERROR(QCBOR_ERR_CONVERSION_UNDER_OVER_FLOW),
+ 0,
+ EXP_AND_MANTISSA_ERROR(QCBOR_ERR_NUMBER_SIGN_CONVERSION),
+ -0.30000000000000004,
+ FLOAT_ERR_CODE_NO_FLOAT_HW(EXP_AND_MANTISSA_ERROR(QCBOR_SUCCESS))
+ },
+ {
+ "Decimal fraction -3/10, neg bignum mantissa",
+ {(uint8_t[]){0xC4, 0x82, 0x20, 0xc3, 0x41, 0x02}, 6},
+ 0,
+ EXP_AND_MANTISSA_ERROR(QCBOR_ERR_CONVERSION_UNDER_OVER_FLOW),
+ 0,
+ EXP_AND_MANTISSA_ERROR(QCBOR_ERR_NUMBER_SIGN_CONVERSION),
+ -0.30000000000000004,
+ FLOAT_ERR_CODE_NO_FLOAT_HW(EXP_AND_MANTISSA_ERROR(QCBOR_SUCCESS))
+ },
+ {
"extreme pos bignum",
{(uint8_t[]){0xc2, 0x59, 0x01, 0x90,
// 50 rows of 8 is 400 digits.
@@ -7790,7 +7810,6 @@
INFINITY,
FLOAT_ERR_CODE_NO_FLOAT_HW(QCBOR_SUCCESS)
},
-
};