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