On Mon, Aug 29, 2016 at 12:36:05PM +0300, Ivan Zhakov wrote: > On 28 August 2016 at 00:02, <s...@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
Hi Ivan, Yes, I agree. I'll try to fix this soon but I'm travelling at the moment. If you have time to fix it, please do! Thanks.
