backout map label checking; doc; test
diff --git a/inc/qcbor/qcbor_common.h b/inc/qcbor/qcbor_common.h
index 0577d0f..4255a86 100644
--- a/inc/qcbor/qcbor_common.h
+++ b/inc/qcbor/qcbor_common.h
@@ -31,6 +31,7 @@
* IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
* ========================================================================= */
+//#define USEFULBUF_DISABLE_ALL_FLOAT
#ifndef qcbor_common_h
#define qcbor_common_h
diff --git a/inc/qcbor/qcbor_decode.h b/inc/qcbor/qcbor_decode.h
index 0acc2a3..c2da5a2 100644
--- a/inc/qcbor/qcbor_decode.h
+++ b/inc/qcbor/qcbor_decode.h
@@ -194,16 +194,27 @@
QCBOR_DECODE_MODE_MAP_STRINGS_ONLY = 1,
/** See QCBORDecode_Init() */
QCBOR_DECODE_MODE_MAP_AS_ARRAY = 2,
- /** TODO:
- * This requires preferred serialization -- no indef lengths; shortest form integers and floats */
+ /**
+ * This checks that the input is encoded with preferred
+ * serialization. The checking is performed as each item is
+ * decoded. If no QCBORDecode_GetXxx() is called for an item,
+ * there's no check on that item. Preferred serialization was first
+ * defined in section 4.1 of RFC 8949, but is more sharply in
+ * draft-ietf-cbor-cde. Summarizing, the requirements are: the use
+ * of definite-length encoding only, integers, including string
+ * lengths and tags, must be in shortest form, and floating-point
+ * numbers must be reduced to shortest form all the way to
+ * half-precision. */
QCBOR_DECODE_MODE_PREFERRED = 3,
- /** This require maps to be sorted by label. This also performs full duplicat label checking
- * on every map before it is returned. This mode adds considerable CPU-time expense
- * to decoding.
+ /** This checks that maps in the input are sorted by label as
+ * described in RFC 8949 section 4.2.1. This also performs
+ * duplicate label checking. This mode adds considerable CPU-time
+ * expense to decoding, though it is probably only of consequence
+ * for large inputs on slow CPUs.
*
- * This also performs all the checks that QCBOR_DECODE_MODE_PREFERRED
- * does. */
+ * This also performs all the checks that
+ * QCBOR_DECODE_MODE_PREFERRED does. */
QCBOR_DECODE_MODE_CDE = 4,
/** This requires integer-float unification. It performs all the checks that
diff --git a/inc/qcbor/qcbor_encode.h b/inc/qcbor/qcbor_encode.h
index 603f1fd..a9aebff 100644
--- a/inc/qcbor/qcbor_encode.h
+++ b/inc/qcbor/qcbor_encode.h
@@ -358,6 +358,7 @@
* - Max items in an array or map when encoding or decoding is
* @ref QCBOR_MAX_ITEMS_IN_ARRAY (typically 65,536).
* - Does not directly support labels in maps other than text strings & integers.
+ * - Traversal, duplicate and sort order checking errors out for labels that are arrays or maps.
* - Does not directly support integer labels beyond whats fits in @c int64_t
* or @c uint64_t.
* - Epoch dates limited to @c INT64_MAX (+/- 292 billion years).
diff --git a/src/qcbor_decode.c b/src/qcbor_decode.c
index 3ed20a2..543b01a 100644
--- a/src/qcbor_decode.c
+++ b/src/qcbor_decode.c
@@ -1932,29 +1932,6 @@
}
-/* Only works in definite lengths. Used to skip over labels that
- * are maps or arrays.
- */
-static QCBORError
-ConsumeArrayOrMap(QCBORDecodeContext *pMe, int nCount)
-{
- QCBORError uErr, uErr2;
- QCBORItem Item;
-
- uErr = QCBOR_SUCCESS;
- for(;nCount > 0; nCount--) {
- uErr2 = QCBORDecode_Private_GetNextTagNumber(pMe, &Item);
- if(QCBORDecode_IsUnrecoverableError(uErr2)) {
- break;
- }
- if(uErr2 != QCBOR_SUCCESS) {
- uErr = uErr2;
- }
- }
-
- return uErr;
-}
-
/**
* @brief Combine a map entry label and value into one item (decode layer 3).
*
@@ -2018,25 +1995,11 @@
/* Decoding a map entry, so the item decoded above was the label */
LabelItem = *pDecodedItem;
-
- if(QCBORItem_IsMapOrArray(LabelItem) && pMe->bAllowAllLabels) {
- int nCount = LabelItem.val.uCount;
- if(LabelItem.uDataType != QCBOR_TYPE_ARRAY) {
- nCount *= 2;
- }
- uErr2 = ConsumeArrayOrMap(pMe, nCount);
- if(QCBORDecode_IsUnrecoverableError(uErr2)) {
- goto Done;
- }
- if(uErr2 != QCBOR_SUCCESS) {
- uErr = uErr2;
- }
- }
+
if(puLabelEndOffset != NULL) {
- /* Cast is OK because lengths are all 32-bit in QCBOR */
- *puLabelEndOffset = (uint32_t)UsefulInputBuf_Tell(&(pMe->InBuf));
- }
-
+ /* Cast is OK because lengths are all 32-bit in QCBOR */
+ *puLabelEndOffset = (uint32_t)UsefulInputBuf_Tell(&(pMe->InBuf));
+ }
/* Get the value of the map item */
uErr2 = QCBORDecode_Private_GetNextTagNumber(pMe, pDecodedItem);
@@ -2081,7 +2044,12 @@
#endif /* ! QCBOR_DISABLE_NON_INTEGER_LABELS */
default:
- if(!pMe->bAllowAllLabels) {
+ /* It is possible to skip over labels that are non-aggregate
+ * types like floats, but not to skip over labels that are
+ * arrays or maps. We might eventually handle more label
+ * types like floats as they are not too hard and we now
+ * have QCBOR_DISABLE_NON_INTEGER_LABELS */
+ if(!pMe->bAllowAllLabels || QCBORItem_IsMapOrArray(LabelItem)) {
uErr = QCBOR_ERR_MAP_LABEL_TYPE;
goto Done;
}
@@ -2959,24 +2927,67 @@
}
-// TODO: move this forward declration?
+/**
+ * @brief Consume an entire map or array including its contents.
+ *
+ * @param[in] pMe The decoder context.
+ * @param[in] pItemToConsume The array/map whose contents are to be
+ * consumed.
+ * @param[out] puNextNestLevel The next nesting level after the item was
+ * fully consumed.
+ *
+ * This may be called when @c pItemToConsume is not an array or
+ * map. In that case, this is just a pass through for @c puNextNestLevel
+ * since there is nothing to do.
+ */
static QCBORError
QCBORDecode_Private_ConsumeItem(QCBORDecodeContext *pMe,
const QCBORItem *pItemToConsume,
bool *pbBreak,
- uint8_t *puNextNestLevel);
+ uint8_t *puNextNestLevel)
+{
+ QCBORError uReturn;
+ QCBORItem Item;
+ /* If it is a map or array, this will tell if it is empty. */
+ const bool bIsEmpty = (pItemToConsume->uNextNestLevel <= pItemToConsume->uNestingLevel);
+ if(QCBORItem_IsMapOrArray(*pItemToConsume) && !bIsEmpty) {
+ /* There is only real work to do for non-empty maps and arrays */
+ /* This works for definite- and indefinite-length maps and
+ * arrays by using the nesting level
+ */
+ do {
+ uReturn = QCBORDecode_Private_GetNextMapOrArray(pMe, pbBreak, &Item, NULL);
+ if(QCBORDecode_IsUnrecoverableError(uReturn) ||
+ uReturn == QCBOR_ERR_NO_MORE_ITEMS) {
+ goto Done;
+ }
+ } while(Item.uNextNestLevel >= pItemToConsume->uNextNestLevel);
+ *puNextNestLevel = Item.uNextNestLevel;
+ uReturn = QCBOR_SUCCESS;
+
+ } else {
+ /* pItemToConsume is not a map or array. Just pass the nesting
+ * level through. */
+ *puNextNestLevel = pItemToConsume->uNextNestLevel;
+
+ uReturn = QCBOR_SUCCESS;
+ }
+
+Done:
+ return uReturn;
+}
/*
*
- * This consumes the next item. It returns the starting
- * position of the label and the length of the label. It
- * also returns the nest level of the item consumed.
+ * This consumes the next item. It returns the starting position of
+ * the label and the length of the label. It also returns the nest
+ * level of the item consumed.
*/
static QCBORError
QCBORDecode_Private_GetLabelAndConsume(QCBORDecodeContext *pMe,
@@ -3006,11 +3017,11 @@
/* Loop over items in a map until the end of the map looking for
- * duplicates. This starts at the current position in the map,
- * not at the beginning of the map.
+ * duplicates. This starts at the current position in the map, not at
+ * the beginning of the map.
*
- * This saves and restores the traversal cursor and nest
- * tracking so they are the same on exit as they were on entry.
+ * This saves and restores the traversal cursor and nest tracking so
+ * they are the same on exit as they were on entry.
*/
static QCBORError
QCBORDecode_Private_CheckDups(QCBORDecodeContext *pMe,
@@ -3040,8 +3051,17 @@
break; /* Successful end of loop */
}
- // TODO: test successful end for indefinite length maps
-
+ /* This check for dups works for labels that are preferred
+ * serialization and are not maps. If the labels are not in
+ * preferred serialization, then the check has to be more
+ * complicated and is type-specific because it uses the decoded
+ * value, not the encoded CBOR. It is further complicated for
+ * maps because the order of items in a map that is a label
+ * doesn't matter when checking that is is the duplicate of
+ * another map that is a label. QCBOR so far only turns on this
+ * dup checking as part of CDE checking which requires preferred
+ * serialization. See 5.6 in RFC 8949.
+ */
nCompare = UsefulInputBuf_Compare(&(pMe->InBuf),
uCompareLabelStart, uCompareLabelLen,
uLabelStart, uLabelLen);
@@ -3058,9 +3078,9 @@
}
-/* This does sort order and duplicate detection on a map. It works
- * on map labels of any type, even map labels that are maps (even
- * though QCBOR doesn't directly support decoding them).
+/* This does sort order and duplicate detection on a map. The and all
+ * it's members must be in preferred serialization so the comparisons
+ * work correctly.
*/
static QCBORError
QCBORDecode_Private_CheckMap(QCBORDecodeContext *pMe, const QCBORItem *pMapToCheck)
@@ -3074,10 +3094,9 @@
pMe->bAllowAllLabels = 1;
/* This loop runs over all the items in the map once, comparing
- * each adjacent pair for correct ordering. It also calls
- * CheckDup on each one which also runs over the remaining
- * items in the map checking for duplicates. So duplicate
- * checking runs in n^2.
+ * each adjacent pair for correct ordering. It also calls CheckDup
+ * on each one which also runs over the remaining items in the map
+ * checking for duplicates. So duplicate checking runs in n^2.
*/
offset2 = SIZE_MAX;
@@ -3096,8 +3115,12 @@
}
if(offset2 != SIZE_MAX) {
- /* Check that the labels are ordered. Check is not done the first
- * time through the loop when offset2 is unset. */
+ /* Check that the labels are ordered. Check is not done the
+ * first time through the loop when offset2 is unset. Since
+ * this does comparison of the items in encoded form they
+ * must be preferred serialization encoded. See RFC 8949
+ * 4.2.1.
+ */
if(UsefulInputBuf_Compare(&(pMe->InBuf), offset2, length2, offset1, length1) > 0) {
uErr = QCBOR_ERR_UNSORTED;
break;
@@ -3570,63 +3593,6 @@
-
-/**
- * @brief Consume an entire map or array including its contents.
- *
- * @param[in] pMe The decoder context.
- * @param[in] pItemToConsume The array/map whose contents are to be
- * consumed.
- * @param[out] puNextNestLevel The next nesting level after the item was
- * fully consumed.
- *
- * This may be called when @c pItemToConsume is not an array or
- * map. In that case, this is just a pass through for @c puNextNestLevel
- * since there is nothing to do.
- */
-static QCBORError
-QCBORDecode_Private_ConsumeItem(QCBORDecodeContext *pMe,
- const QCBORItem *pItemToConsume,
- bool *pbBreak,
- uint8_t *puNextNestLevel)
-{
- QCBORError uReturn;
- QCBORItem Item;
-
- /* If it is a map or array, this will tell if it is empty. */
- const bool bIsEmpty = (pItemToConsume->uNextNestLevel <= pItemToConsume->uNestingLevel);
-
- if(QCBORItem_IsMapOrArray(*pItemToConsume) && !bIsEmpty) {
- /* There is only real work to do for non-empty maps and arrays */
-
- /* This works for definite- and indefinite-length maps and
- * arrays by using the nesting level
- */
- do {
- uReturn = QCBORDecode_Private_GetNextMapOrArray(pMe, pbBreak, &Item, NULL);
- if(QCBORDecode_IsUnrecoverableError(uReturn) ||
- uReturn == QCBOR_ERR_NO_MORE_ITEMS) {
- goto Done;
- }
- } while(Item.uNextNestLevel >= pItemToConsume->uNextNestLevel);
-
- *puNextNestLevel = Item.uNextNestLevel;
-
- uReturn = QCBOR_SUCCESS;
-
- } else {
- /* pItemToConsume is not a map or array. Just pass the nesting
- * level through. */
- *puNextNestLevel = pItemToConsume->uNextNestLevel;
-
- uReturn = QCBOR_SUCCESS;
- }
-
-Done:
- return uReturn;
-}
-
-
/*
* Public function, see header qcbor/qcbor_decode.h file
*/
diff --git a/test/UsefulBuf_Tests.c b/test/UsefulBuf_Tests.c
index 08a3cac..6e47f3c 100644
--- a/test/UsefulBuf_Tests.c
+++ b/test/UsefulBuf_Tests.c
@@ -891,7 +891,6 @@
UsefulBufC CompCheck = UsefulBuf_FROM_SZ_LITERAL("abcd");
UsefulInputBuf_Init(&UIB, CompCheck);
- // TODO: fully test this (and check code coverage)
if(UsefulInputBuf_Compare(&UIB, 0, 2, 2, 2) >= 0) {
return "UB 1 compared greater than UB2";
}
diff --git a/test/qcbor_decode_tests.c b/test/qcbor_decode_tests.c
index ce78c12..9e4985f 100644
--- a/test/qcbor_decode_tests.c
+++ b/test/qcbor_decode_tests.c
@@ -2558,11 +2558,11 @@
}
#endif /* QCBOR_DISABLE_INDEFINITE_LENGTH_STRINGS */
- if(nIndex == 49) {
+ if(nIndex == 54) {
uCBORError = 9; /* For setting break points */
}
- if(strncmp("map with out-of-order labels that ", pF->szDescription, 15) == 0) {
+ if(strncmp("map with map label with non-preferred part", pF->szDescription, 25) == 0) {
uCBORError = 9; /* For setting break points */
}
@@ -10020,13 +10020,19 @@
{"map with out-of-order labels that are arrays",
QCBOR_DECODE_MODE_CDE,
{"\xA3\x83\x00\x01\x02\x61\x63\x83\x00\x01\x00\x61\x61\x83\x00\x01\x01\x61\x62", 19},
- QCBOR_ERR_UNSORTED
+ QCBOR_ERR_MAP_LABEL_TYPE
},
#if !defined(QCBOR_DISABLE_TAGS) && !defined(QCBOR_DISABLE_PREFERRED_FLOAT)
- { "unsorted map with labels of all types",
+ { "unsorted map with labels of all types including arrays and maps",
QCBOR_DECODE_MODE_CDE,
{"\xA8\x80\x07\xC1\x18\x58\x02\x64\x74\x65\x78\x74\x03\x01\x01\xA0\x04"
"\x42\x78\x78\x05\xF5\x06\xFB\x40\x21\x8A\x3D\x70\xA3\xD7\x0A\x07", 33},
+ QCBOR_ERR_MAP_LABEL_TYPE
+ },
+ { "unsorted map with labels of all non-aggregate types",
+ QCBOR_DECODE_MODE_CDE,
+ {"\xA6\xC1\x18\x58\x02\x64\x74\x65\x78\x74\x03\x01\x01"
+ "\x42\x78\x78\x05\xF5\x06\xFB\x40\x21\x8A\x3D\x70\xA3\xD7\x0A\x07", 29},
QCBOR_ERR_UNSORTED
},
{"map with out-of-order labels that have tags",
@@ -10045,16 +10051,34 @@
{"map with dup map labels",
QCBOR_DECODE_MODE_CDE,
{"\xA3\xA1\x03\x03\x61\x61\xA1\x02\x02\x61\x62\xA1\x03\x03\x61\x63", 16},
- QCBOR_ERR_DUPLICATE_LABEL
- }
-};
+ QCBOR_ERR_MAP_LABEL_TYPE
+ },
+
+ /* --- Maps with bad labels --- */
+ { "map with invalid label",
+ QCBOR_DECODE_MODE_CDE,
+ {"\xa1\x1c\x01", 3},
+ QCBOR_ERR_UNSUPPORTED
+ },
+
+ { "map with array label with invalid parts",
+ QCBOR_DECODE_MODE_CDE,
+ {"\xa1\x81\x1c\x01", 4},
+ QCBOR_ERR_MAP_LABEL_TYPE
+ },
+
+ { "map with map label with non-preferred part",
+ QCBOR_DECODE_MODE_CDE,
+ {"\xa1\xa1\x19\x00\x00\x01\x02", 7},
+ QCBOR_ERR_MAP_LABEL_TYPE
+ }};
static UsefulBufC CorrectlySorted[] = {
/* This one is correctly sorted, but is not correct preferred serialization. QCBOR checks
* the sort order of the map without checking the preferred serialization of the
* map items, so this test passes. */
- {"\xa4\x01\x61\x61\xf9\x3C\x00\x61\x62\xFA\x3F\x80\x00\x00\x61\x62\xFB\x3F\xF0\x00\x00\x00\x00\x00\x00\x61\x63", 27},
+ {"\xa4\x01\x61\x61\xf9\x3C\x00\x61\x62\xFA\x3F\x80\x00\x00\x61\x63\xFB\x3F\xF0\x00\x00\x00\x00\x00\x00\x61\x64", 27},
{"\xa3\x00\x61\x61\x01\x61\x62\xa3\x0c\x61\x78\x0b\x61\x79\x0a\x61\x7a\x61\x63", 19},
{"\xA3\xE0\x61\x61\xF5\x61\x62\xFB\x3F\xF1\x99\x99\x99\x99\x99\x9A\x61\x63", 18},
{"\xa2\x00\x00\x01\x01", 5},