diff --git a/include/grpc/census.h b/include/grpc/census.h index dfa3bd7e0df..442a754f0a4 100644 --- a/include/grpc/census.h +++ b/include/grpc/census.h @@ -80,18 +80,18 @@ CENSUSAPI int census_enabled(void); metrics will be recorded. Keys are unique within a context. */ typedef struct census_context census_context; -/* A tag is a key:value pair. The key is a non-empty, printable (UTF-8 - encoded), nil-terminated string. The value is a binary string, that may be - printable. There are limits on the sizes of both keys and values (see - CENSUS_MAX_TAG_KB_LEN definition below), and the number of tags that can be - propagated (CENSUS_MAX_PROPAGATED_TAGS). Users should also remember that - some systems may have limits on, e.g., the number of bytes that can be - transmitted as metadata, and that larger tags means more memory consumed - and time in processing. */ +/* A tag is a key:value pair. Both keys and values are nil-terminated strings, + containing printable ASCII characters (decimal 32-126). Keys must be at + least one character in length. Both keys and values can have at most + CENSUS_MAX_TAG_KB_LEN characters (including the terminating nil). The + maximum number of tags that can be propagated is + CENSUS_MAX_PROPAGATED_TAGS. Users should also remember that some systems + may have limits on, e.g., the number of bytes that can be transmitted as + metadata, and that larger tags means more memory consumed and time in + processing. */ typedef struct { const char *key; const char *value; - size_t value_len; uint8_t flags; } census_tag; @@ -103,28 +103,25 @@ typedef struct { /* Tag flags. */ #define CENSUS_TAG_PROPAGATE 1 /* Tag should be propagated over RPC */ #define CENSUS_TAG_STATS 2 /* Tag will be used for statistics aggregation */ -#define CENSUS_TAG_BINARY 4 /* Tag value is not printable */ -#define CENSUS_TAG_RESERVED 8 /* Reserved for internal use. */ -/* Flag values 8,16,32,64,128 are reserved for future/internal use. Clients +#define CENSUS_TAG_RESERVED 4 /* Reserved for internal use. */ +/* Flag values 4,8,16,32,64,128 are reserved for future/internal use. Clients should not use or rely on their values. */ #define CENSUS_TAG_IS_PROPAGATED(flags) (flags & CENSUS_TAG_PROPAGATE) #define CENSUS_TAG_IS_STATS(flags) (flags & CENSUS_TAG_STATS) -#define CENSUS_TAG_IS_BINARY(flags) (flags & CENSUS_TAG_BINARY) /* An instance of this structure is kept by every context, and records the basic information associated with the creation of that context. */ typedef struct { - int n_propagated_tags; /* number of propagated printable tags */ - int n_propagated_binary_tags; /* number of propagated binary tags */ - int n_local_tags; /* number of non-propagated (local) tags */ - int n_deleted_tags; /* number of tags that were deleted */ - int n_added_tags; /* number of tags that were added */ - int n_modified_tags; /* number of tags that were modified */ - int n_invalid_tags; /* number of tags with bad keys or values (e.g. - longer than CENSUS_MAX_TAG_KV_LEN) */ - int n_ignored_tags; /* number of tags ignored because of - CENSUS_MAX_PROPAGATED_TAGS limit. */ + int n_propagated_tags; /* number of propagated tags */ + int n_local_tags; /* number of non-propagated (local) tags */ + int n_deleted_tags; /* number of tags that were deleted */ + int n_added_tags; /* number of tags that were added */ + int n_modified_tags; /* number of tags that were modified */ + int n_invalid_tags; /* number of tags with bad keys or values (e.g. + longer than CENSUS_MAX_TAG_KV_LEN) */ + int n_ignored_tags; /* number of tags ignored because of + CENSUS_MAX_PROPAGATED_TAGS limit. */ } census_context_status; /* Create a new context, adding and removing tags from an existing context. @@ -132,10 +129,10 @@ typedef struct { to add as many tags in a single operation as is practical for the client. @param base Base context to build upon. Can be NULL. @param tags A set of tags to be added/changed/deleted. Tags with keys that - are in 'tags', but not 'base', are added to the tag set. Keys that are in + are in 'tags', but not 'base', are added to the context. Keys that are in both 'tags' and 'base' will have their value/flags modified. Tags with keys - in both, but with NULL or zero-length values, will be deleted from the tag - set. Tags with invalid (too long or short) keys or values will be ignored. + in both, but with NULL values, will be deleted from the context. Tags with + invalid (too long or short) keys or values will be ignored. If adding a tag will result in more than CENSUS_MAX_PROPAGATED_TAGS in either binary or non-binary tags, they will be ignored, as will deletions of tags that don't exist. @@ -185,32 +182,19 @@ CENSUSAPI int census_context_get_tag(const census_context *context, for use by RPC systems only, for purposes of transmitting/receiving contexts. */ -/* Encode a context into a buffer. The propagated tags are encoded into the - buffer in two regions: one for printable tags, and one for binary tags. +/* Encode a context into a buffer. @param context context to be encoded - @param buffer pointer to buffer. This address will be used to encode the - printable tags. + @param buffer buffer into which the context will be encoded. @param buf_size number of available bytes in buffer. - @param print_buf_size Will be set to the number of bytes consumed by - printable tags. - @param bin_buf_size Will be set to the number of bytes used to encode the - binary tags. - @return A pointer to the binary tag's encoded, or NULL if the buffer was - insufficiently large to hold the encoded tags. Thus, if successful, - printable tags are encoded into - [buffer, buffer + *print_buf_size) and binary tags into - [returned-ptr, returned-ptr + *bin_buf_size) (and the returned - pointer should be buffer + *print_buf_size) */ -CENSUSAPI char *census_context_encode(const census_context *context, - char *buffer, size_t buf_size, - size_t *print_buf_size, - size_t *bin_buf_size); - -/* Decode context buffers encoded with census_context_encode(). Returns NULL + @return The number of buffer bytes consumed for the encoded context, or + zero if the buffer was of insufficient size. */ +CENSUSAPI size_t census_context_encode(const census_context *context, + char *buffer, size_t buf_size); + +/* Decode context buffer encoded with census_context_encode(). Returns NULL if there is an error in parsing either buffer. */ -CENSUSAPI census_context *census_context_decode(const char *buffer, size_t size, - const char *bin_buffer, - size_t bin_size); +CENSUSAPI census_context *census_context_decode(const char *buffer, + size_t size); /* Distributed traces can have a number of options. */ enum census_trace_mask_values { diff --git a/src/core/census/context.c b/src/core/census/context.c index e60330de640..89b8ee0b399 100644 --- a/src/core/census/context.c +++ b/src/core/census/context.c @@ -60,10 +60,10 @@ // limit of 255 for both CENSUS_MAX_TAG_KV_LEN and CENSUS_MAX_PROPAGATED_TAGS. // * Keep all tag information (keys/values/flags) in a single memory buffer, // that can be directly copied to the wire. -// * Binary tags share the same structure as, but are encoded separately from, -// non-binary tags. This is primarily because non-binary tags are far more -// likely to be repeated across multiple RPC calls, so are more efficiently -// cached and compressed in any metadata schemes. + +// min and max valid chars in tag keys and values. All printable ASCII is OK. +#define MIN_VALID_TAG_CHAR 32 // ' ' +#define MAX_VALID_TAG_CHAR 126 // '~' // Structure representing a set of tags. Essentially a count of number of tags // present, and pointer to a chunk of memory that contains the per-tag details. @@ -77,7 +77,7 @@ struct tag_set { char *kvm; // key/value memory. Consists of repeated entries of: // Offset Size Description // 0 1 Key length, including trailing 0. (K) - // 1 1 Value length. (V) + // 1 1 Value length, including trailing 0 (V) // 2 1 Flags // 3 K Key bytes // 3 + K V Value bytes @@ -108,19 +108,36 @@ struct raw_tag { #define CENSUS_TAG_DELETED CENSUS_TAG_RESERVED #define CENSUS_TAG_IS_DELETED(flags) (flags & CENSUS_TAG_DELETED) -// Primary (external) representation of a context. Composed of 3 underlying -// tag_set structs, one for each of the binary/printable propagated tags, and -// one for everything else. This is to efficiently support tag -// encoding/decoding. +// Primary representation of a context. Composed of 2 underlying tag_set +// structs, one each for propagated and local (non-propagated) tags. This is +// to efficiently support tag encoding/decoding. +// TODO(aveitch): need to add tracing id's/structure. struct census_context { - struct tag_set tags[3]; + struct tag_set tags[2]; census_context_status status; }; // Indices into the tags member of census_context #define PROPAGATED_TAGS 0 -#define PROPAGATED_BINARY_TAGS 1 -#define LOCAL_TAGS 2 +#define LOCAL_TAGS 1 + +// Validate (check all characters are in range and size is less than limit) a +// key or value string. Returns 0 if the string is invalid, or the length +// (including terminator) if valid. +static size_t validate_tag(const char *kv) { + size_t len = 1; + char ch; + while ((ch = *kv++) != 0) { + if (ch < MIN_VALID_TAG_CHAR || ch > MAX_VALID_TAG_CHAR) { + return 0; + } + len++; + } + if (len > CENSUS_MAX_TAG_KV_LEN) { + return 0; + } + return len; +} // Extract a raw tag given a pointer (raw) to the tag header. Allow for some // extra bytes in the tag header (see encode/decode functions for usage: this @@ -166,9 +183,7 @@ static bool context_delete_tag(census_context *context, const census_tag *tag, size_t key_len) { return ( tag_set_delete_tag(&context->tags[LOCAL_TAGS], tag->key, key_len) || - tag_set_delete_tag(&context->tags[PROPAGATED_TAGS], tag->key, key_len) || - tag_set_delete_tag(&context->tags[PROPAGATED_BINARY_TAGS], tag->key, - key_len)); + tag_set_delete_tag(&context->tags[PROPAGATED_TAGS], tag->key, key_len)); } // Add a tag to a tag_set. Return true on success, false if the tag could @@ -176,11 +191,11 @@ static bool context_delete_tag(census_context *context, const census_tag *tag, // not be called if the tag may already exist (in a non-deleted state) in // the tag_set, as that would result in two tags with the same key. static bool tag_set_add_tag(struct tag_set *tags, const census_tag *tag, - size_t key_len) { + size_t key_len, size_t value_len) { if (tags->ntags == CENSUS_MAX_PROPAGATED_TAGS) { return false; } - const size_t tag_size = key_len + tag->value_len + TAG_HEADER_SIZE; + const size_t tag_size = key_len + value_len + TAG_HEADER_SIZE; if (tags->kvm_used + tag_size > tags->kvm_size) { // allocate new memory if needed tags->kvm_size += 2 * CENSUS_MAX_TAG_KV_LEN + TAG_HEADER_SIZE; @@ -191,13 +206,12 @@ static bool tag_set_add_tag(struct tag_set *tags, const census_tag *tag, } char *kvp = tags->kvm + tags->kvm_used; *kvp++ = (char)key_len; - *kvp++ = (char)tag->value_len; + *kvp++ = (char)value_len; // ensure reserved flags are not used. - *kvp++ = (char)(tag->flags & (CENSUS_TAG_PROPAGATE | CENSUS_TAG_STATS | - CENSUS_TAG_BINARY)); + *kvp++ = (char)(tag->flags & (CENSUS_TAG_PROPAGATE | CENSUS_TAG_STATS)); memcpy(kvp, tag->key, key_len); kvp += key_len; - memcpy(kvp, tag->value, tag->value_len); + memcpy(kvp, tag->value, value_len); tags->kvm_used += tag_size; tags->ntags++; tags->ntags_alloc++; @@ -207,30 +221,20 @@ static bool tag_set_add_tag(struct tag_set *tags, const census_tag *tag, // Add/modify/delete a tag to/in a context. Caller must validate that tag key // etc. are valid. static void context_modify_tag(census_context *context, const census_tag *tag, - size_t key_len) { + size_t key_len, size_t value_len) { // First delete the tag if it is already present. bool deleted = context_delete_tag(context, tag, key_len); - // Determine if we need to add it back. - bool call_add = tag->value != NULL && tag->value_len != 0; bool added = false; - if (call_add) { - if (CENSUS_TAG_IS_PROPAGATED(tag->flags)) { - if (CENSUS_TAG_IS_BINARY(tag->flags)) { - added = tag_set_add_tag(&context->tags[PROPAGATED_BINARY_TAGS], tag, - key_len); - } else { - added = tag_set_add_tag(&context->tags[PROPAGATED_TAGS], tag, key_len); - } - } else { - added = tag_set_add_tag(&context->tags[LOCAL_TAGS], tag, key_len); - } + if (CENSUS_TAG_IS_PROPAGATED(tag->flags)) { + added = tag_set_add_tag(&context->tags[PROPAGATED_TAGS], tag, key_len, + value_len); + } else { + added = + tag_set_add_tag(&context->tags[LOCAL_TAGS], tag, key_len, value_len); } + if (deleted) { - if (call_add) { - context->status.n_modified_tags++; - } else { - context->status.n_deleted_tags++; - } + context->status.n_modified_tags++; } else { if (added) { context->status.n_added_tags++; @@ -292,8 +296,6 @@ census_context *census_context_create(const census_context *base, memset(context, 0, sizeof(census_context)); } else { tag_set_copy(&context->tags[PROPAGATED_TAGS], &base->tags[PROPAGATED_TAGS]); - tag_set_copy(&context->tags[PROPAGATED_BINARY_TAGS], - &base->tags[PROPAGATED_BINARY_TAGS]); tag_set_copy(&context->tags[LOCAL_TAGS], &base->tags[LOCAL_TAGS]); memset(&context->status, 0, sizeof(context->status)); } @@ -301,22 +303,29 @@ census_context *census_context_create(const census_context *base, // the context to add/replace/delete as required. for (int i = 0; i < ntags; i++) { const census_tag *tag = &tags[i]; - size_t key_len = strlen(tag->key) + 1; - // ignore the tag if it is too long/short. - if (key_len != 1 && key_len <= CENSUS_MAX_TAG_KV_LEN && - tag->value_len <= CENSUS_MAX_TAG_KV_LEN) { - context_modify_tag(context, tag, key_len); - } else { + size_t key_len = validate_tag(tag->key); + // ignore the tag if it is invalid or too short. + if (key_len <= 1) { context->status.n_invalid_tags++; + } else { + if (tag->value != NULL) { + size_t value_len = validate_tag(tag->value); + if (value_len != 0) { + context_modify_tag(context, tag, key_len, value_len); + } else { + context->status.n_invalid_tags++; + } + } else { + if (context_delete_tag(context, tag, key_len)) { + context->status.n_deleted_tags++; + } + } } } // Remove any deleted tags, update status if needed, and return. tag_set_flatten(&context->tags[PROPAGATED_TAGS]); - tag_set_flatten(&context->tags[PROPAGATED_BINARY_TAGS]); tag_set_flatten(&context->tags[LOCAL_TAGS]); context->status.n_propagated_tags = context->tags[PROPAGATED_TAGS].ntags; - context->status.n_propagated_binary_tags = - context->tags[PROPAGATED_BINARY_TAGS].ntags; context->status.n_local_tags = context->tags[LOCAL_TAGS].ntags; if (status) { *status = &context->status; @@ -331,7 +340,6 @@ const census_context_status *census_context_get_status( void census_context_destroy(census_context *context) { gpr_free(context->tags[PROPAGATED_TAGS].kvm); - gpr_free(context->tags[PROPAGATED_BINARY_TAGS].kvm); gpr_free(context->tags[LOCAL_TAGS].kvm); gpr_free(context); } @@ -343,9 +351,6 @@ void census_context_initialize_iterator(const census_context *context, if (context->tags[PROPAGATED_TAGS].ntags != 0) { iterator->base = PROPAGATED_TAGS; iterator->kvm = context->tags[PROPAGATED_TAGS].kvm; - } else if (context->tags[PROPAGATED_BINARY_TAGS].ntags != 0) { - iterator->base = PROPAGATED_BINARY_TAGS; - iterator->kvm = context->tags[PROPAGATED_BINARY_TAGS].kvm; } else if (context->tags[LOCAL_TAGS].ntags != 0) { iterator->base = LOCAL_TAGS; iterator->kvm = context->tags[LOCAL_TAGS].kvm; @@ -363,7 +368,6 @@ int census_context_next_tag(census_context_iterator *iterator, iterator->kvm = decode_tag(&raw, iterator->kvm, 0); tag->key = raw.key; tag->value = raw.value; - tag->value_len = raw.value_len; tag->flags = raw.flags; if (++iterator->index == iterator->context->tags[iterator->base].ntags) { do { @@ -388,7 +392,6 @@ static bool tag_set_get_tag(const struct tag_set *tags, const char *key, if (key_len == raw.key_len && memcmp(raw.key, key, key_len) == 0) { tag->key = raw.key; tag->value = raw.value; - tag->value_len = raw.value_len; tag->flags = raw.flags; return true; } @@ -403,8 +406,6 @@ int census_context_get_tag(const census_context *context, const char *key, return 0; } if (tag_set_get_tag(&context->tags[PROPAGATED_TAGS], key, key_len, tag) || - tag_set_get_tag(&context->tags[PROPAGATED_BINARY_TAGS], key, key_len, - tag) || tag_set_get_tag(&context->tags[LOCAL_TAGS], key, key_len, tag)) { return 1; } @@ -447,21 +448,9 @@ static size_t tag_set_encode(const struct tag_set *tags, char *buffer, return ENCODED_HEADER_SIZE + tags->kvm_used; } -char *census_context_encode(const census_context *context, char *buffer, - size_t buf_size, size_t *print_buf_size, - size_t *bin_buf_size) { - *print_buf_size = - tag_set_encode(&context->tags[PROPAGATED_TAGS], buffer, buf_size); - if (*print_buf_size == 0) { - return NULL; - } - char *b_buffer = buffer + *print_buf_size; - *bin_buf_size = tag_set_encode(&context->tags[PROPAGATED_BINARY_TAGS], - b_buffer, buf_size - *print_buf_size); - if (*bin_buf_size == 0) { - return NULL; - } - return b_buffer; +size_t census_context_encode(const census_context *context, char *buffer, + size_t buf_size) { + return tag_set_encode(&context->tags[PROPAGATED_TAGS], buffer, buf_size); } // Decode a tag set. @@ -506,8 +495,7 @@ static void tag_set_decode(struct tag_set *tags, const char *buffer, } } -census_context *census_context_decode(const char *buffer, size_t size, - const char *bin_buffer, size_t bin_size) { +census_context *census_context_decode(const char *buffer, size_t size) { census_context *context = gpr_malloc(sizeof(census_context)); memset(&context->tags[LOCAL_TAGS], 0, sizeof(struct tag_set)); if (buffer == NULL) { @@ -515,16 +503,7 @@ census_context *census_context_decode(const char *buffer, size_t size, } else { tag_set_decode(&context->tags[PROPAGATED_TAGS], buffer, size); } - if (bin_buffer == NULL) { - memset(&context->tags[PROPAGATED_BINARY_TAGS], 0, sizeof(struct tag_set)); - } else { - tag_set_decode(&context->tags[PROPAGATED_BINARY_TAGS], bin_buffer, - bin_size); - } memset(&context->status, 0, sizeof(context->status)); context->status.n_propagated_tags = context->tags[PROPAGATED_TAGS].ntags; - context->status.n_propagated_binary_tags = - context->tags[PROPAGATED_BINARY_TAGS].ntags; - // TODO(aveitch): check that BINARY flag is correct for each type. return context; } diff --git a/src/python/grpcio/grpc/_cython/imports.generated.h b/src/python/grpcio/grpc/_cython/imports.generated.h index ca30742abcb..a395dce7d6a 100644 --- a/src/python/grpcio/grpc/_cython/imports.generated.h +++ b/src/python/grpcio/grpc/_cython/imports.generated.h @@ -91,10 +91,10 @@ extern census_context_next_tag_type census_context_next_tag_import; typedef int(*census_context_get_tag_type)(const census_context *context, const char *key, census_tag *tag); extern census_context_get_tag_type census_context_get_tag_import; #define census_context_get_tag census_context_get_tag_import -typedef char *(*census_context_encode_type)(const census_context *context, char *buffer, size_t buf_size, size_t *print_buf_size, size_t *bin_buf_size); +typedef size_t(*census_context_encode_type)(const census_context *context, char *buffer, size_t buf_size); extern census_context_encode_type census_context_encode_import; #define census_context_encode census_context_encode_import -typedef census_context *(*census_context_decode_type)(const char *buffer, size_t size, const char *bin_buffer, size_t bin_size); +typedef census_context *(*census_context_decode_type)(const char *buffer, size_t size); extern census_context_decode_type census_context_decode_import; #define census_context_decode census_context_decode_import typedef int(*census_trace_mask_type)(const census_context *context); diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.h b/src/ruby/ext/grpc/rb_grpc_imports.generated.h index b61c5282b6d..38aabfaca8f 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.h +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.h @@ -91,10 +91,10 @@ extern census_context_next_tag_type census_context_next_tag_import; typedef int(*census_context_get_tag_type)(const census_context *context, const char *key, census_tag *tag); extern census_context_get_tag_type census_context_get_tag_import; #define census_context_get_tag census_context_get_tag_import -typedef char *(*census_context_encode_type)(const census_context *context, char *buffer, size_t buf_size, size_t *print_buf_size, size_t *bin_buf_size); +typedef size_t(*census_context_encode_type)(const census_context *context, char *buffer, size_t buf_size); extern census_context_encode_type census_context_encode_import; #define census_context_encode census_context_encode_import -typedef census_context *(*census_context_decode_type)(const char *buffer, size_t size, const char *bin_buffer, size_t bin_size); +typedef census_context *(*census_context_decode_type)(const char *buffer, size_t size); extern census_context_decode_type census_context_decode_import; #define census_context_decode census_context_decode_import typedef int(*census_trace_mask_type)(const census_context *context); diff --git a/test/core/census/context_test.c b/test/core/census/context_test.c index 63e7103ddce..ad4c337465c 100644 --- a/test/core/census/context_test.c +++ b/test/core/census/context_test.c @@ -42,60 +42,48 @@ #include #include "test/core/util/test_config.h" -static uint8_t one_byte_val = 7; -static uint32_t four_byte_val = 0x12345678; -static uint64_t eight_byte_val = 0x1234567890abcdef; - -// A set of tags Used to create a basic context for testing. Each tag has a -// unique set of flags. Note that replace_add_delete_test() relies on specific -// offsets into this array - if you add or delete entries, you will also need -// to change the test. +// A set of tags Used to create a basic context for testing. Note that +// replace_add_delete_test() relies on specific offsets into this array - if +// you add or delete entries, you will also need to change the test. #define BASIC_TAG_COUNT 8 static census_tag basic_tags[BASIC_TAG_COUNT] = { - /* 0 */ {"key0", "printable", 10, 0}, - /* 1 */ {"k1", "a", 2, CENSUS_TAG_PROPAGATE}, - /* 2 */ {"k2", "longer printable string", 24, CENSUS_TAG_STATS}, - /* 3 */ {"key_three", (char *)&one_byte_val, 1, CENSUS_TAG_BINARY}, - /* 4 */ {"really_long_key_4", "random", 7, + /* 0 */ {"key0", "tag value", 0}, + /* 1 */ {"k1", "a", CENSUS_TAG_PROPAGATE}, + /* 2 */ {"k2", "a longer tag value supercalifragilisticexpialiadocious", + CENSUS_TAG_STATS}, + /* 3 */ {"key_three", "", 0}, + /* 4 */ {"a_really_really_really_really_long_key_4", "random", CENSUS_TAG_PROPAGATE | CENSUS_TAG_STATS}, - /* 5 */ {"k5", (char *)&four_byte_val, 4, - CENSUS_TAG_PROPAGATE | CENSUS_TAG_BINARY}, - /* 6 */ {"k6", (char *)&eight_byte_val, 8, - CENSUS_TAG_STATS | CENSUS_TAG_BINARY}, - /* 7 */ {"k7", (char *)&four_byte_val, 4, - CENSUS_TAG_PROPAGATE | CENSUS_TAG_STATS | CENSUS_TAG_BINARY}}; + /* 5 */ {"k5", "v5", CENSUS_TAG_PROPAGATE}, + /* 6 */ {"k6", "v6", CENSUS_TAG_STATS}, + /* 7 */ {"k7", "v7", CENSUS_TAG_PROPAGATE | CENSUS_TAG_STATS}}; // Set of tags used to modify the basic context. Note that // replace_add_delete_test() relies on specific offsets into this array - if // you add or delete entries, you will also need to change the test. Other // tests that rely on specific instances have XXX_XXX_OFFSET definitions (also // change the defines below if you add/delete entires). -#define MODIFY_TAG_COUNT 11 +#define MODIFY_TAG_COUNT 10 static census_tag modify_tags[MODIFY_TAG_COUNT] = { #define REPLACE_VALUE_OFFSET 0 - /* 0 */ {"key0", "replace printable", 18, 0}, // replaces tag value only + /* 0 */ {"key0", "replace key0", 0}, // replaces tag value only #define ADD_TAG_OFFSET 1 - /* 1 */ {"new_key", "xyzzy", 6, CENSUS_TAG_STATS}, // new tag + /* 1 */ {"new_key", "xyzzy", CENSUS_TAG_STATS}, // new tag #define DELETE_TAG_OFFSET 2 - /* 2 */ {"k5", NULL, 5, - 0}, // should delete tag, despite bogus value length - /* 3 */ {"k6", "foo", 0, 0}, // should delete tag, despite bogus value - /* 4 */ {"k6", "foo", 0, 0}, // try deleting already-deleted tag - /* 5 */ {"non-existent", NULL, 0, 0}, // another non-existent tag -#define REPLACE_FLAG_OFFSET 6 - /* 6 */ {"k1", "a", 2, 0}, // change flags only - /* 7 */ {"k7", "bar", 4, CENSUS_TAG_STATS}, // change flags and value - /* 8 */ {"k2", (char *)&eight_byte_val, 8, - CENSUS_TAG_BINARY | CENSUS_TAG_PROPAGATE}, // more flags change - // non-binary -> binary - /* 9 */ {"k6", "bar", 4, 0}, // add back tag, with different value - /* 10 */ {"foo", "bar", 4, CENSUS_TAG_PROPAGATE}, // another new tag + /* 2 */ {"k5", NULL, 0}, // should delete tag + /* 3 */ {"k5", NULL, 0}, // try deleting already-deleted tag + /* 4 */ {"non-existent", NULL, 0}, // delete non-existent tag +#define REPLACE_FLAG_OFFSET 5 + /* 5 */ {"k1", "a", 0}, // change flags only + /* 6 */ {"k7", "bar", CENSUS_TAG_STATS}, // change flags and value + /* 7 */ {"k2", "", CENSUS_TAG_PROPAGATE}, // more value and flags change + /* 8 */ {"k5", "bar", 0}, // add back tag, with different value + /* 9 */ {"foo", "bar", CENSUS_TAG_PROPAGATE}, // another new tag }; // Utility function to compare tags. Returns true if all fields match. static bool compare_tag(const census_tag *t1, const census_tag *t2) { - return (strcmp(t1->key, t2->key) == 0 && t1->value_len == t2->value_len && - memcmp(t1->value, t2->value, t1->value_len) == 0 && + return (strcmp(t1->key, t2->key) == 0 && strcmp(t1->value, t2->value) == 0 && t1->flags == t2->flags); } @@ -111,7 +99,7 @@ static void empty_test(void) { struct census_context *context = census_context_create(NULL, NULL, 0, NULL); GPR_ASSERT(context != NULL); const census_context_status *status = census_context_get_status(context); - census_context_status expected = {0, 0, 0, 0, 0, 0, 0, 0}; + census_context_status expected = {0, 0, 0, 0, 0, 0, 0}; GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); census_context_destroy(context); } @@ -121,7 +109,7 @@ static void basic_test(void) { const census_context_status *status; struct census_context *context = census_context_create(NULL, basic_tags, BASIC_TAG_COUNT, &status); - census_context_status expected = {2, 2, 4, 0, 8, 0, 0, 0}; + census_context_status expected = {4, 4, 0, 8, 0, 0, 0}; GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); census_context_iterator it; census_context_initialize_iterator(context, &it); @@ -161,15 +149,18 @@ static void invalid_test(void) { memset(key, 'k', 299); key[299] = 0; char value[300]; - memset(value, 'v', 300); - census_tag tag = {key, value, 3, CENSUS_TAG_BINARY}; + memset(value, 'v', 299); + value[299] = 0; + census_tag tag = {key, value, 0}; // long keys, short value. Key lengths (including terminator) should be // <= 255 (CENSUS_MAX_TAG_KV_LEN) + value[3] = 0; + GPR_ASSERT(strlen(value) == 3); GPR_ASSERT(strlen(key) == 299); const census_context_status *status; struct census_context *context = census_context_create(NULL, &tag, 1, &status); - census_context_status expected = {0, 0, 0, 0, 0, 0, 1, 0}; + census_context_status expected = {0, 0, 0, 0, 0, 1, 0}; GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); census_context_destroy(context); key[CENSUS_MAX_TAG_KV_LEN] = 0; @@ -180,24 +171,44 @@ static void invalid_test(void) { key[CENSUS_MAX_TAG_KV_LEN - 1] = 0; GPR_ASSERT(strlen(key) == CENSUS_MAX_TAG_KV_LEN - 1); context = census_context_create(NULL, &tag, 1, &status); - census_context_status expected2 = {0, 0, 1, 0, 1, 0, 0, 0}; + census_context_status expected2 = {0, 1, 0, 1, 0, 0, 0}; GPR_ASSERT(memcmp(status, &expected2, sizeof(expected2)) == 0); census_context_destroy(context); // now try with long values - tag.value_len = 300; + value[3] = 'v'; + GPR_ASSERT(strlen(value) == 299); context = census_context_create(NULL, &tag, 1, &status); GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); census_context_destroy(context); - tag.value_len = CENSUS_MAX_TAG_KV_LEN + 1; + value[CENSUS_MAX_TAG_KV_LEN] = 0; + GPR_ASSERT(strlen(value) == CENSUS_MAX_TAG_KV_LEN); context = census_context_create(NULL, &tag, 1, &status); GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); census_context_destroy(context); - tag.value_len = CENSUS_MAX_TAG_KV_LEN; + value[CENSUS_MAX_TAG_KV_LEN - 1] = 0; + GPR_ASSERT(strlen(value) == CENSUS_MAX_TAG_KV_LEN - 1); context = census_context_create(NULL, &tag, 1, &status); GPR_ASSERT(memcmp(status, &expected2, sizeof(expected2)) == 0); census_context_destroy(context); // 0 length key. key[0] = 0; + GPR_ASSERT(strlen(key) == 0); + context = census_context_create(NULL, &tag, 1, &status); + GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); + census_context_destroy(context); + // invalid key character + key[0] = 31; // 32 (' ') is the first valid character value + key[1] = 0; + GPR_ASSERT(strlen(key) == 1); + context = census_context_create(NULL, &tag, 1, &status); + GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); + census_context_destroy(context); + // invalid value character + key[0] = ' '; + value[5] = 127; // 127 (DEL) is ('~' + 1) + value[8] = 0; + GPR_ASSERT(strlen(key) == 1); + GPR_ASSERT(strlen(value) == 8); context = census_context_create(NULL, &tag, 1, &status); GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); census_context_destroy(context); @@ -210,7 +221,7 @@ static void copy_test(void) { const census_context_status *status; struct census_context *context2 = census_context_create(context, NULL, 0, &status); - census_context_status expected = {2, 2, 4, 0, 0, 0, 0, 0}; + census_context_status expected = {4, 4, 0, 0, 0, 0, 0}; GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); for (int i = 0; i < BASIC_TAG_COUNT; i++) { census_tag tag; @@ -228,7 +239,7 @@ static void replace_value_test(void) { const census_context_status *status; struct census_context *context2 = census_context_create( context, modify_tags + REPLACE_VALUE_OFFSET, 1, &status); - census_context_status expected = {2, 2, 4, 0, 0, 1, 0, 0}; + census_context_status expected = {4, 4, 0, 0, 1, 0, 0}; GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); census_tag tag; GPR_ASSERT(census_context_get_tag( @@ -245,7 +256,7 @@ static void replace_flags_test(void) { const census_context_status *status; struct census_context *context2 = census_context_create( context, modify_tags + REPLACE_FLAG_OFFSET, 1, &status); - census_context_status expected = {1, 2, 5, 0, 0, 1, 0, 0}; + census_context_status expected = {3, 5, 0, 0, 1, 0, 0}; GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); census_tag tag; GPR_ASSERT(census_context_get_tag( @@ -262,7 +273,7 @@ static void delete_tag_test(void) { const census_context_status *status; struct census_context *context2 = census_context_create( context, modify_tags + DELETE_TAG_OFFSET, 1, &status); - census_context_status expected = {2, 1, 4, 1, 0, 0, 0, 0}; + census_context_status expected = {3, 4, 1, 0, 0, 0, 0}; GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); census_tag tag; GPR_ASSERT(census_context_get_tag( @@ -278,7 +289,7 @@ static void add_tag_test(void) { const census_context_status *status; struct census_context *context2 = census_context_create(context, modify_tags + ADD_TAG_OFFSET, 1, &status); - census_context_status expected = {2, 2, 5, 0, 1, 0, 0, 0}; + census_context_status expected = {4, 5, 0, 1, 0, 0, 0}; GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); census_tag tag; GPR_ASSERT(census_context_get_tag(context2, modify_tags[ADD_TAG_OFFSET].key, @@ -295,24 +306,24 @@ static void replace_add_delete_test(void) { const census_context_status *status; struct census_context *context2 = census_context_create(context, modify_tags, MODIFY_TAG_COUNT, &status); - census_context_status expected = {2, 1, 6, 2, 3, 4, 0, 2}; + census_context_status expected = {3, 7, 1, 3, 4, 0, 0}; GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); // validate context contents. Use specific indices into the two arrays // holding tag values. GPR_ASSERT(validate_tag(context2, &basic_tags[3])); GPR_ASSERT(validate_tag(context2, &basic_tags[4])); + GPR_ASSERT(validate_tag(context2, &basic_tags[6])); GPR_ASSERT(validate_tag(context2, &modify_tags[0])); GPR_ASSERT(validate_tag(context2, &modify_tags[1])); + GPR_ASSERT(validate_tag(context2, &modify_tags[5])); GPR_ASSERT(validate_tag(context2, &modify_tags[6])); GPR_ASSERT(validate_tag(context2, &modify_tags[7])); GPR_ASSERT(validate_tag(context2, &modify_tags[8])); GPR_ASSERT(validate_tag(context2, &modify_tags[9])); - GPR_ASSERT(validate_tag(context2, &modify_tags[10])); GPR_ASSERT(!validate_tag(context2, &basic_tags[0])); GPR_ASSERT(!validate_tag(context2, &basic_tags[1])); GPR_ASSERT(!validate_tag(context2, &basic_tags[2])); GPR_ASSERT(!validate_tag(context2, &basic_tags[5])); - GPR_ASSERT(!validate_tag(context2, &basic_tags[6])); GPR_ASSERT(!validate_tag(context2, &basic_tags[7])); census_context_destroy(context); census_context_destroy(context2); @@ -325,21 +336,15 @@ static void encode_decode_test(void) { char buffer[BUF_SIZE]; struct census_context *context = census_context_create(NULL, basic_tags, BASIC_TAG_COUNT, NULL); - size_t print_bsize; - size_t bin_bsize; // Test with too small a buffer - GPR_ASSERT(census_context_encode(context, buffer, 2, &print_bsize, - &bin_bsize) == NULL); - char *b_buffer = census_context_encode(context, buffer, BUF_SIZE, - &print_bsize, &bin_bsize); - GPR_ASSERT(b_buffer != NULL && print_bsize > 0 && bin_bsize > 0 && - print_bsize + bin_bsize <= BUF_SIZE && - b_buffer == buffer + print_bsize); - census_context *context2 = - census_context_decode(buffer, print_bsize, b_buffer, bin_bsize); + GPR_ASSERT(census_context_encode(context, buffer, 2) == 0); + // Test with sufficient buffer + size_t buf_used = census_context_encode(context, buffer, BUF_SIZE); + GPR_ASSERT(buf_used != 0); + census_context *context2 = census_context_decode(buffer, buf_used); GPR_ASSERT(context2 != NULL); const census_context_status *status = census_context_get_status(context2); - census_context_status expected = {2, 2, 0, 0, 0, 0, 0, 0}; + census_context_status expected = {4, 0, 0, 0, 0, 0, 0}; GPR_ASSERT(memcmp(status, &expected, sizeof(expected)) == 0); for (int i = 0; i < BASIC_TAG_COUNT; i++) { census_tag tag;