[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fix for access violation in svn_fs__path_valid()

From: Stefan <luke1410_at_posteo.de>
Date: Tue, 9 Feb 2016 23:20:42 +0100

On 2/9/2016 04:22, Daniel Shahaf wrote:
> Stefan wrote on Sun, Feb 07, 2016 at 20:51:08 +0100:
>> On 2/7/2016 01:22, Daniel Shahaf wrote:
>>> Stefan wrote on Mon, Feb 01, 2016 at 00:34:32 +0100:
>>>> +++ fs-loader.c (working copy)
>>>> @@ -461,7 +461,8 @@
>>>> if (! svn_utf__cstring_is_valid(path))
>>>> {
>>>> return svn_error_createf(SVN_ERR_FS_PATH_SYNTAX, NULL,
>>>> - _("Path '%s' is not in UTF-8"), path);
>>>> + _("Path '%s' is not in UTF-8"),
>>>> + path ? path : "NULL");
>>> Is this actually a problem? svn_error_createf() uses apr_pvsprintf()
>>> which (by code inspection) accepts NULL here.
>> TBH I didn't look further... Just assumed that apr_pvsprintf() would just be
>> a redefine. If it's a specific implementation which checks for NULL here and
>> handles it, then pls disregard my patch.
> https://svn.apache.org/viewvc/apr/apr/tags/1.3.0/memory/unix/apr_pools.c?revision=661875&view=markup#l1100
> https://svn.apache.org/viewvc/apr/apr/tags/1.3.0/strings/apr_snprintf.c?revision=661875&view=markup#l953
>
> I didn't check later versions. (In theory, apr-2.0 might have
> changed this behaviour. I have no reason to think it did.)
Right, with these references found the code in the 1.5.2 source and it's
still the same there (with regards to checking for null-ptrs with %s).
Thanks for digging out the references for me
>
>> P.S. I tried to check out the function definition myself, but could only
>> spot the declaration in the apr source. Couldn't trace down where the
>> function is defined...
> I found it with ctags(1) with -IAPR_DECLARE:
>
> % ctags --extra=+qf --c-types=+p --python-types=-i -IAPR_DECLARE -R .
> % grep apr_pvsprintf tags
> apr_pvsprintf include/apr_strings.h /^APR_DECLARE(char *) apr_pvsprintf(apr_pool_t *p, const char *fmt, va_list ap);$/;" p
> apr_pvsprintf memory/unix/apr_pools.c /^APR_DECLARE(char *) apr_pvsprintf(apr_pool_t *pool, const char *fmt, va_list ap)$/;" f
> % vim -t apr_pvsprintf
>
Was always there in front of me directly. Should have just looked at the
code. At first spot to me it looked as if it wouldn't apply here, since
the definitions were in memory/unix/xxx and I assumed that's just code
which is unix specific... And I got confused by APR_DECLARE() assuming
that these were just forward declarations of these functions...
Now me knows better. ;-)

Regards,
Stefan
Received on 2016-02-09 23:39:41 CET

This is an archived mail posted to the Subversion Dev mailing list.