diff options
| -rw-r--r-- | Makefile | 2 | ||||
| -rw-r--r-- | src/upb_msg.c | 25 | ||||
| -rw-r--r-- | src/upb_parse.c | 15 | ||||
| -rw-r--r-- | src/upb_parse.h | 11 | ||||
| -rw-r--r-- | tests/tests.c | 21 |
5 files changed, 52 insertions, 22 deletions
@@ -28,7 +28,7 @@ CC=gcc CXX=g++ CFLAGS=-std=c99 INCLUDE=-Idescriptor -Isrc -Itests -I. -CPPFLAGS=-Wall -Wextra -g $(INCLUDE) $(strip $(shell test -f perf-cppflags && cat perf-cppflags)) +CPPFLAGS=-Wall -Wextra -Werror -g $(INCLUDE) $(strip $(shell test -f perf-cppflags && cat perf-cppflags)) LDLIBS=-lpthread LIBUPB=src/libupb.a diff --git a/src/upb_msg.c b/src/upb_msg.c index 7bacc89..ce443e1 100644 --- a/src/upb_msg.c +++ b/src/upb_msg.c @@ -136,9 +136,6 @@ struct upb_msg_parser { struct upb_msg_parser_frame stack[UPB_MAX_NESTING], *top; }; -void upb_msgparser_init(struct upb_msg_parser *p, - struct upb_msg *msg, bool byref); - /* Parses protocol buffer data out of data which has length of len. The data * need not be a complete protocol buffer. The number of bytes parsed is * returned in *read, and the next call to upb_msg_parse must supply data that @@ -255,17 +252,6 @@ static void end_cb(void *udata) /* Externally-visible functions for the msg parser. */ -upb_status_t upb_msg_parsestr(struct upb_msg *msg, void *buf, size_t len) -{ - struct upb_msg_parser mp; - upb_msgparser_init(&mp, msg, false); - size_t read; - upb_msg_clear(msg); - upb_status_t ret = upb_msg_parser_parse(&mp, buf, len, &read); - upb_msgparser_free(&mp); - return ret; -} - void upb_msgparser_init(struct upb_msg_parser *s, struct upb_msg *msg, bool byref) { s->s = upb_cbparser_new(); @@ -280,6 +266,17 @@ void upb_msgparser_free(struct upb_msg_parser *s) upb_cbparser_free(s->s); } +upb_status_t upb_msg_parsestr(struct upb_msg *msg, void *buf, size_t len) +{ + struct upb_msg_parser mp; + upb_msgparser_init(&mp, msg, false); + size_t read; + upb_msg_clear(msg); + upb_status_t ret = upb_msg_parser_parse(&mp, buf, len, &read); + upb_msgparser_free(&mp); + return ret; +} + upb_status_t upb_msg_parser_parse(struct upb_msg_parser *s, void *data, size_t len, size_t *read) { diff --git a/src/upb_parse.c b/src/upb_parse.c index bd511e4..68df31f 100644 --- a/src/upb_parse.c +++ b/src/upb_parse.c @@ -158,7 +158,10 @@ static upb_status_t push(struct upb_cbparser *s, uint8_t *start, if(s->start_cb) s->start_cb(s->udata, user_field_desc); - *submsg_end = start + (*s->top > 0 ? (*s->top - s->completed_offset) : 0); + if(*s->top > 0) + *submsg_end = start + (*s->top - s->completed_offset); + else + *submsg_end = (void*)UINTPTR_MAX; return UPB_STATUS_OK; } @@ -176,7 +179,7 @@ static void *pop(struct upb_cbparser *s, uint8_t *start) if(*s->top > 0) return (char*)start + (*s->top - s->completed_offset); else - return (char*)start; // group. + return (void*)UINTPTR_MAX; // group. } @@ -187,7 +190,7 @@ upb_status_t upb_cbparser_parse(struct upb_cbparser *s, void *_buf, size_t len, uint8_t *completed = buf; uint8_t *const start = buf; // ptr equivalent of s->completed_offset uint8_t *end = buf + len; - uint8_t *submsg_end = buf + (*s->top > 0 ? *s->top : 0); + uint8_t *submsg_end = *s->top > 0 ? buf + *s->top : (uint8_t*)UINTPTR_MAX; upb_status_t status = UPB_STATUS_OK; // Make local copies so optimizer knows they won't change. @@ -238,8 +241,12 @@ upb_status_t upb_cbparser_parse(struct upb_cbparser *s, void *_buf, size_t len, } } - while(buf == submsg_end) + while(buf >= submsg_end) { + if(buf > submsg_end) { + return UPB_ERROR_BAD_SUBMESSAGE_END; + } submsg_end = pop(s, start); + } // while(buf < s->packed_end) { TODO: packed arrays } completed = buf; } diff --git a/src/upb_parse.h b/src/upb_parse.h index df26089..1dc4b4c 100644 --- a/src/upb_parse.h +++ b/src/upb_parse.h @@ -27,9 +27,11 @@ extern "C" { // client should determine whether it wants to parse or skip the corresponding // value. If it wants to parse it, it must discover and return the correct // .proto type (the tag only contains the wire type) and check that the wire -// type is appropriate for the .proto type. To skip the value (which means -// skipping all submessages, in the case of a submessage), the callback should -// return zero. +// type is appropriate for the .proto type. Returning a type for which +// upb_check_type(tag->wire_type, type) == false invokes undefined behavior. +// +// To skip the value (which means skipping all submessages, in the case of a +// submessage), the callback should return zero. // // The client can store a void* in *user_field_desc; this will be passed to // the value callback or the string callback. @@ -67,6 +69,9 @@ void upb_cbparser_free(struct upb_cbparser *p); // Resets the internal state of an already-allocated parser. Parsers must be // reset before they can be used. A parser can be reset multiple times. udata // will be passed as the first argument to callbacks. +// +// tagcb must be set, but all other callbacks can be NULL, in which case they +// will just be skipped. void upb_cbparser_reset(struct upb_cbparser *p, void *udata, upb_tag_cb tagcb, upb_value_cb valuecb, diff --git a/tests/tests.c b/tests/tests.c index e625f8a..7f6f6a4 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -227,6 +227,26 @@ static void test_upb_context() { upb_context_unref(c); } + +static upb_field_type_t tag_cb(void *udata, struct upb_tag *tag, + void **user_field_desc) +{ + (void)udata; + (void)tag; + (void)user_field_desc; + return 0; +} + +static void test_cbparser() +{ + struct upb_cbparser *p = upb_cbparser_new(); + ASSERT(p); + upb_cbparser_reset(p, NULL, tag_cb, NULL, NULL, NULL, NULL); + size_t read; + ASSERT(upb_cbparser_parse(p, NULL, 0, &read) == UPB_STATUS_OK); + ASSERT(read == 0); +} + int main() { #define TEST(func) do { \ @@ -241,6 +261,7 @@ int main() TEST(test_skip_v_uint64_t); TEST(test_get_f_uint32_t); TEST(test_upb_context); + TEST(test_cbparser); printf("All tests passed (%d assertions).\n", num_assertions); return 0; } |