I've attached an updated patch with some test cases and a couple of fixes to the parsing logic.
On Sun, Aug 30, 2015 at 11:04 AM, Darshit Shah <[email protected]> wrote: > On Sun, Aug 30, 2015 at 12:51 AM, Tim Rühsen <[email protected]> wrote: >> Am Samstag, 29. August 2015, 23:13:23 schrieb Darshit Shah: >>> I've written a unit test for the parse_content_range() method. >>> However, I haven't yet populated it with various test cases. >>> Sharing the patch for the unit test here. I will add more test cases >>> for this test later. >>> >>> Kindly do review the patch. If no one complains, I'll push it in a >>> couple of days. >> >> Hi Darshit, >> >> some of the 'valid' tests >> > On closer inspection, some of these are *NOT* valid Header values. But > Wget currently passes them. This is a parsing bug in my opinion. > > RFC 7233 states: > > "A Content-Range field value is invalid if it contains a > byte-range-resp that has a last-byte-pos value less than its > first-byte-pos value, or a complete-length value less than or equal > to its last-byte-pos value. The recipient of an invalid > Content-Range MUST NOT attempt to recombine the received content with > a stored representation." > > Based on this, the first two examples provided are illegal. Similarly > a header value such as: > { "bytes 100-99/1000", 100, 99, 1000} > should also be very clearly illegal, but Wget currently allows it. I'm > not sure about the behaviour of the program on receipt of such a > header, but the function should clearly be failing on this test. > >> 0-max >> { "bytes 0-1000/1000", 0, 1000, 1000} >> non0-max >> { "bytes 1-1000/1000", 1, 1000, 1000} >> 0-valid >> { "bytes 0-500/1000", 0, 500, 1000} >> non0-valid >> { "bytes 1-500/1000", 1, 500, 1000} >> 0-(max-1) >> { "bytes 0-999/1000", 0, 999, 1000} >> non0-(max-1) >> { "bytes 1-999/1000", 1, 999, 1000} >> >> And please add some tests using >=2^31 and >=2^32 as values. >> >> Regards, Tim > > > > -- > Thanking You, > Darshit Shah -- Thanking You, Darshit Shah
From 727ca78eb615b04f159d5c76cc1bae20119b628d Mon Sep 17 00:00:00 2001 From: Darshit Shah <[email protected]> Date: Sat, 29 Aug 2015 23:08:39 +0530 Subject: [PATCH] Add unit test for parse_content_range() method * http.c (test_parse_range_header): New function to test the function for parsing the HTTP/1.1 Content-Range header. * test.[ch]: Same * http.c (parse_content_range): Fix parsing code. Fail on scenarios mentioned in rfc 7233. --- src/http.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++- src/test.c | 1 + src/test.h | 1 + 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/http.c b/src/http.c index e96cad7..20275ea 100644 --- a/src/http.c +++ b/src/http.c @@ -909,9 +909,13 @@ parse_content_range (const char *hdr, wgint *first_byte_ptr, ++hdr; for (num = 0; c_isdigit (*hdr); hdr++) num = 10 * num + (*hdr - '0'); - if (*hdr != '/' || !c_isdigit (*(hdr + 1))) + if (*hdr != '/') return false; *last_byte_ptr = num; + if (!(c_isdigit (*(hdr + 1)) || *(hdr + 1) == '*')) + return false; + if (*last_byte_ptr < *first_byte_ptr) + return false; ++hdr; if (*hdr == '*') num = -1; @@ -919,6 +923,8 @@ parse_content_range (const char *hdr, wgint *first_byte_ptr, for (num = 0; c_isdigit (*hdr); hdr++) num = 10 * num + (*hdr - '0'); *entity_length_ptr = num; + if ((*entity_length_ptr <= *last_byte_ptr) && *entity_length_ptr != -1) + return false; return true; } @@ -4892,6 +4898,51 @@ ensure_extension (struct http_stat *hs, const char *ext, int *dt) } #ifdef TESTING + +const char * +test_parse_range_header(void) +{ + static const struct { + const char * rangehdr; + const wgint firstbyte; + const wgint lastbyte; + const wgint length; + const bool shouldPass; + } test_array[] = { + { "bytes 0-1000/1000", 0, 1000, 1000, false }, + { "bytes 0-999/1000", 0, 999, 1000, true }, + { "bytes 100-99/1000", 100, 99, 1000, false }, + { "bytes 100-100/1000", 100, 100, 1000, true }, + { "bytes 0-1000/100000000", 0, 1000, 100000000, true }, + { "bytes 1-999/1000", 1, 999, 1000, true }, + { "bytes 42-1233/1234", 42, 1233, 1234, true }, + { "bytes 42-1233/*", 42, 1233, -1, true }, + { "bytes 0-2147483648/2147483649", 0, 2147483648, 2147483649, true }, + { "bytes 2147483648-4294967296/4294967297", 2147483648, 4294967296, 4294967297, true } + }; + + wgint firstbyteptr[sizeof(wgint)]; + wgint lastbyteptr[sizeof(wgint)]; + wgint lengthptr[sizeof(wgint)]; + bool result; + for (unsigned i = 0; i < countof (test_array); i++) + { + result = parse_content_range (test_array[i].rangehdr, firstbyteptr, lastbyteptr, lengthptr); +#if 0 + printf ("%ld %ld\n", test_array[i].firstbyte, *firstbyteptr); + printf ("%ld %ld\n", test_array[i].lastbyte, *lastbyteptr); + printf ("%ld %ld\n", test_array[i].length, *lengthptr); + printf ("\n"); +#endif + mu_assert ("test_parse_range_header: False Negative", result == test_array[i].shouldPass); + mu_assert ("test_parse_range_header: Bad parse", test_array[i].firstbyte == *firstbyteptr && + test_array[i].lastbyte == *lastbyteptr && + test_array[i].length == *lengthptr); + } + + return NULL; +} + const char * test_parse_content_disposition(void) { diff --git a/src/test.c b/src/test.c index 5278925..cb01de3 100644 --- a/src/test.c +++ b/src/test.c @@ -54,6 +54,7 @@ all_tests(void) mu_run_test (test_has_key); #endif mu_run_test (test_parse_content_disposition); + mu_run_test (test_parse_range_header); mu_run_test (test_subdir_p); mu_run_test (test_dir_matches_p); mu_run_test (test_commands_sorted); diff --git a/src/test.h b/src/test.h index f74c162..4e0e1f2 100644 --- a/src/test.h +++ b/src/test.h @@ -48,6 +48,7 @@ const char *test_has_key (void); const char *test_find_key_value (void); const char *test_find_key_values (void); const char *test_parse_content_disposition(void); +const char *test_parse_range_header(void); const char *test_commands_sorted(void); const char *test_cmd_spec_restrict_file_names(void); const char *test_is_robots_txt_url(void); -- 2.5.0
