diff options
| -rw-r--r-- | docs/reference.rst | 13 | ||||
| -rw-r--r-- | pb.h | 6 | ||||
| -rw-r--r-- | pb_decode.c | 40 | ||||
| -rw-r--r-- | tests/Makefile | 7 | ||||
| -rw-r--r-- | tests/missing_fields.proto | 138 | ||||
| -rw-r--r-- | tests/test_missing_fields.c | 49 |
6 files changed, 229 insertions, 24 deletions
diff --git a/docs/reference.rst b/docs/reference.rst index f4957804..81251fe6 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -10,11 +10,12 @@ Compilation options =================== The following options can be specified using -D switch given to the C compiler: -================== ============================================================================================== -__BIG_ENDIAN__ Set this if your platform stores integers and floats in big-endian format. - Mixed-endian systems (different layout for ints and floats) are currently not supported. -NANOPB_INTERNALS Set this to expose the field encoder functions that are hidden since nanopb-0.1.3. -================== ============================================================================================== +============================ ============================================================================================== +__BIG_ENDIAN__ Set this if your platform stores integers and floats in big-endian format. + Mixed-endian systems (different layout for ints and floats) are currently not supported. +NANOPB_INTERNALS Set this to expose the field encoder functions that are hidden since nanopb-0.1.3. +PB_MAX_REQUIRED_FIELDS Maximum number of required fields to check for presence. Default value is 64. +============================ ============================================================================================== pb.h ==== @@ -310,8 +311,6 @@ In addition to EOF, the pb_decode implementation supports terminating a message For optional fields, this function applies the default value and sets *has_<field>* to false if the field is not present. -Because of memory concerns, the detection of missing required fields is not perfect if the structure contains more than 32 fields. - pb_decode_varint ---------------- Read and decode a varint_ encoded integer. :: @@ -22,6 +22,12 @@ #define UNUSED(x) (void)(x) #endif +/* Number of required fields to keep track of + * (change here or on compiler command line). */ +#ifndef PB_MAX_REQUIRED_FIELDS +#define PB_MAX_REQUIRED_FIELDS 64 +#endif + /* List of possible field types. These are used in the autogenerated code. * Least-significant 4 bits tell the scalar type * Most-significant 4 bits specify repeated/required/packed etc. diff --git a/pb_decode.c b/pb_decode.c index 50a11c40..458d5f1e 100644 --- a/pb_decode.c +++ b/pb_decode.c @@ -206,18 +206,20 @@ static bool checkreturn make_string_substream(pb_istream_t *stream, pb_istream_t /* Iterator for pb_field_t list */ typedef struct { - const pb_field_t *start; - const pb_field_t *current; - int field_index; - void *dest_struct; - void *pData; - void *pSize; + const pb_field_t *start; /* Start of the pb_field_t array */ + const pb_field_t *current; /* Current position of the iterator */ + int field_index; /* Zero-based index of the field. */ + int required_field_index; /* Zero-based index that counts only the required fields */ + void *dest_struct; /* Pointer to the destination structure to decode to */ + void *pData; /* Pointer where to store current field value */ + void *pSize; /* Pointer where to store the size of current array field */ } pb_field_iterator_t; static void pb_field_init(pb_field_iterator_t *iter, const pb_field_t *fields, void *dest_struct) { iter->start = iter->current = fields; iter->field_index = 0; + iter->required_field_index = 0; iter->pData = (char*)dest_struct + iter->current->data_offset; iter->pSize = (char*)iter->pData + iter->current->size_offset; iter->dest_struct = dest_struct; @@ -231,12 +233,16 @@ static bool pb_field_next(pb_field_iterator_t *iter) if (PB_HTYPE(iter->current->type) == PB_HTYPE_ARRAY) prev_size *= iter->current->array_size; + if (PB_HTYPE(iter->current->type) == PB_HTYPE_REQUIRED) + iter->required_field_index++; + iter->current++; iter->field_index++; if (iter->current->tag == 0) { iter->current = iter->start; iter->field_index = 0; + iter->required_field_index = 0; iter->pData = iter->dest_struct; prev_size = 0; notwrapped = false; @@ -403,9 +409,8 @@ static void pb_message_set_to_defaults(const pb_field_t fields[], void *dest_str bool checkreturn pb_decode(pb_istream_t *stream, const pb_field_t fields[], void *dest_struct) { - uint32_t fields_seen = 0; /* Used to check for required fields */ + uint8_t fields_seen[(PB_MAX_REQUIRED_FIELDS + 7) / 8] = {}; /* Used to check for required fields */ pb_field_iterator_t iter; - int i; pb_message_set_to_defaults(fields, dest_struct); @@ -433,21 +438,26 @@ bool checkreturn pb_decode(pb_istream_t *stream, const pb_field_t fields[], void continue; } - fields_seen |= 1 << (iter.field_index & 31); + if (PB_HTYPE(iter.current->type) == PB_HTYPE_REQUIRED + && iter.required_field_index < PB_MAX_REQUIRED_FIELDS) + { + fields_seen[iter.required_field_index >> 3] |= 1 << (iter.required_field_index & 7); + } if (!decode_field(stream, wire_type, &iter)) return false; } - /* Check that all required fields (mod 31) were present. */ - for (i = 0; fields[i].tag != 0; i++) - { - if (PB_HTYPE(fields[i].type) == PB_HTYPE_REQUIRED && - !(fields_seen & (1 << (i & 31)))) + /* Check that all required fields were present. */ + pb_field_init(&iter, fields, dest_struct); + do { + if (PB_HTYPE(iter.current->type) == PB_HTYPE_REQUIRED && + iter.required_field_index < PB_MAX_REQUIRED_FIELDS && + !(fields_seen[iter.required_field_index >> 3] & (1 << (iter.required_field_index & 7)))) { return false; } - } + } while (pb_field_next(&iter)); return true; } diff --git a/tests/Makefile b/tests/Makefile index 8e2df140..24e9ba62 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -1,6 +1,6 @@ CFLAGS=-ansi -Wall -Werror -I .. -g -O0 --coverage LDFLAGS=--coverage -DEPS=../pb_decode.h ../pb_encode.h ../pb.h person.pb.h callbacks.pb.h unittests.h unittestproto.pb.h alltypes.pb.h +DEPS=../pb_decode.h ../pb_encode.h ../pb.h person.pb.h callbacks.pb.h unittests.h unittestproto.pb.h alltypes.pb.h missing_fields.pb.h TESTS=test_decode1 test_encode1 decode_unittests encode_unittests all: breakpoints $(TESTS) run_unittests @@ -25,6 +25,7 @@ test_encode2: test_encode2.o pb_encode.o person.pb.o test_encode3: test_encode3.o pb_encode.o alltypes.pb.o test_decode_callbacks: test_decode_callbacks.o pb_decode.o callbacks.pb.o test_encode_callbacks: test_encode_callbacks.o pb_encode.o callbacks.pb.o +test_missing_fields: test_missing_fields.o pb_encode.o pb_decode.o missing_fields.pb.o decode_unittests: decode_unittests.o pb_decode.o unittestproto.pb.o encode_unittests: encode_unittests.o pb_encode.o unittestproto.pb.o @@ -41,7 +42,7 @@ coverage: run_unittests gcov pb_encode.gcda gcov pb_decode.gcda -run_unittests: decode_unittests encode_unittests test_encode1 test_encode2 test_encode3 test_decode1 test_decode2 test_decode3 test_encode_callbacks test_decode_callbacks +run_unittests: decode_unittests encode_unittests test_encode1 test_encode2 test_encode3 test_decode1 test_decode2 test_decode3 test_encode_callbacks test_decode_callbacks test_missing_fields rm -f *.gcda ./decode_unittests > /dev/null @@ -61,6 +62,8 @@ run_unittests: decode_unittests encode_unittests test_encode1 test_encode2 test_ ./test_encode3 | ./test_decode3 ./test_encode3 | protoc --decode=AllTypes -I. -I../generator -I/usr/include alltypes.proto >/dev/null + + ./test_missing_fields run_fuzztest: test_decode2 bash -c 'I=1; while true; do cat /dev/urandom | ./test_decode2 > /dev/null; I=$$(($$I+1)); echo -en "\r$$I"; done' diff --git a/tests/missing_fields.proto b/tests/missing_fields.proto new file mode 100644 index 00000000..cbb23ba1 --- /dev/null +++ b/tests/missing_fields.proto @@ -0,0 +1,138 @@ +/* Test for one missing field among many */ + +message AllFields +{ + required int32 field1 = 1; + required int32 field2 = 2; + required int32 field3 = 3; + required int32 field4 = 4; + required int32 field5 = 5; + required int32 field6 = 6; + required int32 field7 = 7; + required int32 field8 = 8; + required int32 field9 = 9; + required int32 field10 = 10; + required int32 field11 = 11; + required int32 field12 = 12; + required int32 field13 = 13; + required int32 field14 = 14; + required int32 field15 = 15; + required int32 field16 = 16; + required int32 field17 = 17; + required int32 field18 = 18; + required int32 field19 = 19; + required int32 field20 = 20; + required int32 field21 = 21; + required int32 field22 = 22; + required int32 field23 = 23; + required int32 field24 = 24; + required int32 field25 = 25; + required int32 field26 = 26; + required int32 field27 = 27; + required int32 field28 = 28; + required int32 field29 = 29; + required int32 field30 = 30; + required int32 field31 = 31; + required int32 field32 = 32; + required int32 field33 = 33; + required int32 field34 = 34; + required int32 field35 = 35; + required int32 field36 = 36; + required int32 field37 = 37; + required int32 field38 = 38; + required int32 field39 = 39; + required int32 field40 = 40; + required int32 field41 = 41; + required int32 field42 = 42; + required int32 field43 = 43; + required int32 field44 = 44; + required int32 field45 = 45; + required int32 field46 = 46; + required int32 field47 = 47; + required int32 field48 = 48; + required int32 field49 = 49; + required int32 field50 = 50; + required int32 field51 = 51; + required int32 field52 = 52; + required int32 field53 = 53; + required int32 field54 = 54; + required int32 field55 = 55; + required int32 field56 = 56; + required int32 field57 = 57; + required int32 field58 = 58; + required int32 field59 = 59; + required int32 field60 = 60; + required int32 field61 = 61; + required int32 field62 = 62; + required int32 field63 = 63; + required int32 field64 = 64; +} + +message MissingField +{ + required int32 field1 = 1; + required int32 field2 = 2; + required int32 field3 = 3; + required int32 field4 = 4; + required int32 field5 = 5; + required int32 field6 = 6; + required int32 field7 = 7; + required int32 field8 = 8; + required int32 field9 = 9; + required int32 field10 = 10; + required int32 field11 = 11; + required int32 field12 = 12; + required int32 field13 = 13; + required int32 field14 = 14; + required int32 field15 = 15; + required int32 field16 = 16; + required int32 field17 = 17; + required int32 field18 = 18; + required int32 field19 = 19; + required int32 field20 = 20; + required int32 field21 = 21; + required int32 field22 = 22; + required int32 field23 = 23; + required int32 field24 = 24; + required int32 field25 = 25; + required int32 field26 = 26; + required int32 field27 = 27; + required int32 field28 = 28; + required int32 field29 = 29; + required int32 field30 = 30; + required int32 field31 = 31; + required int32 field32 = 32; + required int32 field33 = 33; + required int32 field34 = 34; + required int32 field35 = 35; + required int32 field36 = 36; + required int32 field37 = 37; + required int32 field38 = 38; + required int32 field39 = 39; + required int32 field40 = 40; + required int32 field41 = 41; + required int32 field42 = 42; + required int32 field43 = 43; + required int32 field44 = 44; + required int32 field45 = 45; + required int32 field46 = 46; + required int32 field47 = 47; + required int32 field48 = 48; + required int32 field49 = 49; + required int32 field50 = 50; + required int32 field51 = 51; + required int32 field52 = 52; + required int32 field53 = 53; + required int32 field54 = 54; + required int32 field55 = 55; + required int32 field56 = 56; + required int32 field57 = 57; + required int32 field58 = 58; + required int32 field59 = 59; + required int32 field60 = 60; + required int32 field61 = 61; + required int32 field62 = 62; +/* required int32 field63 = 63; */ + required int32 field64 = 64; +} + diff --git a/tests/test_missing_fields.c b/tests/test_missing_fields.c new file mode 100644 index 00000000..46cd7d96 --- /dev/null +++ b/tests/test_missing_fields.c @@ -0,0 +1,49 @@ +/* Checks that missing required fields are detected properly */ + +#include <stdio.h> +#include <pb_encode.h> +#include <pb_decode.h> +#include "missing_fields.pb.h" + +int main() +{ + uint8_t buffer[512] = {}; + + /* Create a message with one missing field */ + { + MissingField msg = {}; + pb_ostream_t stream = pb_ostream_from_buffer(buffer, sizeof(buffer)); + if (!pb_encode(&stream, MissingField_fields, &msg)) + { + printf("Encode failed.\n"); + return 1; + } + } + + /* Test that it decodes properly if we don't require that field */ + { + MissingField msg = {}; + pb_istream_t stream = pb_istream_from_buffer(buffer, sizeof(buffer)); + + if (!pb_decode(&stream, MissingField_fields, &msg)) + { + printf("Decode failed.\n"); + return 2; + } + } + + /* Test that it does *not* decode properly if we require the field */ + { + AllFields msg = {}; + pb_istream_t stream = pb_istream_from_buffer(buffer, sizeof(buffer)); + + if (pb_decode(&stream, AllFields_fields, &msg)) + { + printf("Decode didn't detect missing field.\n"); + return 3; + } + } + + return 0; /* All ok */ +} + |