You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Küng <to...@gmail.com> on 2010/11/02 19:20:43 UTC

svn_client_delete with multiple targets

Hi,

I found a small problem in svn_client_delete4() when passing multiple 
urls to delete.

If at least one of the urls contain escaped chars, then the deletion 
fails. The reason is in libsvn_client/delete.c, line 209:
       item_url = svn_path_url_add_component2(common, path, subpool);

that line combines the common root of all delete targets with the 
relative path of each of the targets, but it escapes those again.

I thought that the svn API requires all urls passed to it to be already 
escaped - this at least is required if only one single target is passed 
to the API. Only if multiple targets are passed, then those get escaped 
again.

The problem isn't just on trunk but also in 1.6.x.

Does this mean that for multiple targets clients must not escape the 
urls? Not sure if this is a bug, but it's at least an inconsistency in 
the API.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

RE: svn_client_delete with multiple targets

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> Sent: dinsdag 2 november 2010 20:21
> To: Subversion Development
> Subject: svn_client_delete with multiple targets
> 
> Hi,
> 
> I found a small problem in svn_client_delete4() when passing multiple
> urls to delete.
> 
> If at least one of the urls contain escaped chars, then the deletion
> fails. The reason is in libsvn_client/delete.c, line 209:
>        item_url = svn_path_url_add_component2(common, path, subpool);
> 
> that line combines the common root of all delete targets with the
> relative path of each of the targets, but it escapes those again.
> 
> I thought that the svn API requires all urls passed to it to be already
> escaped - this at least is required if only one single target is passed
> to the API. Only if multiple targets are passed, then those get escaped
> again.

Yes, all APIs which don't explicitly document other behavior require
canonical paths/dirents/urls/relpaths for their arguments.

I added a regression test on this problem in r1030279.

The easy fix: Just replacing svn_path_url_add_component2() with
svn_uri_join() doesn't fix this test (and breaks basic_tests.py 50), because
svn doesn't pass the urls properly escaped. So this requires a bit more
work.

	Bert
> 
> The problem isn't just on trunk but also in 1.6.x.
> 
> Does this mean that for multiple targets clients must not escape the
> urls? Not sure if this is a bug, but it's at least an inconsistency in
> the API.
> 
> Stefan


RE: svn_client_delete with multiple targets

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> Sent: dinsdag 2 november 2010 20:21
> To: Subversion Development
> Subject: svn_client_delete with multiple targets
> 
> Hi,
> 
> I found a small problem in svn_client_delete4() when passing multiple
> urls to delete.
> 
> If at least one of the urls contain escaped chars, then the deletion
> fails. The reason is in libsvn_client/delete.c, line 209:
>        item_url = svn_path_url_add_component2(common, path, subpool);
> 
> that line combines the common root of all delete targets with the
> relative path of each of the targets, but it escapes those again.
> 
> I thought that the svn API requires all urls passed to it to be already
> escaped - this at least is required if only one single target is passed
> to the API. Only if multiple targets are passed, then those get escaped
> again.

Yes, all APIs which don't explicitly document other behavior require
canonical paths/dirents/urls/relpaths for their arguments.

I added a regression test on this problem in r1030279.

The easy fix: Just replacing svn_path_url_add_component2() with
svn_uri_join() doesn't fix this test (and breaks basic_tests.py 50), because
svn doesn't pass the urls properly escaped. So this requires a bit more
work.

	Bert
> 
> The problem isn't just on trunk but also in 1.6.x.
> 
> Does this mean that for multiple targets clients must not escape the
> urls? Not sure if this is a bug, but it's at least an inconsistency in
> the API.
> 
> Stefan