Improve the detection of missing required fields.
Now the limit of tracked fields is configurable at compile-time using PB_MAX_REQUIRED_FIELDS. Added related test and updated documentation. Fixes issue #18.
This commit is contained in:
@@ -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.
|
||||
================== ==============================================================================================
|
||||
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. ::
|
||||
|
||||
6
pb.h
6
pb.h
@@ -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.
|
||||
|
||||
40
pb_decode.c
40
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;
|
||||
}
|
||||
|
||||
@@ -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
|
||||
@@ -62,5 +63,7 @@ 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'
|
||||
|
||||
138
tests/missing_fields.proto
Normal file
138
tests/missing_fields.proto
Normal file
@@ -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;
|
||||
}
|
||||
|
||||
49
tests/test_missing_fields.c
Normal file
49
tests/test_missing_fields.c
Normal file
@@ -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 */
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user