optimize out-of-place break error handling
diff --git a/inc/qcbor/qcbor_common.h b/inc/qcbor/qcbor_common.h
index 752d149..195b418 100644
--- a/inc/qcbor/qcbor_common.h
+++ b/inc/qcbor/qcbor_common.h
@@ -31,7 +31,6 @@
* IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
* ========================================================================= */
-
#ifndef qcbor_common_h
#define qcbor_common_h
diff --git a/inc/qcbor/qcbor_decode.h b/inc/qcbor/qcbor_decode.h
index 0a7eca0..98a76eb 100644
--- a/inc/qcbor/qcbor_decode.h
+++ b/inc/qcbor/qcbor_decode.h
@@ -510,10 +510,6 @@
} val;
- /** Union for the different label types selected based on @c uLabelType.
- * If compiles with QCBOR_DISABLE_NON_INTEGER_LABELS, int64 is the
- * only one used.
- */
union {
/** The label for @c uLabelType for @ref QCBOR_TYPE_INT64 */
int64_t int64;
diff --git a/src/qcbor_decode.c b/src/qcbor_decode.c
index b603f07..9c5d2d2 100644
--- a/src/qcbor_decode.c
+++ b/src/qcbor_decode.c
@@ -1425,11 +1425,38 @@
*/
QCBORError uReturn;
+ /* A note about string allocation -- Memory for strings is
+ * allocated either because 1) indefinte-length string chunks are
+ * being coalecsed or 2) caller has requested all strings be
+ * allocated. The first case is handed below here. The second case
+ * is handled in DecodeString if the bAllocate is true. That
+ * boolean originates here with pMe->bStringAllocateAll immediately
+ * below. That is, QCBOR_Private_DecodeAtomicDataItem() is called
+ * in two different contexts here 1) main-line processing which is
+ * where definite-length strings need to be allocated if
+ * bStringAllocateAll is true and 2) processing chunks of
+ * indefinite-lengths strings in in which case there must be no
+ * allocation.
+ */
+
+
uReturn = QCBOR_Private_DecodeAtomicDataItem(pMe, pMe->bStringAllocateAll, pDecodedItem);
if(uReturn != QCBOR_SUCCESS) {
goto Done;
}
+
+ /* This is where out-of-place break is detected for the whole
+ * decoding stack. Break is an error for everything that calls
+ * QCBORDecode_Private_GetNextFullString(), so the check is
+ * centralized here.
+ */
+ if(pDecodedItem->uDataType == QCBOR_TYPE_BREAK) {
+ uReturn = QCBOR_ERR_BAD_BREAK;
+ goto Done;
+ }
+
+
/* Skip out if not an indefinite-length string */
const uint8_t uStringType = pDecodedItem->uDataType;
if(uStringType != QCBOR_TYPE_BYTE_STRING &&
@@ -1453,10 +1480,10 @@
for(;;) {
/* Get QCBORItem for next chunk */
QCBORItem StringChunkItem;
- /* Pass a false to DecodeAtomicDataItem() because the
- * individual string chunks in an indefinite-length must not
- * be allocated. They are always copied into the allocated contiguous
- * buffer allocated here.
+ /* Pass false to DecodeAtomicDataItem() because the individual
+ * string chunks in an indefinite-length must not be
+ * allocated. They are always copied into the allocated
+ * contiguous buffer allocated here.
*/
uReturn = QCBOR_Private_DecodeAtomicDataItem(pMe, false, &StringChunkItem);
if(uReturn) {
@@ -1730,16 +1757,13 @@
* @retval QCBOR_ERR_ARRAY_DECODE_TOO_LONG Too many items in array.
* @retval QCBOR_ERR_MAP_LABEL_TYPE Map label not string or integer.
*
- * If the current nesting level is a map, then this
- * combines pairs of items into one data item with a label
- * and value.
+ * If the current nesting level is a map, then this combines pairs of
+ * items into one data item with a label and value.
*
- * This is passthrough if the current nesting level is
- * not a map.
+ * This is passthrough if the current nesting level is not a map.
*
- * This also implements maps-as-array mode where a map
- * is treated like an array to allow caller to do their
- * own label processing.
+ * This also implements maps-as-array mode where a map is treated like
+ * an array to allow caller to do their own label processing.
*/
static QCBORError
@@ -1754,29 +1778,30 @@
goto Done;
}
- if(pDecodedItem->uDataType == QCBOR_TYPE_BREAK) {
- /* Break can't be a map entry */
- goto Done;
- }
-
if(!DecodeNesting_IsCurrentTypeMap(&(pMe->nesting))) {
- /* Not decoding a map; nothing to do */
- /* This is where maps-as-arrays is effected too */
+ /* Not decoding a map. Nothing to do. */
+ /* When decoding maps-as-arrays, the type will be
+ * QCBOR_TYPE_MAP_AS_ARRAY and this function will exit
+ * here. This is now map processing for maps-as-arrays is not
+ * done. */
goto Done;
}
- /* Decoding a map entry, so the item so far is the label; must get value */
+ /* Decoding a map entry, so the item decoded above was the label */
LabelItem = *pDecodedItem;
+
+ /* Get the value of the map item */
uErr = QCBORDecode_Private_GetNextTagNumber(pMe, pDecodedItem);
if(QCBORDecode_IsUnrecoverableError(uErr)) {
goto Done;
}
+ /* Combine the label item and value item into one */
pDecodedItem->uLabelAlloc = LabelItem.uDataAlloc;
pDecodedItem->uLabelType = LabelItem.uDataType;
#ifndef QCBOR_DISABLE_NON_INTEGER_LABELS
- /* QCBOR_DECODE_MODE_MAP_STRINGS_ONLY was a bad idea. Maybe
+ /* QCBOR_DECODE_MODE_MAP_STRINGS_ONLY might have been a bad idea. Maybe
* get rid of it in QCBOR 2.0
*/
if(pMe->uDecodeMode == QCBOR_DECODE_MODE_MAP_STRINGS_ONLY &&
@@ -1784,7 +1809,7 @@
uErr = QCBOR_ERR_MAP_LABEL_TYPE;
goto Done;
}
-#endif /* !QCBOR_DISABLE_NON_INTEGER_LABELS */
+#endif /* ! QCBOR_DISABLE_NON_INTEGER_LABELS */
switch(LabelItem.uDataType) {
case QCBOR_TYPE_INT64:
@@ -1800,11 +1825,7 @@
case QCBOR_TYPE_BYTE_STRING:
pDecodedItem->label.string = LabelItem.val.string;
break;
-#endif /* !QCBOR_DISABLE_NON_INTEGER_LABELS */
-
- /* case QCBOR_TYPE_BREAK:
- uErr = 99;
- goto Done; */
+#endif /* ! QCBOR_DISABLE_NON_INTEGER_LABELS */
default:
uErr = QCBOR_ERR_MAP_LABEL_TYPE;
@@ -2040,14 +2061,6 @@
goto Done;
}
- /* Breaks ending arrays/maps are processed later in the call to
- * QCBORDecode_NestLevelAscender(). They should never show up here.
- */
- if(pDecodedItem->uDataType == QCBOR_TYPE_BREAK) {
- uReturn = QCBOR_ERR_BAD_BREAK;
- goto Done;
- }
-
/* Record the nesting level for this data item before processing
* any of decrementing and descending.
*/
diff --git a/test/not_well_formed_cbor.h b/test/not_well_formed_cbor.h
index e505887..5a213ef 100644
--- a/test/not_well_formed_cbor.h
+++ b/test/not_well_formed_cbor.h
@@ -159,9 +159,10 @@
{(uint8_t[]){0xa1, 0xff, 0x00}, 3},
// Array of length 1 with 2nd member value replaced by a break
{(uint8_t[]){0xa1, 0x00, 0xff}, 3},
- // Map of length 2 with 2nd member replaced by a break
- {(uint8_t[]){0xa2, 0x00, 0x00, 0xff}, 4},
-
+ // Map of length 2 with 2nd entry label replaced by a break
+ {(uint8_t[]){0xa2, 0x00, 0x00, 0xff, 0x00}, 5},
+ // Map of length 2 with 2nd entry value replaced by a break
+ {(uint8_t[]){0xa2, 0x00, 0x00, 0x00, 0xff}, 5},
// Breaks must not occur on their own out of an indefinite length
// data item
diff --git a/test/qcbor_decode_tests.c b/test/qcbor_decode_tests.c
index 5be3876..45e4eab 100644
--- a/test/qcbor_decode_tests.c
+++ b/test/qcbor_decode_tests.c
@@ -2420,10 +2420,6 @@
const struct someBinaryBytes *pBytes = &paNotWellFormedCBOR[nIterate];
const UsefulBufC Input = (UsefulBufC){pBytes->p, pBytes->n};
- if(nIterate == 86) {
- nIterate = 86;
- }
-
// Set up decoder context. String allocator needed for indefinite
// string test cases
QCBORDecodeContext DCtx;
@@ -2850,9 +2846,14 @@
{"\xa1\x00\xff", 3},
QCBOR_ERR_BAD_BREAK
},
- { "Map of length 2 with 2nd member replaced by a break",
+ { "Map of length 2 with 2nd entry label replaced by a break",
QCBOR_DECODE_MODE_NORMAL,
- {"\xa2\x00\x00\xff", 4},
+ {"\xa2\x00\x00\xff\x00", 5},
+ QCBOR_ERR_BAD_BREAK
+ },
+ { "Map of length 2 with 2nd entry value replaced by a break",
+ QCBOR_DECODE_MODE_NORMAL,
+ {"\xa2\x00\x00\x01\xff", 5},
QCBOR_ERR_BAD_BREAK
},