path-test is failing:
$ valgrind -q subversion/tests/libsvn_subr/.libs/lt-path-test 16
==13539== Invalid write of size 1
==13539== at 0x557C2A7: vsprintf (in /usr/lib/debug/libc-2.3.6.so)
==13539== by 0x55674F7: sprintf (in /usr/lib/debug/libc-2.3.6.so)
==13539== by 0x4C5431A: illegal_path_escape (path.c:1110)
==13539== by 0x4C5441A: svn_path_check_valid (path.c:1140)
==13539== by 0x40378B: test_path_check_valid (path-test.c:1012)
==13539== by 0x4B2499A: do_test_num (svn_test_main.c:192)
==13539== by 0x4B25132: main (svn_test_main.c:395)
==13539== Address 0x77B687C is 0 bytes after a block of size 4 alloc'd
==13539== at 0x4A1B858: malloc (vg_replace_malloc.c:149)
==13539== by 0x4EB2F95: pool_alloc (apr_pools.c:1287)
==13539== by 0x4C5FB63: my__realloc (svn_string.c:46)
==13539== by 0x4C604BB: svn_stringbuf_ensure (svn_string.c:370)
==13539== by 0x4C542ED: illegal_path_escape (path.c:1109)
==13539== by 0x4C5441A: svn_path_check_valid (path.c:1140)
==13539== by 0x40378B: test_path_check_valid (path-test.c:1012)
==13539== by 0x4B2499A: do_test_num (svn_test_main.c:192)
==13539== by 0x4B25132: main (svn_test_main.c:395)
The problem is that illegal_path_escape doesn't allocate enough space
here:
/* Now, sprintf() in our escaped character, making sure our
buffer is big enough to hold the '%' and two digits. We cast
the C to unsigned char here because the 'X' format character
will be tempted to treat it as an unsigned int...which causes
problem when messing with 0x80-0xFF chars. We also need space
for a null as sprintf will write one. */
/*### The backslash separator doesn't work too great with Windows,
but it's what we'll use for consistency with invalid utf8
formatting (until someone has a better idea) */
svn_stringbuf_ensure(retstr, retstr->len + 4);
sprintf(retstr->data + retstr->len, "\\%03o", (unsigned char)c);
The comment seems to assume hex ("two digits") but the code uses octal
so gives three digits, plus '\' plus null so 5 in total not the 4
allocated. Should this be octal or hex?
I would assume the code is correct except that the function is called
like this:
svn_error_t *
svn_path_check_valid(const char *path, apr_pool_t *pool)
{
const char *c;
for (c = path; *c; c++)
{
if (svn_ctype_iscntrl(*c))
{
return svn_error_createf
(SVN_ERR_FS_PATH_SYNTAX, NULL,
_("Invalid control character '0x%02x' in path '%s'"),
*c,
illegal_path_escape(svn_path_local_style(path, pool), pool));
}
which prints hex. (Note that in that code the *c should really be
cast to unsigned char before printing, although since none of the
cntrl characters are greater than 0x7f it doesn't matter in practice.
I also think that invalid_path_escape is a better name, but really I
wonder why it is a separate function from svn_path_check_valid which
is the only caller.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-11-23 00:09:22 CET