Skip to content

Commit cc2bfbb

Browse files
Encoder: Change the way we reckon the number of items added
Instead of counting forward the number of items added, which meant we couldn't know in cbot_encoder_close_container() whether we had added enough, let's count backwards. The number is offset by 1 so we should be at 1 if we added exactly as many items as we had expected to. Zero means we added too many. Any other number means we added too few. Signed-off-by: Thiago Macieira <[email protected]>
1 parent d3c47d2 commit cc2bfbb

File tree

4 files changed

+30
-38
lines changed

4 files changed

+30
-38
lines changed

src/cbor.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ struct CborEncoder
209209
ptrdiff_t bytes_needed;
210210
} data;
211211
const uint8_t *end;
212-
size_t added;
212+
size_t remaining;
213213
int flags;
214214
};
215215
typedef struct CborEncoder CborEncoder;

src/cborencoder.c

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ void cbor_encoder_init(CborEncoder *encoder, uint8_t *buffer, size_t size, int f
204204
{
205205
encoder->data.ptr = buffer;
206206
encoder->end = buffer + size;
207-
encoder->added = 0;
207+
encoder->remaining = 2;
208208
encoder->flags = flags;
209209
}
210210

@@ -305,9 +305,15 @@ static inline CborError encode_number_no_update(CborEncoder *encoder, uint64_t u
305305
return append_to_buffer(encoder, bufstart, bufend - bufstart);
306306
}
307307

308+
static inline void saturated_decrement(CborEncoder *encoder)
309+
{
310+
if (encoder->remaining)
311+
--encoder->remaining;
312+
}
313+
308314
static inline CborError encode_number(CborEncoder *encoder, uint64_t ui, uint8_t shiftedMajorType)
309315
{
310-
++encoder->added;
316+
saturated_decrement(encoder);
311317
return encode_number_no_update(encoder, ui, shiftedMajorType);
312318
}
313319

@@ -389,7 +395,7 @@ CborError cbor_encode_floating_point(CborEncoder *encoder, CborType fpType, cons
389395
put32(buf + 1, *(const uint32_t*)value);
390396
else
391397
put16(buf + 1, *(const uint16_t*)value);
392-
++encoder->added;
398+
saturated_decrement(encoder);
393399
return append_to_buffer(encoder, buf, size + 1);
394400
}
395401

@@ -454,8 +460,8 @@ static CborError create_container(CborEncoder *encoder, CborEncoder *container,
454460
CborError err;
455461
container->data.ptr = encoder->data.ptr;
456462
container->end = encoder->end;
457-
++encoder->added;
458-
container->added = 0;
463+
saturated_decrement(encoder);
464+
container->remaining = length + 1; /* overflow ok on CborIndefiniteLength */
459465

460466
cbor_static_assert(((MapType << MajorTypeShift) & CborIteratorFlag_ContainerIsMap) == CborIteratorFlag_ContainerIsMap);
461467
cbor_static_assert(((ArrayType << MajorTypeShift) & CborIteratorFlag_ContainerIsMap) == 0);
@@ -465,6 +471,8 @@ static CborError create_container(CborEncoder *encoder, CborEncoder *container,
465471
container->flags |= CborIteratorFlag_UnknownLength;
466472
err = append_byte_to_buffer(container, shiftedMajorType + IndefiniteLength);
467473
} else {
474+
if (shiftedMajorType & CborIteratorFlag_ContainerIsMap)
475+
container->remaining += length;
468476
err = encode_number_no_update(container, length, shiftedMajorType);
469477
}
470478
return err;
@@ -520,8 +528,8 @@ CborError cbor_encoder_create_map(CborEncoder *encoder, CborEncoder *mapEncoder,
520528
* same as were passed to cbor_encoder_create_array() or
521529
* cbor_encoder_create_map().
522530
*
523-
* This function does not verify that the number of items (or pair of items, in
524-
* the case of a map) was correct. To execute that verification, call
531+
* Since version 0.5, this function verifies that the number of items (or pair
532+
* of items, in the case of a map) was correct. It is no longer needed to call
525533
* cbor_encoder_close_container_checked() instead.
526534
*
527535
* \sa cbor_encoder_create_array(), cbor_encoder_create_map()
@@ -535,6 +543,10 @@ CborError cbor_encoder_close_container(CborEncoder *encoder, const CborEncoder *
535543
encoder->end = containerEncoder->end;
536544
if (containerEncoder->flags & CborIteratorFlag_UnknownLength)
537545
return append_byte_to_buffer(encoder, BreakByte);
546+
547+
if (containerEncoder->remaining != 1)
548+
return containerEncoder->remaining == 0 ? CborErrorTooManyItems : CborErrorTooFewItems;
549+
538550
if (!encoder->end)
539551
return CborErrorOutOfMemory; /* keep the state */
540552
return CborNoError;

src/cborencoder_close_container_checked.c

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -29,49 +29,29 @@
2929
#endif
3030

3131
#include "cbor.h"
32-
#include "cborinternal_p.h"
33-
#include "compilersupport_p.h"
3432

3533
/**
3634
* \addtogroup CborEncoding
3735
* @{
3836
*/
3937

4038
/**
39+
* @deprecated
4140
*
4241
* Closes the CBOR container (array or map) provided by \a containerEncoder and
4342
* updates the CBOR stream provided by \a encoder. Both parameters must be the
4443
* same as were passed to cbor_encoder_create_array() or
4544
* cbor_encoder_create_map().
4645
*
47-
* Unlike cbor_encoder_close_container(), this function checks that the number
48-
* of items (or pair of items, in the case of a map) was correct. If the number
49-
* of items inserted does not match the length originally passed to
50-
* cbor_encoder_create_array() or cbor_encoder_create_map(), this function
51-
* returns either CborErrorTooFewItems or CborErrorTooManyItems.
46+
* Prior to version 0.5, cbor_encoder_close_container() did not check the
47+
* number of items added. Since that version, it does and now
48+
* cbor_encoder_close_container_checked() is no longer needed.
5249
*
5350
* \sa cbor_encoder_create_array(), cbor_encoder_create_map()
5451
*/
5552
CborError cbor_encoder_close_container_checked(CborEncoder *encoder, const CborEncoder *containerEncoder)
5653
{
57-
const uint8_t *ptr = encoder->data.ptr;
58-
CborError err = cbor_encoder_close_container(encoder, containerEncoder);
59-
if (containerEncoder->flags & CborIteratorFlag_UnknownLength || encoder->end == NULL)
60-
return err;
61-
62-
/* check what the original length was */
63-
uint64_t actually_added;
64-
err = _cbor_value_extract_number(&ptr, encoder->data.ptr, &actually_added);
65-
if (err)
66-
return err;
67-
68-
if (containerEncoder->flags & CborIteratorFlag_ContainerIsMap) {
69-
if (actually_added > SIZE_MAX / 2)
70-
return CborErrorDataTooLarge;
71-
actually_added *= 2;
72-
}
73-
return actually_added == containerEncoder->added ? CborNoError :
74-
actually_added < containerEncoder->added ? CborErrorTooManyItems : CborErrorTooFewItems;
54+
return cbor_encoder_close_container(encoder, containerEncoder);
7555
}
7656

7757
/** @} */

tests/encoder/tst_encoder.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ void compare(const QVariant &input, const QByteArray &output)
276276
cbor_encoder_init(&encoder, bufptr, buffer.length(), 0);
277277

278278
QCOMPARE(encodeVariant(&encoder, input), CborNoError);
279-
QCOMPARE(encoder.added, size_t(1));
279+
QCOMPARE(encoder.remaining, size_t(1));
280280
QCOMPARE(cbor_encoder_get_extra_bytes_needed(&encoder), size_t(0));
281281

282282
buffer.resize(int(cbor_encoder_get_buffer_size(&encoder, bufptr)));
@@ -642,7 +642,7 @@ void tst_Encoder::tooShortArrays()
642642
cbor_encoder_init(&encoder, reinterpret_cast<quint8 *>(buffer.data()), buffer.length(), 0);
643643
QCOMPARE(cbor_encoder_create_array(&encoder, &container, 2), CborNoError);
644644
QCOMPARE(encodeVariant(&container, input), CborNoError);
645-
QCOMPARE(container.added, size_t(1));
645+
QCOMPARE(container.remaining, size_t(2));
646646
QCOMPARE(cbor_encoder_close_container_checked(&encoder, &container), CborErrorTooFewItems);
647647
}
648648

@@ -656,7 +656,7 @@ void tst_Encoder::tooShortMaps()
656656
cbor_encoder_init(&encoder, reinterpret_cast<quint8 *>(buffer.data()), buffer.length(), 0);
657657
QCOMPARE(cbor_encoder_create_map(&encoder, &container, 2), CborNoError);
658658
QCOMPARE(encodeVariant(&container, input), CborNoError);
659-
QCOMPARE(container.added, size_t(1));
659+
QCOMPARE(container.remaining, size_t(4));
660660
QCOMPARE(cbor_encoder_close_container_checked(&encoder, &container), CborErrorTooFewItems);
661661
}
662662

@@ -671,7 +671,7 @@ void tst_Encoder::tooBigArrays()
671671
QCOMPARE(cbor_encoder_create_array(&encoder, &container, 1), CborNoError);
672672
QCOMPARE(encodeVariant(&container, input), CborNoError);
673673
QCOMPARE(encodeVariant(&container, input), CborNoError);
674-
QCOMPARE(container.added, size_t(2));
674+
QCOMPARE(container.remaining, size_t(0));
675675
QCOMPARE(cbor_encoder_close_container_checked(&encoder, &container), CborErrorTooManyItems);
676676
}
677677

@@ -687,7 +687,7 @@ void tst_Encoder::tooBigMaps()
687687
QCOMPARE(encodeVariant(&container, input), CborNoError);
688688
QCOMPARE(encodeVariant(&container, input), CborNoError);
689689
QCOMPARE(encodeVariant(&container, input), CborNoError);
690-
QCOMPARE(container.added, size_t(3));
690+
QCOMPARE(container.remaining, size_t(0));
691691
QCOMPARE(cbor_encoder_close_container_checked(&encoder, &container), CborErrorTooManyItems);
692692
}
693693

0 commit comments

Comments
 (0)