OpenBytes() is mostly working and somewhat tested
diff --git a/inc/qcbor/UsefulBuf.h b/inc/qcbor/UsefulBuf.h
index 01fc3ed..e7f4d4f 100644
--- a/inc/qcbor/UsefulBuf.h
+++ b/inc/qcbor/UsefulBuf.h
@@ -1,6 +1,6 @@
/*============================================================================
Copyright (c) 2016-2018, The Linux Foundation.
- Copyright (c) 2018-2021, Laurence Lundblade.
+ Copyright (c) 2018-2022, Laurence Lundblade.
Copyright (c) 2021, Arm Limited. All rights reserved.
Redistribution and use in source and binary forms, with or without
@@ -42,6 +42,7 @@
when who what, where, why
-------- ---- --------------------------------------------------
+ 4/11/2022 llundblade Add GetOutPlace and Advance to UsefulOutBuf.
9/21/2021 llundbla Clarify UsefulOutBuf size calculation mode
8/8/2021 dthaler/llundbla Work with C++ without compiler extensions
5/11/2021 llundblade Improve comments and comment formatting.
@@ -1287,9 +1288,9 @@
/**
- *@brief Returns pointer and length of the output buffer not yet used.
+ * @brief Returns pointer and length of the output buffer not yet used.
*
- * @param[in] pUOutBuf Pointer to the @ref UsefulOutBuf
+ * @param[in] pUOutBuf Pointer to the @ref UsefulOutBuf.
*
* @return pointer and length of output buffer not used.
*
@@ -1298,12 +1299,12 @@
* change the output buffer or state. It just returns a pointer
* and length of the bytes remaining.
*
- * This is useful to avoid having the bytes to be written all
- * in a contiguous buffer. It use can save memory. A good
+ * This is useful to avoid having the bytes to be added all
+ * in a contiguous buffer. Its use can save memory. A good
* example is in the COSE encrypt implementation where
* the output of the symmetric cipher can go directly
* into the output buffer, rather than having to go into
- * an intermediate.
+ * an intermediate buffer.
*
* See UsefulOutBuf_Advance() which is used to tell
* UsefulOutBuf how much was written.
@@ -1316,23 +1317,21 @@
/**
- *@brief Advance the amount output assuming it was written by the caller.
+ * @brief Advance the amount output assuming it was written by the caller.
*
- * @param[in] pUOutBuf Pointer to the @ref UsefulOutBuf
- * @param[in] uAmount The amount to advance
+ * @param[in] pUOutBuf Pointer to the @ref UsefulOutBuf.
+ * @param[in] uAmount The amount to advance.
*
* This advances the position in the output buffer
* by \c uAmount. This assumes that the
- * caller has written uAmount to the pointer obtained
+ * caller has written \c uAmount to the pointer obtained
* with UsefulOutBuf_GetOutPlace().
*
- * See UsefulOutBuf_GetOutPlace() which is used to
- * get the pointer to write to.
- *
* Warning: this bypasses the buffer safety provided by
* UsefulOutBuf!
*/
-void UsefulOutBuf_Advance(UsefulOutBuf *pUOutBuf, size_t uAmount);
+void
+UsefulOutBuf_Advance(UsefulOutBuf *pUOutBuf, size_t uAmount);
/**
@@ -2180,12 +2179,17 @@
return pMe->UB.ptr == NULL;
}
+
static inline UsefulBuf UsefulOutBuf_GetOutPlace(UsefulOutBuf *pUOutBuf)
{
UsefulBuf R;
- R.ptr = (uint8_t *)pUOutBuf->UB.ptr + pUOutBuf->data_len;
R.len = UsefulOutBuf_RoomLeft(pUOutBuf);
+ if(R.len > 0) {
+ R.ptr = (uint8_t *)pUOutBuf->UB.ptr + pUOutBuf->data_len;
+ } else {
+ R.ptr = NULL;
+ }
return R;
}
diff --git a/inc/qcbor/qcbor_common.h b/inc/qcbor/qcbor_common.h
index b9353b7..e1fe0a8 100644
--- a/inc/qcbor/qcbor_common.h
+++ b/inc/qcbor/qcbor_common.h
@@ -524,7 +524,11 @@
/** Floating point support is completely turned off, encoding/decoding
floating point numbers is not possible. */
- QCBOR_ERR_ALL_FLOAT_DISABLED = 46
+ QCBOR_ERR_ALL_FLOAT_DISABLED = 46,
+
+ /** During encode, the amount given to QCBOREncode_CloseBytes() was
+ past the end of what was returned by QCBOREncode_OpenBytes(). */
+ QCBOR_ERR_ADVANCE_TOO_FAR = 47
/* This is stored in uint8_t; never add values > 255 */
} QCBORError;
diff --git a/inc/qcbor/qcbor_encode.h b/inc/qcbor/qcbor_encode.h
index 5bba2ae..095cb96 100644
--- a/inc/qcbor/qcbor_encode.h
+++ b/inc/qcbor/qcbor_encode.h
@@ -861,6 +861,9 @@
the length in \c pPlace will be writing off the end of the
output buffer.
+ If there is no room in the output buffer NULLUsefulBuf will be returned and
+ there is no need to call QCBOREncode_CloseBytes().
+
The byte string must be closed by calling QCBOREncode_CloseBytes().
TODO: finish this documentation, write the implementation, test the code.
@@ -868,9 +871,11 @@
*/
void QCBOREncode_OpenBytes(QCBOREncodeContext *pCtx, UsefulBuf *pPlace);
-void QCBOREncode_OpenBytesInMapSZ(QCBOREncodeContext *pCtx, const char *szLabel, UsefulBuf *pPlace);
+static void
+QCBOREncode_OpenBytesInMapSZ(QCBOREncodeContext *pCtx, const char *szLabel, UsefulBuf *pPlace);
-void QCBOREncode_OpenBytesInMapN(QCBOREncodeContext *pCtx, int64_t nLabel, UsefulBuf *pPlace);
+static void
+QCBOREncode_OpenBytesInMapN(QCBOREncodeContext *pCtx, int64_t nLabel, UsefulBuf *pPlace);
/**
@@ -881,6 +886,9 @@
This inserts a CBOR header at the front of the byte string value to make
it a well-formed byte string.
+
+ If there was no call to QCBOREncode_OpenBytes() then @ref QCBOR_ERR_TOO_MANY_CLOSES is
+ set.
*/
void QCBOREncode_CloseBytes(QCBOREncodeContext *pCtx, size_t uAmount);
@@ -2416,6 +2424,20 @@
}
static inline void
+QCBOREncode_OpenBytesInMapSZ(QCBOREncodeContext *pMe, const char *szLabel, UsefulBuf *pPlace)
+{
+ QCBOREncode_AddSZString(pMe, szLabel);
+ QCBOREncode_OpenBytes(pMe, pPlace);
+}
+
+static inline void
+QCBOREncode_OpenBytesInMapN(QCBOREncodeContext *pMe, int64_t nLabel, UsefulBuf *pPlace)
+{
+ QCBOREncode_AddInt64(pMe, nLabel);
+ QCBOREncode_OpenBytes(pMe, pPlace);
+}
+
+static inline void
QCBOREncode_AddBytesLenOnly(QCBOREncodeContext *pMe, UsefulBufC Bytes)
{
QCBOREncode_AddBuffer(pMe, CBOR_MAJOR_NONE_TYPE_BSTR_LEN_ONLY, Bytes);
diff --git a/inc/qcbor/qcbor_private.h b/inc/qcbor/qcbor_private.h
index 9f189e6..9a2a720 100644
--- a/inc/qcbor/qcbor_private.h
+++ b/inc/qcbor/qcbor_private.h
@@ -320,6 +320,8 @@
#define CBOR_MAJOR_NONE_TYPE_RAW 9
#define CBOR_MAJOR_NONE_TAG_LABEL_REORDER 10
#define CBOR_MAJOR_NONE_TYPE_BSTR_LEN_ONLY 11
+#define CBOR_MAJOR_NONE_TYPE_OPEN_BSTR 12
+
// Add this to types to indicate they are to be encoded as indefinite lengths
#define QCBOR_INDEFINITE_LEN_TYPE_MODIFIER 0x80
diff --git a/src/UsefulBuf.c b/src/UsefulBuf.c
index 748bc56..f5149a5 100644
--- a/src/UsefulBuf.c
+++ b/src/UsefulBuf.c
@@ -1,6 +1,6 @@
/*==============================================================================
Copyright (c) 2016-2018, The Linux Foundation.
- Copyright (c) 2018-2021, Laurence Lundblade.
+ Copyright (c) 2018-2022, Laurence Lundblade.
Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
@@ -41,6 +41,7 @@
when who what, where, why
-------- ---- ---------------------------------------------------
+ 4/11/2022 llundblade Add GetOutPlace and Advance to UsefulOutBuf
3/6/2021 mcr/llundblade Fix warnings related to --Wcast-qual
01/28/2020 llundblade Refine integer signedness to quiet static analysis.
01/08/2020 llundblade Documentation corrections & improved code formatting.
@@ -309,49 +310,53 @@
void UsefulOutBuf_Advance(UsefulOutBuf *pMe, size_t uAmount)
{
/* This function is a trimmed down version of
- UsefulOutBuf_InsertUsefulBuf() */
+ * UsefulOutBuf_InsertUsefulBuf(). This could be combined with the
+ * code in UsefulOutBuf_InsertUsefulBuf(), but that would make
+ * UsefulOutBuf_InsertUsefulBuf() bigger and this will be very
+ * rarely used.
+ */
if(pMe->err) {
- /* Already in error state. */
- return;
- }
+ /* Already in error state. */
+ return;
+ }
- /* 0. Sanity check the UsefulOutBuf structure
- *
- * A "counter measure". If magic number is not the right number it
- * probably means me was not initialized or it was
- * corrupted. Attackers can defeat this, but it is a hurdle and
- * does good with very little code.
- */
- if(pMe->magic != USEFUL_OUT_BUF_MAGIC) {
- pMe->err = 1;
- return; /* Magic number is wrong due to uninitalization or corrption */
- }
+ /* 0. Sanity check the UsefulOutBuf structure
+ *
+ * A "counter measure". If magic number is not the right number it
+ * probably means me was not initialized or it was
+ * corrupted. Attackers can defeat this, but it is a hurdle and
+ * does good with very little code.
+ */
+ if(pMe->magic != USEFUL_OUT_BUF_MAGIC) {
+ pMe->err = 1;
+ return; /* Magic number is wrong due to uninitalization or corrption */
+ }
- /* Make sure valid data is less than buffer size. This would only
- * occur if there was corruption of me, but it is also part of the
- * checks to be sure there is no pointer arithmatic
- * under/overflow.
- */
- if(pMe->data_len > pMe->UB.len) { // Check #1
- pMe->err = 1;
- /* Offset of valid data is off the end of the UsefulOutBuf due
- * to uninitialization or corruption.
- */
- return;
- }
+ /* Make sure valid data is less than buffer size. This would only
+ * occur if there was corruption of me, but it is also part of the
+ * checks to be sure there is no pointer arithmatic
+ * under/overflow.
+ */
+ if(pMe->data_len > pMe->UB.len) { // Check #1
+ pMe->err = 1;
+ /* Offset of valid data is off the end of the UsefulOutBuf due
+ * to uninitialization or corruption.
+ */
+ return;
+ }
- /* 1. Will it fit?
- *
- * WillItFit() is the same as: NewData.len <= (me->UB.len -
- * me->data_len) Check #1 makes sure subtraction in RoomLeft will
- * not wrap around
- */
- if(! UsefulOutBuf_WillItFit(pMe, uAmount)) { /* Check #2 */
- /* The new data will not fit into the the buffer. */
- pMe->err = 1;
- return;
- }
+ /* 1. Will it fit?
+ *
+ * WillItFit() is the same as: NewData.len <= (me->UB.len -
+ * me->data_len) Check #1 makes sure subtraction in RoomLeft will
+ * not wrap around
+ */
+ if(! UsefulOutBuf_WillItFit(pMe, uAmount)) { /* Check #2 */
+ /* The new data will not fit into the the buffer. */
+ pMe->err = 1;
+ return;
+ }
pMe->data_len += uAmount;
}
diff --git a/src/qcbor_encode.c b/src/qcbor_encode.c
index 760de0e..6cfeb8b 100644
--- a/src/qcbor_encode.c
+++ b/src/qcbor_encode.c
@@ -1,6 +1,6 @@
/*==============================================================================
Copyright (c) 2016-2018, The Linux Foundation.
- Copyright (c) 2018-2021, Laurence Lundblade.
+ Copyright (c) 2018-2022, Laurence Lundblade.
Copyright (c) 2021, Arm Limited.
All rights reserved.
@@ -541,6 +541,9 @@
}
}
#endif /* QCBOR_DISABLE_ENCODE_USAGE_GUARDS */
+ if(uMajorType == CBOR_MAJOR_NONE_TYPE_OPEN_BSTR) {
+ uMajorType = CBOR_MAJOR_TYPE_BYTE_STRING;
+ }
/* A stack buffer large enough for a CBOR head */
UsefulBuf_MAKE_STACK_UB(pBufferForEncodedHead, QCBOR_HEAD_BUFFER_SIZE);
@@ -931,51 +934,30 @@
}
+/*
+ * Public function for opening a byte string. See qcbor/qcbor_encode.h
+ */
void QCBOREncode_OpenBytes(QCBOREncodeContext *pMe, UsefulBuf *pPlace)
{
- /* Add one item to the nesting level we are in for the new map or array */
- IncrementMapOrArrayCount(pMe);
-
- /* The offset where the length of an array or map will get written
- * is stored in a uint32_t, not a size_t to keep stack usage
- * smaller. This checks to be sure there is no wrap around when
- * recording the offset. Note that on 64-bit machines CBOR larger
- * than 4GB can be encoded as long as no array/map offsets occur
- * past the 4GB mark, but the public interface says that the
- * maximum is 4GB to keep the discussion simpler.
- */
- size_t uEndPosition = UsefulOutBuf_GetEndPosition(&(pMe->OutBuf));
-
- /* QCBOR_MAX_ARRAY_OFFSET is slightly less than UINT32_MAX so this
- * code can run on a 32-bit machine and tests can pass on a 32-bit
- * machine. If it was exactly UINT32_MAX, then this code would not
- * compile or run on a 32-bit machine and an #ifdef or some machine
- * size detection would be needed reducing portability.
- */
- if(uEndPosition >= QCBOR_MAX_ARRAY_OFFSET) {
- pMe->uError = QCBOR_ERR_BUFFER_TOO_LARGE;
-
- *pPlace = NULLUsefulBuf;
-
- } else {
- /* Increase nesting level because this is a map or array. Cast
- * from size_t to uin32_t is safe because of check above.
- */
- // TODO: proper type constant
- pMe->uError = Nesting_Increase(&(pMe->nesting), 200, (uint32_t)uEndPosition);
-
- *pPlace = UsefulOutBuf_Stuff(&(pMe->OutBuf));
+ *pPlace = UsefulOutBuf_GetOutPlace(&(pMe->OutBuf));
+ if(!UsefulBuf_IsNULL(*pPlace)){
+ QCBOREncode_OpenMapOrArray(pMe, CBOR_MAJOR_NONE_TYPE_OPEN_BSTR);
}
}
+/*
+ * Public function for closing a byte string. See qcbor/qcbor_encode.h
+ */
void QCBOREncode_CloseBytes(QCBOREncodeContext *pMe, size_t uAmount)
{
- UsefulOutBuf_StuffDone(&(pMe->OutBuf), uAmount);
- // TODO: sort out the major type
- InsertCBORHead(pMe,
- CBOR_MAJOR_TYPE_BYTE_STRING,
- uAmount);
+ UsefulOutBuf_Advance(&(pMe->OutBuf), uAmount);
+ if(UsefulOutBuf_GetError(&(pMe->OutBuf))) {
+ pMe->uError = QCBOR_ERR_ADVANCE_TOO_FAR;
+ return;
+ }
+
+ InsertCBORHead(pMe, CBOR_MAJOR_NONE_TYPE_OPEN_BSTR, uAmount);
}
diff --git a/test/UsefulBuf_Tests.c b/test/UsefulBuf_Tests.c
index b36c976..ccd07b8 100644
--- a/test/UsefulBuf_Tests.c
+++ b/test/UsefulBuf_Tests.c
@@ -1,6 +1,6 @@
/*==============================================================================
Copyright (c) 2016-2018, The Linux Foundation.
- Copyright (c) 2018-2021, Laurence Lundblade.
+ Copyright (c) 2018-2022, Laurence Lundblade.
Copyright (c) 2021, Arm Limited.
All rights reserved.
@@ -817,11 +817,12 @@
const char *UBAdvanceTest()
{
- UsefulOutBuf_MakeOnStack(UOB, 10);
+ #define ADVANCE_TEST_SIZE 10
+ UsefulOutBuf_MakeOnStack(UOB, ADVANCE_TEST_SIZE);
UsefulBuf Place = UsefulOutBuf_GetOutPlace(&UOB);
if(Place.len != 10) {
- return "Wrong Place";
+ return "GetOutPlace wrong size";
}
memset(Place.ptr, 'x', Place.len/2);
@@ -831,8 +832,8 @@
UsefulOutBuf_AppendByte(&UOB, 'y');
Place = UsefulOutBuf_GetOutPlace(&UOB);
- if(Place.len != 10/2 -1 ) {
- return "shit";
+ if(Place.len != ADVANCE_TEST_SIZE/2 -1 ) {
+ return "GetOutPlace wrong size 2";
}
memset(Place.ptr, 'z', Place.len);
@@ -843,7 +844,19 @@
UsefulBuf_Compare(O, UsefulBuf_FROM_SZ_LITERAL("xxxxxyzzzz"));
- // TODO: check for advancing too far, full buffer, ....
+ Place = UsefulOutBuf_GetOutPlace(&UOB);
+ if(Place.len != 0 || Place.ptr != NULL) {
+ return "GetOutPlace not null";
+ }
+
+ if(UsefulOutBuf_GetError(&UOB)) {
+ return "GetOutPlace error set";
+ }
+
+ UsefulOutBuf_Advance(&UOB, 1);
+ if(!UsefulOutBuf_GetError(&UOB)) {
+ return "Advance off end didn't set error";
+ }
return NULL;
}
diff --git a/test/qcbor_encode_tests.c b/test/qcbor_encode_tests.c
index d3ada13..196733f 100644
--- a/test/qcbor_encode_tests.c
+++ b/test/qcbor_encode_tests.c
@@ -2875,3 +2875,72 @@
return 0;
}
+
+
+
+
+static const uint8_t spExpectedBytes[] = {
+ 0x50, 0x78, 0x78, 0x78, 0x78, 0x78, 0x78, 0x78,
+ 0x78, 0x78, 0x78, 0x78, 0x78, 0x78, 0x78, 0x78,
+ 0x78
+};
+
+int32_t OpenCloseBytesTest(void)
+{
+ UsefulBuf_MAKE_STACK_UB( TestBuf, 20);
+ QCBOREncodeContext EC;
+ UsefulBuf Place;
+ UsefulBufC Encoded;
+ QCBORError uErr;
+
+ QCBOREncode_Init(&EC, TestBuf);
+
+ QCBOREncode_OpenBytes(&EC, &Place);
+ if(Place.ptr != TestBuf.ptr ||
+ Place.len != TestBuf.len) {
+ return 1;
+ }
+
+ Place.len -= 4;
+ UsefulBuf_Set(Place, 'x');
+
+ QCBOREncode_CloseBytes(&EC, Place.len);
+
+ QCBOREncode_Finish(&EC, &Encoded);
+
+ if(UsefulBuf_Compare(Encoded,
+ UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spExpectedBytes))) {
+ return 2;
+ }
+
+
+ QCBOREncode_Init(&EC, TestBuf);
+ QCBOREncode_AddSZString(&EC, "0123456789012345678");
+ QCBOREncode_OpenBytes(&EC, &Place);
+ if(Place.ptr != NULL ||
+ Place.len != 0) {
+ return 3;
+ }
+
+ QCBOREncode_Init(&EC, TestBuf);
+ QCBOREncode_AddSZString(&EC, "012345678");
+ QCBOREncode_CloseBytes(&EC, 1);
+ uErr = QCBOREncode_GetErrorState(&EC);
+ if(uErr != QCBOR_ERR_TOO_MANY_CLOSES) {
+ return 4;
+ }
+
+ QCBOREncode_Init(&EC, TestBuf);
+ QCBOREncode_AddSZString(&EC, "012345678901234567");
+ QCBOREncode_OpenBytes(&EC, &Place);
+ /* Don't bother to write any bytes*/
+ QCBOREncode_CloseBytes(&EC, Place.len+1);
+ uErr = QCBOREncode_GetErrorState(&EC);
+ // TODO: sort out this error -- issues with UOB internal error state
+ if(uErr != QCBOR_ERR_BUFFER_TOO_SMALL) {
+ return 5;
+ }
+
+
+ return 0;
+}
diff --git a/test/qcbor_encode_tests.h b/test/qcbor_encode_tests.h
index 69f694a..bac1085 100644
--- a/test/qcbor_encode_tests.h
+++ b/test/qcbor_encode_tests.h
@@ -1,6 +1,6 @@
/*==============================================================================
Copyright (c) 2016-2018, The Linux Foundation.
- Copyright (c) 2018-2021, Laurence Lundblade.
+ Copyright (c) 2018-2022, Laurence Lundblade.
All rights reserved.
Redistribution and use in source and binary forms, with or without
@@ -41,11 +41,6 @@
- All the functions in qcbor_encode.h are called once in the aggregation of all
the tests below.
- - All the types that are supported are given as input and parsed by these tests
-
- - There is some hostile input such as invalid lengths and CBOR too complex
- and types this parser doesn't handle
-
*/
@@ -141,10 +136,11 @@
*/
int32_t AllAddMethodsTest(void);
+
/*
The binary string wrapping of maps and arrays used by COSE
*/
-int32_t BstrWrapTest(void);
+int32_t BstrWrapTest(void);
/*
@@ -190,5 +186,11 @@
int32_t QCBORHeadTest(void);
+/* Fully test QCBOREncode_OpenBytes(), QCBOREncode_CloseBytes()
+ * and friends.
+ */
+int32_t OpenCloseBytesTest(void);
+
+
#endif /* defined(__QCBOR__qcbor_encode_tests__) */
diff --git a/test/run_tests.c b/test/run_tests.c
index 27da619..cb784a8 100644
--- a/test/run_tests.c
+++ b/test/run_tests.c
@@ -66,6 +66,7 @@
static test_entry s_tests[] = {
+ TEST_ENTRY(OpenCloseBytesTest),
TEST_ENTRY(EnterBstrTest),
TEST_ENTRY(IntegerConvertTest),
TEST_ENTRY(EnterMapTest),