From eb5e0599afe3bae86a375acea4d8ca106594c45b Mon Sep 17 00:00:00 2001 From: Vincent Sanders Date: Wed, 3 Aug 2016 15:04:28 +0100 Subject: Improve percent escaping testing, parameter checking and documentation --- test/urlescape.c | 186 ++++++++++++++++++++++++++++++++++++++++++++++++------- utils/url.c | 21 +++++-- 2 files changed, 179 insertions(+), 28 deletions(-) diff --git a/test/urlescape.c b/test/urlescape.c index 5d9d9327b..e3d701aeb 100644 --- a/test/urlescape.c +++ b/test/urlescape.c @@ -19,6 +19,15 @@ /** * \file * Test url percent encoding operations. + * + * Percent encoding of URI is subject to RFC3986 however this is not + * testing URI behaviour purely the percent encoding so only the + * unreserved set is not encoded. + * + * \note Earlier RFC (2396, 1738 and 1630) list the tilde ~ character + * as reserved so its handling is ambiguious + * + * \todo The url_escape should be tested for application/x-www-form-urlencoded type operation */ #include @@ -35,6 +44,7 @@ struct test_pairs { const char* test; + const size_t test_len; const char* res; const size_t res_len; }; @@ -57,8 +67,8 @@ const char all_chars[] = "\xe0\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef" "\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff"; -const char all_escaped[] = - "%01%02%03%04%05%06%07%08%09%0A%0B%0C%0D%0E%0F" +const char most_escaped_upper[] = + "%01%02%03%04%05%06%07%08%09%0A%0B%0C%0D%0E%0F" "%10%11%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F" "%20%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F" "0123456789%3A%3B%3C%3D%3E%3F" @@ -75,12 +85,32 @@ const char all_escaped[] = "%E0%E1%E2%E3%E4%E5%E6%E7%E8%E9%EA%EB%EC%ED%EE%EF" "%F0%F1%F2%F3%F4%F5%F6%F7%F8%F9%FA%FB%FC%FD%FE%FF"; +const char all_escaped_lower[] = + "%01%02%03%04%05%06%07%08%09%0A%0B%0C%0D%0E%0F" + "%10%11%12%13%14%15%16%17%18%19%1a%1b%1c%1d%1e%1f" + "%20%21%22%23%24%25%26%27%28%29%2a%2b%2c%2d%2e%2f" + "%30%31%32%33%34%35%36%37%38%39%3a%3b%3c%3d%3e%3f" + "%40%41%42%43%44%45%46%47%48%49%4a%4b%4c%4d%4e%4f" + "%50%51%52%53%54%55%56%57%58%59%5a%5b%5c%5d%5e%5f" + "%60%61%62%63%64%65%66%67%68%69%6a%6b%6c%6d%6e%6f" + "%70%71%72%73%74%75%76%77%78%79%7a%7b%7c%7d%7e%7f" + "%80%81%82%83%84%85%86%87%88%89%8a%8b%8c%8d%8e%8f" + "%90%91%92%93%94%95%96%97%98%99%9a%9b%9c%9d%9e%9f" + "%a0%a1%a2%a3%a4%a5%a6%a7%a8%a9%aa%ab%ac%ad%ae%af" + "%b0%b1%b2%b3%b4%b5%b6%b7%b8%b9%ba%bb%bc%bd%be%bf" + "%c0%c1%c2%c3%c4%c5%c6%c7%c8%c9%ca%cb%cc%cd%ce%cf" + "%d0%d1%d2%d3%d4%d5%d6%d7%d8%d9%da%db%dc%dd%de%df" + "%e0%e1%e2%e3%e4%e5%e6%e7%e8%e9%ea%eb%ec%ed%ee%ef" + "%f0%f1%f2%f3%f4%f5%f6%f7%f8%f9%fa%fb%fc%fd%fe%ff"; + +/** simple string that needs no escaping nor has escape in it */ +const char simple_string[] = "A.string.that.does.not.need.escaping"; + static const struct test_pairs url_escape_test_vec[] = { - { "", "" , 0 }, - { "A.string.that.does.not.need.escaping", - "A.string.that.does.not.need.escaping" , 0 }, - { " ", "%20" , 0 }, - { &all_chars[0], &all_escaped[0], 0 }, + { "", 0, "" , 0 }, + { &simple_string[0], SLEN(simple_string), &simple_string[0], 0 }, + { " ", 1, "%20" , 0 }, + { &all_chars[0], SLEN(all_chars), &most_escaped_upper[0], 0 }, }; @@ -93,57 +123,165 @@ START_TEST(url_escape_test) err = url_escape(tst->test, false, "", &esc_str); ck_assert(err == NSERROR_OK); + /* ensure result data is correct */ ck_assert_str_eq(esc_str, tst->res); + + free(esc_str); +} +END_TEST + +START_TEST(url_escape_api_nullparam_test) +{ + nserror err; + char *esc_str; + + err = url_escape(NULL, false, NULL, &esc_str); + ck_assert(err == NSERROR_BAD_PARAMETER); + + err = url_escape(&simple_string[0], false, NULL, NULL); + ck_assert(err == NSERROR_BAD_PARAMETER); + + err = url_escape(&simple_string[0], false, NULL, &esc_str); + ck_assert(err == NSERROR_OK); + + free(esc_str); } END_TEST +TCase *url_escape_case_create(void) +{ + TCase *tc; + tc = tcase_create("Escape"); + + tcase_add_test(tc, url_escape_api_nullparam_test); + + tcase_add_loop_test(tc, url_escape_test, + 0, NELEMS(url_escape_test_vec)); + + return tc; +} + static const struct test_pairs url_unescape_test_vec[] = { - { "", "" , 0 }, - { "A.string.that.does.not.need.unescaping", - "A.string.that.does.not.need.unescaping", - SLEN("A.string.that.does.not.need.unescaping") }, - { "%20", " " , 1 }, - { &all_escaped[0], &all_chars[0], SLEN(all_chars) }, + { "", 0, "" , 0 }, /* empty string */ + { "%20", 3, " ", 1 }, /* single character properly escaped */ + { "%0G", 3, "%0G", 3 }, /* single character with bad hex value */ + { "%20%0G%20", 9, " %0G ", 5 }, /* three src chars with bad hex value */ + { "%20%00%20", 9, " ", 3 }, /* three src chars with null hex value */ + { &simple_string[0], SLEN(simple_string), + &simple_string[0], SLEN(simple_string) }, /* normal string with no percent encoded characters */ + { &most_escaped_upper[0], SLEN(most_escaped_upper), + &all_chars[0], SLEN(all_chars) }, /* all characters that must be percent encoded */ + { &all_escaped_lower[0], SLEN(all_escaped_lower), + &all_chars[0], SLEN(all_chars) }, /* all characters percent encoded */ }; -START_TEST(url_unescape_test) +/** + * test that unescapes a test vector of strings and checks the result + * + * The source data length is not given. The result must be the correct + * length and match the expected output. + */ +START_TEST(url_unescape_simple_test) { nserror err; char *unesc_str; size_t unesc_length; - + const struct test_pairs *tst = &url_unescape_test_vec[_i]; err = url_unescape(tst->test, 0 , &unesc_length, &unesc_str); ck_assert(err == NSERROR_OK); + /* ensure length */ ck_assert_uint_eq(unesc_length, tst->res_len); + /* ensure contents */ ck_assert_str_eq(unesc_str, tst->res); } END_TEST +/** + * test that unescapes a test vector of strings and checks the result + * + * The source data length is specified. The result must be the correct + * length and match the expected output. + */ +START_TEST(url_unescape_length_test) +{ + nserror err; + char *unesc_str; + size_t unesc_length; -TCase *url_escape_case_create(void) + const struct test_pairs *tst = &url_unescape_test_vec[_i]; + + err = url_unescape(tst->test, tst->test_len , &unesc_length, &unesc_str); + + ck_assert(err == NSERROR_OK); + + /* ensure length */ + ck_assert_uint_eq(unesc_length, tst->res_len); + + /* ensure contents */ + ck_assert_str_eq(unesc_str, tst->res); + + free(unesc_str); +} +END_TEST + + +/** + * Test to ensure unescape works as expected when no returned length + * is provided. + */ +START_TEST(url_unescape_api_retlen_test) { - TCase *tc; - tc = tcase_create("Escape"); + nserror err; + char *unesc_str; - tcase_add_loop_test(tc, url_escape_test, - 0, NELEMS(url_escape_test_vec)); - - return tc; + err = url_unescape(&most_escaped_upper[0], 0, NULL, &unesc_str); + + ck_assert(err == NSERROR_OK); + + /* ensure length */ + ck_assert_uint_eq(strlen(unesc_str), SLEN(all_chars)); + + /* ensure contents */ + ck_assert_str_eq(unesc_str, &all_chars[0]); + + free(unesc_str); +} +END_TEST + +START_TEST(url_unescape_api_nullparam_test) +{ + nserror err; + char *unesc_str; + + err = url_unescape(NULL, 0, NULL, &unesc_str); + ck_assert(err == NSERROR_BAD_PARAMETER); + + err = url_unescape(&simple_string[0], 0, NULL, NULL); + ck_assert(err == NSERROR_BAD_PARAMETER); } +END_TEST + TCase *url_unescape_case_create(void) { TCase *tc; tc = tcase_create("Unescape"); - tcase_add_loop_test(tc, url_unescape_test, + tcase_add_test(tc, url_unescape_api_nullparam_test); + + tcase_add_test(tc, url_unescape_api_retlen_test); + + tcase_add_loop_test(tc, url_unescape_simple_test, 0, NELEMS(url_unescape_test_vec)); - + + tcase_add_loop_test(tc, url_unescape_length_test, + 0, NELEMS(url_unescape_test_vec)); + return tc; } diff --git a/utils/url.c b/utils/url.c index 861a62bdc..7a7b7a196 100644 --- a/utils/url.c +++ b/utils/url.c @@ -18,8 +18,17 @@ * along with this program. If not, see . */ -/** \file - * \brief Implementation of URL parsing and joining operations. +/** + * \file + * \brief Implementation of URI percent escaping. + * + * Percent encoding of URI is subject to RFC3986 however this is not + * implementing URI behaviour purely the percent encoding so only the + * unreserved set is not encoded and arbitrary binary data may be + * unescaped. + * + * \note Earlier RFC (2396, 1738 and 1630) list the tilde ~ character + * as special so its handling is ambiguious */ #include @@ -37,7 +46,7 @@ * * Must be called with valid hex char, results undefined otherwise. * - * \param[in] c char to convert yo value + * \param[in] c character to convert to value * \return the value of c */ static inline char xdigit_to_hex(char c) @@ -61,6 +70,10 @@ nserror url_unescape(const char *str, size_t length, char *res_pos; char *result; + if ((str == NULL) || (result_out == NULL)) { + return NSERROR_BAD_PARAMETER; + } + if (length == 0) { length = strlen(str); } @@ -121,7 +134,7 @@ nserror url_escape(const char *unescaped, bool sptoplus, const char *c; if (unescaped == NULL || result == NULL) { - return NSERROR_NOT_FOUND; + return NSERROR_BAD_PARAMETER; } len = strlen(unescaped); -- cgit v1.2.3