Branko Čibej wrote:
> Klaus Rennecke wrote:
>> Index: D:/kre/workspace/svn/subversion/libsvn_subr/path.c
>> ===================================================================
>> [...]
>> +#include <ctype.h> /* for svn_path_uri_decode() */
>> [...]
> This should be "#include <apr_lib.h>" and "apr_isxdigit". Otherwise, +1.
> We should make a similar change in svn_path_is_uri_safe.
That is fixed with this new patch. I had a look around and noticed that
isxdigit is used in some places. For instance in fs_fs.c. But I cannot
judge if that's any problem.
> Hm. I hope noone ever tries to port Subversion to an EBCDIC platform,
> because not one of these checks will work correctly there... :-\
I'm now directly comparing to ASCII values, so this should not be an
issue anymore. Keeps things simple as well. :-)
Should I try to squeeze the new patches onto the issue tracker?
[[[
Check that the two characters following the % escape are valid hex
digits. This serves to check for premature end of input as well.
Fixes Issue #1947 svn_path_uri_decode may copy garbage and overrun
buffer when given partial % escape.
* subversion/libsvn_subr/path.c
(svn_path_uri_decode): Check syntax of % escape.
* subversion/tests/libsvn_subr/path-test.c
(test_uri_decode): New test function.
(test_funcs): Added test_uri_decode.
]]]
/Klaus
Index: D:/kre/workspace/svn/subversion/libsvn_subr/path.c
===================================================================
--- D:/kre/workspace/svn/subversion/libsvn_subr/path.c (revision 10168)
+++ D:/kre/workspace/svn/subversion/libsvn_subr/path.c (working copy)
@@ -43,6 +43,12 @@
the OS! */
#define SVN_PATH_IS_PLATFORM_EMPTY(s,n) ((n) == 1 && (s)[0] == '.')
+/* TRUE if c is a valid hex digit in ASCII, as defined per RFC 2396
+ section 2. Used in svn_path_uri_decode. */
+#define SVN_PATH_IS_URLXDIGIT(c) (((c) >= 48 && (c) <= 57) \
+ || ((c) >= 65 && (c) <= 70) \
+ || ((c) >= 97 && (c) <= 102))
+
const char *
svn_path_internal_style (const char *path, apr_pool_t *pool)
@@ -892,7 +898,9 @@
* RFC 2396, section 3.3 */
c = ' ';
}
- else if (c == '%')
+ else if (c == '%'
+ && SVN_PATH_IS_URLXDIGIT (path[i + 1])
+ && SVN_PATH_IS_URLXDIGIT (path[i + 2]))
{
char digitz[3];
digitz[0] = path[++i];
Index: D:/kre/workspace/svn/subversion/tests/libsvn_subr/path-test.c
===================================================================
--- D:/kre/workspace/svn/subversion/tests/libsvn_subr/path-test.c (revision 10170)
+++ D:/kre/workspace/svn/subversion/tests/libsvn_subr/path-test.c (working copy)
@@ -292,6 +292,45 @@
static svn_error_t *
+test_uri_decode (const char **msg,
+ svn_boolean_t msg_only,
+ apr_pool_t *pool)
+{
+ int i;
+
+ const char *paths[3][2] = {
+ { "http://c.r.a/s%\0008me",
+ "http://c.r.a/s%"},
+ { "http://c.r.a/s%6\000me",
+ "http://c.r.a/s%6" },
+ { "http://c.r.a/s%68me",
+ "http://c.r.a/shme" },
+ };
+
+ *msg = "test svn_path_uri_decode with invalid escape";
+
+ if (msg_only)
+ return SVN_NO_ERROR;
+
+ for (i = 0; i < 3; i++)
+ {
+ const char *de_path;
+
+ /* URI-decode the path, and verify the results. */
+ de_path = svn_path_uri_decode (paths[i][0], pool);
+ if (strcmp (de_path, paths[i][1]))
+ {
+ return svn_error_createf
+ (SVN_ERR_TEST_FAILED, NULL,
+ "svn_path_uri_decode ('%s') returned '%s' instead of '%s'",
+ paths[i][0], de_path, paths[i][1]);
+ }
+ }
+ return SVN_NO_ERROR;
+}
+
+
+static svn_error_t *
test_join (const char **msg,
svn_boolean_t msg_only,
apr_pool_t *pool)
@@ -605,6 +644,7 @@
SVN_TEST_PASS (test_is_url),
SVN_TEST_PASS (test_is_uri_safe),
SVN_TEST_PASS (test_uri_encode),
+ SVN_TEST_PASS (test_uri_decode),
SVN_TEST_PASS (test_join),
SVN_TEST_PASS (test_basename),
SVN_TEST_PASS (test_decompose),
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jul 7 22:36:28 2004