From 10c54e44ba5224b501705e8568b8014c4564c884 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 23 Mar 2013 19:31:22 +0900 Subject: [PATCH] Don't pack multiple empty header values in one header field SPDY spec does not allow multiple empty header values in one header field. This change makes out-going framer ignore such empty header value if there is non-empty header value with the same name. --- lib/spdylay_frame.c | 54 ++++++++++++++----- tests/main.c | 4 ++ tests/spdylay_frame_test.c | 104 +++++++++++++++++++++++++++++++++++++ tests/spdylay_frame_test.h | 2 + 4 files changed, 150 insertions(+), 14 deletions(-) diff --git a/lib/spdylay_frame.c b/lib/spdylay_frame.c index c0204eb6..dfd5b10c 100644 --- a/lib/spdylay_frame.c +++ b/lib/spdylay_frame.c @@ -250,17 +250,28 @@ size_t spdylay_frame_count_nv_space(char **nv, size_t len_size) int i; const char *prev = ""; size_t prevlen = 0; + size_t prevvallen = 0; for(i = 0; nv[i]; i += 2) { const char *key = nv[i]; const char *val = nv[i+1]; size_t keylen = strlen(key); size_t vallen = strlen(val); if(prevlen == keylen && memcmp(prev, key, keylen) == 0) { - /* Join previous value, with NULL character */ - sum += vallen+1; + if(vallen) { + if(prevvallen) { + /* Join previous value, with NULL character */ + sum += vallen+1; + prevvallen = vallen; + } else { + /* Previous value is empty. In this case, drop the + previous. */ + sum += vallen; + } + } } else { prev = key; prevlen = keylen; + prevvallen = vallen; /* SPDY NV header does not include terminating NULL byte */ sum += keylen+vallen+len_size*2; } @@ -273,28 +284,43 @@ ssize_t spdylay_frame_pack_nv(uint8_t *buf, char **nv, size_t len_size) int i; uint8_t *bufp = buf+len_size; uint32_t num_nv = 0; - /* TODO Join values with same keys, using '\0' as a delimiter */ const char *prev = ""; - uint8_t *prev_vallen_buf = NULL; - uint32_t prev_vallen = 0; + uint8_t *cur_vallen_buf = NULL; + uint32_t cur_vallen = 0; + size_t prevkeylen = 0; + size_t prevvallen = 0; for(i = 0; nv[i]; i += 2) { const char *key = nv[i]; const char *val = nv[i+1]; size_t keylen = strlen(key); size_t vallen = strlen(val); - if(strcmp(prev, key) == 0) { - prev_vallen += vallen+1; - spdylay_frame_put_nv_len(prev_vallen_buf, prev_vallen, len_size); - *bufp = '\0'; - ++bufp; - memcpy(bufp, val, vallen); - bufp += vallen; + if(prevkeylen == keylen && memcmp(prev, key, keylen) == 0) { + if(vallen) { + if(prevvallen) { + /* Join previous value, with NULL character */ + cur_vallen += vallen+1; + spdylay_frame_put_nv_len(cur_vallen_buf, cur_vallen, len_size); + *bufp = '\0'; + ++bufp; + memcpy(bufp, val, vallen); + bufp += vallen; + } else { + /* Previous value is empty. In this case, drop the + previous. */ + cur_vallen += vallen; + spdylay_frame_put_nv_len(cur_vallen_buf, cur_vallen, len_size); + memcpy(bufp, val, vallen); + bufp += vallen; + } + } } else { ++num_nv; bufp = spdylay_pack_str(bufp, key, keylen, len_size); prev = key; - prev_vallen_buf = bufp; - prev_vallen = vallen; + cur_vallen_buf = bufp; + cur_vallen = vallen; + prevkeylen = keylen; + prevvallen = vallen; bufp = spdylay_pack_str(bufp, val, vallen, len_size); } } diff --git a/tests/main.c b/tests/main.c index d0613d57..e2bfc640 100644 --- a/tests/main.c +++ b/tests/main.c @@ -210,6 +210,10 @@ int main(int argc, char* argv[]) test_spdylay_frame_nv_downcase) || !CU_add_test(pSuite, "frame_pack_nv_duplicate_keys", test_spdylay_frame_pack_nv_duplicate_keys) || + !CU_add_test(pSuite, "frame_pack_nv_empty_value_spdy2", + test_spdylay_frame_pack_nv_empty_value_spdy2) || + !CU_add_test(pSuite, "frame_pack_nv_empty_value_spdy3", + test_spdylay_frame_pack_nv_empty_value_spdy3) || !CU_add_test(pSuite, "frame_nv_2to3", test_spdylay_frame_nv_2to3) || !CU_add_test(pSuite, "frame_nv_3to2", test_spdylay_frame_nv_3to2) || !CU_add_test(pSuite, "frame_unpack_nv_check_name_spdy2", diff --git a/tests/spdylay_frame_test.c b/tests/spdylay_frame_test.c index 56b2d808..343df38d 100644 --- a/tests/spdylay_frame_test.c +++ b/tests/spdylay_frame_test.c @@ -24,12 +24,29 @@ */ #include "spdylay_frame_test.h" +#include + #include #include "spdylay_frame.h" #include "spdylay_helper.h" #include "spdylay_test_helper.h" +/* Reads |len_size| byte from |data| as |len_size| byte network byte + order integer, and returns it in host byte order. Currently, we + only support len_size == 2 or 4 */ +static int get_packed_hd_len(uint8_t *data, size_t len_size) +{ + if(len_size == 2) { + return spdylay_get_uint16(data); + } else if(len_size == 4) { + return spdylay_get_uint32(data); + } else { + /* Not supported */ + assert(0); + } +} + static const char *headers[] = { "method", "GET", "scheme", "https", @@ -181,12 +198,99 @@ void test_spdylay_frame_pack_nv_duplicate_keys(void) spdylay_frame_nv_del(nv); } +static const char *multi_empty_headers1[] = { + "a", "", + "a", "", + NULL +}; + +static const char *multi_empty_headers2[] = { + "a", "/", + "a", "", + NULL +}; + +static const char *multi_empty_headers3[] = { + "a", "", + "a", "/", + NULL +}; + void test_spdylay_frame_count_nv_space(void) { size_t len_size = 2; CU_ASSERT(85 == spdylay_frame_count_nv_space((char**)headers, len_size)); len_size = 4; CU_ASSERT(111 == spdylay_frame_count_nv_space((char**)headers, len_size)); + /* only ("a", "") is counted */ + CU_ASSERT(13 == spdylay_frame_count_nv_space((char**)multi_empty_headers1, + len_size)); + /* only ("a", "/") is counted */ + CU_ASSERT(14 == spdylay_frame_count_nv_space((char**)multi_empty_headers2, + len_size)); + /* only ("a", "/") is counted */ + CU_ASSERT(14 == spdylay_frame_count_nv_space((char**)multi_empty_headers3, + len_size)); +} + +static void frame_pack_nv_empty_value_check(uint8_t *outptr, + int vallen, + const char *val, + size_t len_size) +{ + int len; + len = get_packed_hd_len(outptr, len_size); + CU_ASSERT(1 == len); + outptr += len_size; + len = get_packed_hd_len(outptr, len_size); + CU_ASSERT(1 == len); + outptr += len_size; + CU_ASSERT(0 == memcmp("a", outptr, len)); + outptr += len; + len = get_packed_hd_len(outptr, len_size); + CU_ASSERT(vallen == len); + len += len_size; + if(vallen == len) { + CU_ASSERT(0 == memcmp(val, outptr, vallen)); + } +} + +static void test_spdylay_frame_pack_nv_empty_value_with(size_t len_size) +{ + uint8_t out[256]; + char **nv; + ssize_t rv; + int off = (len_size == 2 ? -6 : 0); + + nv = spdylay_frame_nv_copy(multi_empty_headers1); + rv = spdylay_frame_pack_nv(out, nv, len_size); + CU_ASSERT(13+off == rv); + frame_pack_nv_empty_value_check(out, 0, NULL, len_size); + spdylay_frame_nv_del(nv); + + nv = spdylay_frame_nv_copy(multi_empty_headers2); + rv = spdylay_frame_pack_nv(out, nv, len_size); + CU_ASSERT(14+off == rv); + frame_pack_nv_empty_value_check(out, 1, "/", len_size); + spdylay_frame_nv_del(nv); + + nv = spdylay_frame_nv_copy(multi_empty_headers3); + rv = spdylay_frame_pack_nv(out, nv, len_size); + CU_ASSERT(14+off == rv); + frame_pack_nv_empty_value_check(out, 1, "/", len_size); + spdylay_frame_nv_del(nv); +} + +void test_spdylay_frame_pack_nv_empty_value_spdy2(void) +{ + test_spdylay_frame_pack_nv_empty_value_with + (spdylay_frame_get_len_size(SPDYLAY_PROTO_SPDY2)); +} + +void test_spdylay_frame_pack_nv_empty_value_spdy3(void) +{ + test_spdylay_frame_pack_nv_empty_value_with + (spdylay_frame_get_len_size(SPDYLAY_PROTO_SPDY3)); } void test_spdylay_frame_count_unpack_nv_space(void) diff --git a/tests/spdylay_frame_test.h b/tests/spdylay_frame_test.h index a4ee35a6..555dbe0f 100644 --- a/tests/spdylay_frame_test.h +++ b/tests/spdylay_frame_test.h @@ -28,6 +28,8 @@ void test_spdylay_frame_unpack_nv_spdy2(void); void test_spdylay_frame_unpack_nv_spdy3(void); void test_spdylay_frame_pack_nv_duplicate_keys(void); +void test_spdylay_frame_pack_nv_empty_value_spdy2(void); +void test_spdylay_frame_pack_nv_empty_value_spdy3(void); void test_spdylay_frame_count_nv_space(void); void test_spdylay_frame_count_unpack_nv_space(void); void test_spdylay_frame_pack_ping(void);