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

Reply via email to