Julian Foad wrote: > Are you willing to add random-input testing for them?
The attached patch 'dirent-uri-test-random-2.patch' tests rules like: * every result should pass an X_is_canonical() test (obvious by code inspection); * every other input should produce SVN_ERR_CANONICALIZATION_FAILED; * when a path is "canonical", it should be unchanged by "canonicalize". Some findings: * svn_uri_canonicalize_safe("") aborts; * svn_uri_canonicalize_safe("/foo") aborts; * upper/lower case inconsistencies in URIs I previously also found upper/lower case inconsistencies in dirent drive letters, when running these tests with "#define SVN_USE_DOS_PATHS" set in dirent_uri.c, but am right now failing to replicate that. > * the 'relpath' one is not needed because, AFAIK, it's possible to > canonicalize any > relpath and we already do; Maybe that will no longer be true if we decide to disallow some characters (control characters) and/or check for valid UTF-8 in the future. -- - Julian
Proof of concept: add random testing of *_canonicalize_safe and of *_is_canonical and of *_canonicalize. Note: the test avoids inputs of "" and "/..." to svn_uri_canonicalize, because it assertion-fails on them. Sample outputs (test_canonicalize_safe_random): [[[ Expected: 'j://5:0R' Found: 'j://5:0r' at dirent_uri-test.c:1234 ]]] * subversion/tests/libsvn_subr/dirent_uri-test.c (random_filename): New. (relpath_is_canonical): New, wrapper macro. (SVN_TEST_STRING_ASSERT): Temporarily redefine so the test keeps going. (test_canonicalize_safe_random, test_canonicalize_is_canonical_random): New tests. (test_funcs): Run them. --This line, and those below, will be ignored-- Index: subversion/tests/libsvn_subr/dirent_uri-test.c =================================================================== --- subversion/tests/libsvn_subr/dirent_uri-test.c (revision 1848952) +++ subversion/tests/libsvn_subr/dirent_uri-test.c (working copy) @@ -1129,12 +1129,182 @@ test_dirent_is_canonical(apr_pool_t *poo canonicalized); } return SVN_NO_ERROR; } +/* return a random string, somewhat like a filename/path */ +static char * +random_filename(void) +{ + static char s[10]; + int len = (long)rand() * 9 / RAND_MAX; + int i; + + for (i = 0; i < len; ++i) + { + float r1 = (float)rand() / RAND_MAX; + float r2 = (float)rand() / RAND_MAX; + char c; + + if (r1 < 0.2) /* about 1 in 5: special path chars */ + { + c = "./:"[(int)(r2 * 2.5)]; /* many . and / and a few : */ + } + else if (r1 < 0.3) + { + c = (char)(r2 * 32); /* about 1 in 10: control chars */ + if (c == 0) c = 127; + } + else + { + c = (char)(32 + r2 * 95); /* the rest: printable ASCII chars */ + } + s[i] = c; + } + s[i] = '\0'; + + return s; +} + +/* a version of svn_relpath_is_canonical that matches the signature + * of the other _is_canonical funtions. */ +#define relpath_is_canonical(f, pool) svn_relpath_is_canonical(f) + +#define SVN_TEST_STRING_ASSERT(expr, expected_expr) \ + (strcmp(expr, expected_expr) == 0 ? (void)0 : \ + SVN_DBG(("'%s' != '%s'", expr, expected_expr))) + +static svn_error_t * +test_canonicalize_safe_random(apr_pool_t *pool) +{ + int tries = 10000000; + int i; + + for (i = 0; i < tries; ++i) + { + const char *f = random_filename(); + const char *ized, *again; + svn_error_t *err; + + /*SVN_DBG(("f='%s'", f));*/ + + /* relpath */ + err = svn_relpath_canonicalize_safe(&ized, NULL, f, pool, pool); + if (relpath_is_canonical(f, pool)) + { + SVN_TEST_ASSERT(!err); + SVN_TEST_STRING_ASSERT(ized, f); + } + else if (!err) + { + SVN_TEST_ASSERT(relpath_is_canonical(ized, pool)); + SVN_ERR(svn_relpath_canonicalize_safe(&again, NULL, ized, pool, pool)); + SVN_TEST_STRING_ASSERT(again, ized); + } + else + { + SVN_TEST_ASSERT(!relpath_is_canonical(ized, pool)); + SVN_TEST_ASSERT(err->apr_err == SVN_ERR_CANONICALIZATION_FAILED); + svn_error_clear(err); + } + + /* dirent */ + err = svn_dirent_canonicalize_safe(&ized, NULL, f, pool, pool); + if (svn_dirent_is_canonical(f, pool)) + { + SVN_TEST_ASSERT(!err); + SVN_TEST_STRING_ASSERT(ized, f); + } + else if (!err) + { + SVN_TEST_ASSERT(svn_dirent_is_canonical(ized, pool)); + SVN_ERR(svn_dirent_canonicalize_safe(&again, NULL, ized, pool, pool)); + SVN_TEST_STRING_ASSERT(again, ized); + } + else + { + SVN_TEST_ASSERT(!svn_dirent_is_canonical(ized, pool)); + SVN_TEST_ASSERT(err->apr_err == SVN_ERR_CANONICALIZATION_FAILED); + svn_error_clear(err); + } + + /* URI */ + /* svn_uri_canonicalize assertion-fails when input is empty or begins with '/' */ + if (f[0] == '\0') continue; /* ### known fail */ + if (f[0] == '/') continue; /* ### known fail */ + err = svn_uri_canonicalize_safe(&ized, NULL, f, pool, pool); + if (svn_uri_is_canonical(f, pool)) + { + SVN_TEST_ASSERT(!err); + SVN_TEST_STRING_ASSERT(ized, f); + } + else if (!err) + { + SVN_TEST_ASSERT(svn_uri_is_canonical(ized, pool)); + SVN_ERR(svn_uri_canonicalize_safe(&again, NULL, ized, pool, pool)); + SVN_TEST_STRING_ASSERT(again, ized); + } + else + { + SVN_TEST_ASSERT(!svn_uri_is_canonical(ized, pool)); + SVN_TEST_ASSERT(err->apr_err == SVN_ERR_CANONICALIZATION_FAILED); + svn_error_clear(err); + } + } + + return SVN_NO_ERROR; +} + +static svn_error_t * +test_canonicalize_is_canonical_random(apr_pool_t *pool) +{ + int tries = 1000000; + int i; + + for (i = 0; i < tries; ++i) + { + const char *f = random_filename(); + const char *ized; + + /*SVN_DBG(("f='%s'", f));*/ + + /* relpath */ + ized = svn_relpath_canonicalize(f, pool); + if (!svn_relpath_is_canonical(ized)) + { + SVN_DBG(("non-c. relpath: '%s' -> '%s'", f, ized)); + /*SVN_TEST_ASSERT(svn_relpath_is_canonical( + svn_relpath_canonicalize(f, pool)));*/ + } + + /* dirent */ + ized = svn_dirent_canonicalize(f, pool); + if (!svn_dirent_is_canonical(ized, pool)) + { + SVN_DBG(("non-c. dirent: '%s' -> '%s'", f, ized)); + /*SVN_TEST_ASSERT(svn_dirent_is_canonical( + svn_dirent_canonicalize(f, pool), pool));*/ + } + + /* URI */ + /* svn_uri_canonicalize assertion-fails when input is empty or begins with '/' */ + if (f[0] == '\0') continue; /* ### known fail */ + if (f[0] == '/') continue; /* ### known fail */ + ized = svn_uri_canonicalize(f, pool); + if (!svn_uri_is_canonical(ized, pool)) + { + SVN_DBG(("non-c. URI: '%s' -> '%s'", f, ized)); + /*SVN_TEST_ASSERT(svn_uri_is_canonical( + svn_uri_canonicalize(f, pool), pool));*/ + } + } + + return SVN_NO_ERROR; +} + static svn_error_t * test_relpath_is_canonical(apr_pool_t *pool) { const testcase_is_canonical_t *t; static const testcase_is_canonical_t tests[] = { { "", TRUE }, @@ -3016,10 +3186,14 @@ static struct svn_test_descriptor_t test SVN_TEST_PASS2(test_fspath_get_longest_ancestor, "test svn_fspath__get_longest_ancestor"), SVN_TEST_PASS2(test_cert_match_dns_identity, "test svn_cert__match_dns_identity"), SVN_TEST_XFAIL2(test_rule3, "test match with RFC 6125 s. 6.4.3 Rule 3"), + SVN_TEST_PASS2(test_canonicalize_safe_random, + "test_canonicalize_safe_random"), + SVN_TEST_PASS2(test_canonicalize_is_canonical_random, + "test_canonicalize_is_canonical_random"), SVN_TEST_NULL }; SVN_TEST_MAIN