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

Re: svn commit: r1758069 - /subversion/trunk/subversion/libsvn_subr/dirent_uri.c

From: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Mon, 29 Aug 2016 12:36:05 +0300

On 28 August 2016 at 00:02, <stsp_at_apache.org> wrote:
> Author: stsp
> Date: Sat Aug 27 21:02:27 2016
> New Revision: 1758069
>
> URL: http://svn.apache.org/viewvc?rev=1758069&view=rev
> Log:
> Fix issue #4652 which shows how to trigger an assertion failure in
> svn_dirent_get_absolute() by passing invalid input on the command line.
>
> Report a proper error message instead.
>
> * subversion/libsvn_subr/dirent_uri.c
> (svn_dirent_get_absolute): If the caller passed a URL, return an error.
>
Hi Stefan,

I'm not sure that it's proper way to fix the reported reported:
svn_dirent_get_absolute() strictly requires @a absolute to be a
*canonicalized dirent*:
[[[
* Convert @a relative canonicalized dirent to an absolute dirent and
* return the results in @a *pabsolute.
]]]

Passing URL to svn_dirent_get_absolute() is an API violation and
SVN_ERR_ASSERT() is a proper way to react. As far I remember all
svn_dirent_*() functions are only expected to work with filesystem
paths, svn_url_*() functions are designed to work with URLs, while
svn_path_*() functions accepts filesystem path and URLs.

User input should be validated where it's received, i.e. in Subversion
command line client. We also may have problem above the stack, when
this URL is passed to another function that doesn't have an assertion
or something like that. In this particular case the problem is in
svn_cl__merge() which passes URL as the TARGET_WCPATH argument to
svn_client_merge5().

-- 
Ivan Zhakov
Received on 2016-08-29 11:43:22 CEST

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