You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by James McCoy <ve...@gmail.com> on 2013/09/05 05:50:49 UTC

upgrade_tests 7 FAIL when working directory has a "+" character

The basic_upgrade_1_0 test fails the svn_path_is_canonical(base, pool)
assertion in svn_path_join:

CMD: svnadmin create svn-test-work/repositories/upgrade_tests-7 --bdb-txn-nosync
<TIME = 0.472257>
CMD: svnadmin dump svn-test-work/local_tmp/repos | svnadmin load
svn-test-work/repositories/upgrade_tests-7 --ignore-uuid
<TIME = 0.034047>
CMD: svn info svn-test-work/working_copies/upgrade_tests-7
--config-dir '/build/buildd-subversion_1.7.9-1+nmu4-s390x-OpG8rc/subversion-1.7.9/BUILD/subversion/tests/cmdline/svn-test-work/local_tmp/config'
--password rayjandom --no-auth-cache --username jrandom
CMD: /build/buildd-subversion_1.7.9-1+nmu4-s390x-OpG8rc/subversion-1.7.9/BUILD/subversion/svn/svn
info svn-test-work/working_copies/upgrade_tests-7 --config-dir
'/build/buildd-subversion_1.7.9-1+nmu4-s390x-OpG8rc/subversion-1.7.9/BUILD/subversion/tests/cmdline/svn-test-work/local_tmp/config'
--password rayjandom --no-auth-cache --username jrandom exited with 1
<TIME = 0.119307>
svn: E155036: Please see the 'svn upgrade' command
svn: E155036: Working copy
'/build/buildd-subversion_1.7.9-1+nmu4-s390x-OpG8rc/subversion-1.7.9/BUILD/subversion/tests/cmdline/svn-test-work/working_copies/upgrade_tests-7'
is too old (format 4, created by Subversion <=1.3)
CMD: svn upgrade svn-test-work/working_copies/upgrade_tests-7
--config-dir '/build/buildd-subversion_1.7.9-1+nmu4-s390x-OpG8rc/subversion-1.7.9/BUILD/subversion/tests/cmdline/svn-test-work/local_tmp/config'
--password rayjandom --no-auth-cache --username jrandom
lt-svn: /build/buildd-subversion_1.7.9-1+nmu4-s390x-OpG8rc/subversion-1.7.9/subversion/libsvn_subr/path.c:102:
svn_path_join: Assertion `svn_path_is_canonical(base, pool)' failed.


The full build log is available at:
https://buildd.debian.org/status/fetch.php?pkg=subversion&arch=s390x&ver=1.7.9-1%2Bnmu4&stamp=1378306094

This was with svn 1.7.9.  I haven't had a chance to test newer
versions, but looking through the code in trunk, it looks like it
would still be a problem.

Cheers,
James

RE: upgrade_tests 7 FAIL when working directory has a "+" character

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

> -----Original Message-----
> From: Branko Čibej [mailto:brane@wandisco.com]
> Sent: donderdag 5 september 2013 22:00
> To: dev@subversion.apache.org
> Subject: Re: upgrade_tests 7 FAIL when working directory has a "+" character
> 
> On 05.09.2013 21:20, Philip Martin wrote:
> > Philip Martin <ph...@wandisco.com> writes:
> >
> >> Daniel Shahaf <da...@apache.org> writes:
> >>
> >>> On Wed, Sep 04, 2013 at 11:50:49PM -0400, James McCoy wrote:
> >>>> This was with svn 1.7.9.  I haven't had a chance to test newer
> >>>> versions, but looking through the code in trunk, it looks like it
> >>>> would still be a problem.
> >>> Confirmed, it reproduces in trunk:
> >>>
> >>> Index: subversion/tests/cmdline/upgrade_tests.py
> >>>
> ==========================================================
> =========
> >>> --- subversion/tests/cmdline/upgrade_tests.py   (revision 1520363)
> >>> +++ subversion/tests/cmdline/upgrade_tests.py   (working copy)
> >>> @@ -425,7 +425,7 @@ def simple_entries_replace(path, from_url,
> to_url)
> >>>  def basic_upgrade_1_0(sbox):
> >>>    "test upgrading a working copy created with 1.0.0"
> >>>
> >>> -  sbox.build(create_wc = False)
> >>> +  sbox.build(name='foo+', create_wc = False)
> >>>    replace_sbox_with_tarfile(sbox, 'upgrade_1_0.tar.bz2')
> >>>
> >>>    url = sbox.repo_url
> >>>
> >>> % ../runpytest upgrade basic_upgrade_1_0 2>&1 | head
> >>> W: lt-svn: subversion/libsvn_subr/path.c:119: svn_path_join_internal:
> Assertion `svn_path_is_canonical_internal(base, pool)' failed.
> >>>
> >>> W: CMD: /home/danielsh/src/svn/t1/subversion/svn/svn upgrade 'svn-
> test-work/working_copies/foo+' --config-dir
> /home/danielsh/src/svn/t1/subversion/tests/cmdline/svn-test-
> work/local_tmp/config --password rayjandom --no-auth-cache --username
> jrandom terminated by signal 6
> >> It looks like the %2B is failing svn_uri_is_canonical.  Looking at
> >> svn_uri__char_validity in libsvn_subr/path.c the table says that '+'
> >> should not be % encoded.  The problem is that the Python testsuite uses
> >> urllib.pathname2url(self.repo_dir) and that has different ideas, it
> >> does % encode '+'.
> > Index: subversion/tests/cmdline/svntest/sandbox.py
> >
> ==========================================================
> =========
> > --- subversion/tests/cmdline/svntest/sandbox.py	(revision 1520259)
> > +++ subversion/tests/cmdline/svntest/sandbox.py	(working copy)
> > @@ -57,7 +57,7 @@
> >      if not read_only:
> >        self.repo_dir = os.path.join(svntest.main.general_repo_dir, self.name)
> >        self.repo_url = (svntest.main.options.test_area_url + '/'
> > -                       + urllib.pathname2url(self.repo_dir))
> > +                       + urllib.quote(self.repo_dir, '/+'))
> >        self.add_test_path(self.repo_dir)
> >      else:
> >        self.repo_dir = svntest.main.pristine_greek_repos_dir
> >
> > That works for '+'.  A real patch would need to ensure that the safe set
> > passed to urllib.quote matches the safe set required by Subversion.
> 
> This:
> 
> http://tools.ietf.org/html/rfc3986#section-2
> 
> appears to say that '+' /should/ be %-encoded if it is in the path name
> part of the URL. So it would appear the svn_uri__char_validity is wrong.
> A '+' in the query part is a different matter, it's usually substituted
> for a space in that case. But Subversion does not generate URLs with
> non-empty queries.

Changing this table requires a working copy format bump...

I'm more surprised on how the url can enter the low level libraries without being canonicalized?

Especially since we re-canonicalize paths from 'entries' on upgrading to wc_db.db based formats. (Over the years we found a few extreme rare corner cases, but '+' in a path doesn't look uncommon to me)

	Bert


Re: upgrade_tests 7 FAIL when working directory has a "+" character

Posted by Branko Čibej <br...@wandisco.com>.
On 05.09.2013 21:20, Philip Martin wrote:
> Philip Martin <ph...@wandisco.com> writes:
>
>> Daniel Shahaf <da...@apache.org> writes:
>>
>>> On Wed, Sep 04, 2013 at 11:50:49PM -0400, James McCoy wrote:
>>>> This was with svn 1.7.9.  I haven't had a chance to test newer
>>>> versions, but looking through the code in trunk, it looks like it
>>>> would still be a problem.
>>> Confirmed, it reproduces in trunk:
>>>
>>> Index: subversion/tests/cmdline/upgrade_tests.py
>>> ===================================================================
>>> --- subversion/tests/cmdline/upgrade_tests.py   (revision 1520363)
>>> +++ subversion/tests/cmdline/upgrade_tests.py   (working copy)
>>> @@ -425,7 +425,7 @@ def simple_entries_replace(path, from_url, to_url)
>>>  def basic_upgrade_1_0(sbox):
>>>    "test upgrading a working copy created with 1.0.0"
>>>  
>>> -  sbox.build(create_wc = False)
>>> +  sbox.build(name='foo+', create_wc = False)
>>>    replace_sbox_with_tarfile(sbox, 'upgrade_1_0.tar.bz2')
>>>  
>>>    url = sbox.repo_url
>>>
>>> % ../runpytest upgrade basic_upgrade_1_0 2>&1 | head
>>> W: lt-svn: subversion/libsvn_subr/path.c:119: svn_path_join_internal: Assertion `svn_path_is_canonical_internal(base, pool)' failed.
>>>
>>> W: CMD: /home/danielsh/src/svn/t1/subversion/svn/svn upgrade 'svn-test-work/working_copies/foo+' --config-dir /home/danielsh/src/svn/t1/subversion/tests/cmdline/svn-test-work/local_tmp/config --password rayjandom --no-auth-cache --username jrandom terminated by signal 6
>> It looks like the %2B is failing svn_uri_is_canonical.  Looking at
>> svn_uri__char_validity in libsvn_subr/path.c the table says that '+'
>> should not be % encoded.  The problem is that the Python testsuite uses
>> urllib.pathname2url(self.repo_dir) and that has different ideas, it
>> does % encode '+'.
> Index: subversion/tests/cmdline/svntest/sandbox.py
> ===================================================================
> --- subversion/tests/cmdline/svntest/sandbox.py	(revision 1520259)
> +++ subversion/tests/cmdline/svntest/sandbox.py	(working copy)
> @@ -57,7 +57,7 @@
>      if not read_only:
>        self.repo_dir = os.path.join(svntest.main.general_repo_dir, self.name)
>        self.repo_url = (svntest.main.options.test_area_url + '/'
> -                       + urllib.pathname2url(self.repo_dir))
> +                       + urllib.quote(self.repo_dir, '/+'))
>        self.add_test_path(self.repo_dir)
>      else:
>        self.repo_dir = svntest.main.pristine_greek_repos_dir
>
> That works for '+'.  A real patch would need to ensure that the safe set
> passed to urllib.quote matches the safe set required by Subversion.

This:

http://tools.ietf.org/html/rfc3986#section-2

appears to say that '+' /should/ be %-encoded if it is in the path name
part of the URL. So it would appear the svn_uri__char_validity is wrong.
A '+' in the query part is a different matter, it's usually substituted
for a space in that case. But Subversion does not generate URLs with
non-empty queries.

-- Brane

-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com

RE: upgrade_tests 7 FAIL when working directory has a "+" character

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

> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: donderdag 5 september 2013 22:59
> To: 'Philip Martin'; 'Daniel Shahaf'
> Cc: 'James McCoy'; dev@subversion.apache.org
> Subject: RE: upgrade_tests 7 FAIL when working directory has a "+"
character
> 
> 
> 
> > -----Original Message-----
> > From: Philip Martin [mailto:philip.martin@wandisco.com]
> > Sent: donderdag 5 september 2013 21:20
> > To: Daniel Shahaf
> > Cc: James McCoy; dev@subversion.apache.org
> > Subject: Re: upgrade_tests 7 FAIL when working directory has a "+"
> character
> >
> > Philip Martin <ph...@wandisco.com> writes:
> >
> > > Daniel Shahaf <da...@apache.org> writes:
> > >
> > >> On Wed, Sep 04, 2013 at 11:50:49PM -0400, James McCoy wrote:
> > >>> This was with svn 1.7.9.  I haven't had a chance to test newer
> > >>> versions, but looking through the code in trunk, it looks like it
> > >>> would still be a problem.
> > >>
> > >> Confirmed, it reproduces in trunk:
> > >>
> > >> Index: subversion/tests/cmdline/upgrade_tests.py
> > >>
> >
> ==========================================================
> > =========
> > >> --- subversion/tests/cmdline/upgrade_tests.py   (revision 1520363)
> > >> +++ subversion/tests/cmdline/upgrade_tests.py   (working copy)
> > >> @@ -425,7 +425,7 @@ def simple_entries_replace(path, from_url,
> > to_url)
> > >>  def basic_upgrade_1_0(sbox):
> > >>    "test upgrading a working copy created with 1.0.0"
> > >>
> > >> -  sbox.build(create_wc = False)
> > >> +  sbox.build(name='foo+', create_wc = False)
> > >>    replace_sbox_with_tarfile(sbox, 'upgrade_1_0.tar.bz2')
> > >>
> > >>    url = sbox.repo_url
> > >>
> > >> % ../runpytest upgrade basic_upgrade_1_0 2>&1 | head
> > >> W: lt-svn: subversion/libsvn_subr/path.c:119: svn_path_join_internal:
> > Assertion `svn_path_is_canonical_internal(base, pool)' failed.
> > >>
> > >> W: CMD: /home/danielsh/src/svn/t1/subversion/svn/svn upgrade 'svn-
> > test-work/working_copies/foo+' --config-dir
> > /home/danielsh/src/svn/t1/subversion/tests/cmdline/svn-test-
> > work/local_tmp/config --password rayjandom --no-auth-cache --username
> > jrandom terminated by signal 6
> > >
> > > It looks like the %2B is failing svn_uri_is_canonical.  Looking at
> > > svn_uri__char_validity in libsvn_subr/path.c the table says that '+'
> > > should not be % encoded.  The problem is that the Python testsuite
uses
> > > urllib.pathname2url(self.repo_dir) and that has different ideas, it
> > > does % encode '+'.
> >
> > Index: subversion/tests/cmdline/svntest/sandbox.py
> >
> ==========================================================
> > =========
> > --- subversion/tests/cmdline/svntest/sandbox.py	(revision 1520259)
> > +++ subversion/tests/cmdline/svntest/sandbox.py	(working copy)
> > @@ -57,7 +57,7 @@
> >      if not read_only:
> >        self.repo_dir = os.path.join(svntest.main.general_repo_dir,
> self.name)
> >        self.repo_url = (svntest.main.options.test_area_url + '/'
> > -                       + urllib.pathname2url(self.repo_dir))
> > +                       + urllib.quote(self.repo_dir, '/+'))
> >        self.add_test_path(self.repo_dir)
> >      else:
> >        self.repo_dir = svntest.main.pristine_greek_repos_dir
> >
> > That works for '+'.  A real patch would need to ensure that the safe set
> > passed to urllib.quote matches the safe set required by Subversion.
> 
> How does this path enter 'svn'?
> 
> Shouldn't this just be handled by the 'svn_uri_canonicalize()' that
handles
> all url commandline arguments?
> 
> (Or does this code introduce it in the raw 'entries' files before the
> conversion?)

Yes,  basic_upgrade_1_0 (=upgrade tests 7) does indeed do a raw search and
replace on the entries file via xml_entries_relocate().

So the fix is probably to just add the necessary svn_uri_canonicalize() call
in the right place of the upgrade (=read from 'entries') code.

[Note the other mail: Changing the canonicalization rules requires a format
bump, as that can make urls in wc.db invalid]


> 	Bert


RE: upgrade_tests 7 FAIL when working directory has a "+" character

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

> -----Original Message-----
> From: Philip Martin [mailto:philip.martin@wandisco.com]
> Sent: donderdag 5 september 2013 21:20
> To: Daniel Shahaf
> Cc: James McCoy; dev@subversion.apache.org
> Subject: Re: upgrade_tests 7 FAIL when working directory has a "+"
character
> 
> Philip Martin <ph...@wandisco.com> writes:
> 
> > Daniel Shahaf <da...@apache.org> writes:
> >
> >> On Wed, Sep 04, 2013 at 11:50:49PM -0400, James McCoy wrote:
> >>> This was with svn 1.7.9.  I haven't had a chance to test newer
> >>> versions, but looking through the code in trunk, it looks like it
> >>> would still be a problem.
> >>
> >> Confirmed, it reproduces in trunk:
> >>
> >> Index: subversion/tests/cmdline/upgrade_tests.py
> >>
> ==========================================================
> =========
> >> --- subversion/tests/cmdline/upgrade_tests.py   (revision 1520363)
> >> +++ subversion/tests/cmdline/upgrade_tests.py   (working copy)
> >> @@ -425,7 +425,7 @@ def simple_entries_replace(path, from_url,
> to_url)
> >>  def basic_upgrade_1_0(sbox):
> >>    "test upgrading a working copy created with 1.0.0"
> >>
> >> -  sbox.build(create_wc = False)
> >> +  sbox.build(name='foo+', create_wc = False)
> >>    replace_sbox_with_tarfile(sbox, 'upgrade_1_0.tar.bz2')
> >>
> >>    url = sbox.repo_url
> >>
> >> % ../runpytest upgrade basic_upgrade_1_0 2>&1 | head
> >> W: lt-svn: subversion/libsvn_subr/path.c:119: svn_path_join_internal:
> Assertion `svn_path_is_canonical_internal(base, pool)' failed.
> >>
> >> W: CMD: /home/danielsh/src/svn/t1/subversion/svn/svn upgrade 'svn-
> test-work/working_copies/foo+' --config-dir
> /home/danielsh/src/svn/t1/subversion/tests/cmdline/svn-test-
> work/local_tmp/config --password rayjandom --no-auth-cache --username
> jrandom terminated by signal 6
> >
> > It looks like the %2B is failing svn_uri_is_canonical.  Looking at
> > svn_uri__char_validity in libsvn_subr/path.c the table says that '+'
> > should not be % encoded.  The problem is that the Python testsuite uses
> > urllib.pathname2url(self.repo_dir) and that has different ideas, it
> > does % encode '+'.
> 
> Index: subversion/tests/cmdline/svntest/sandbox.py
> ==========================================================
> =========
> --- subversion/tests/cmdline/svntest/sandbox.py	(revision 1520259)
> +++ subversion/tests/cmdline/svntest/sandbox.py	(working copy)
> @@ -57,7 +57,7 @@
>      if not read_only:
>        self.repo_dir = os.path.join(svntest.main.general_repo_dir,
self.name)
>        self.repo_url = (svntest.main.options.test_area_url + '/'
> -                       + urllib.pathname2url(self.repo_dir))
> +                       + urllib.quote(self.repo_dir, '/+'))
>        self.add_test_path(self.repo_dir)
>      else:
>        self.repo_dir = svntest.main.pristine_greek_repos_dir
> 
> That works for '+'.  A real patch would need to ensure that the safe set
> passed to urllib.quote matches the safe set required by Subversion.

How does this path enter 'svn'?

Shouldn't this just be handled by the 'svn_uri_canonicalize()' that handles
all url commandline arguments?

(Or does this code introduce it in the raw 'entries' files before the
conversion?)

	Bert


Re: upgrade_tests 7 FAIL when working directory has a "+" character

Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:

> Daniel Shahaf <da...@apache.org> writes:
>
>> On Wed, Sep 04, 2013 at 11:50:49PM -0400, James McCoy wrote:
>>> This was with svn 1.7.9.  I haven't had a chance to test newer
>>> versions, but looking through the code in trunk, it looks like it
>>> would still be a problem.
>>
>> Confirmed, it reproduces in trunk:
>>
>> Index: subversion/tests/cmdline/upgrade_tests.py
>> ===================================================================
>> --- subversion/tests/cmdline/upgrade_tests.py   (revision 1520363)
>> +++ subversion/tests/cmdline/upgrade_tests.py   (working copy)
>> @@ -425,7 +425,7 @@ def simple_entries_replace(path, from_url, to_url)
>>  def basic_upgrade_1_0(sbox):
>>    "test upgrading a working copy created with 1.0.0"
>>  
>> -  sbox.build(create_wc = False)
>> +  sbox.build(name='foo+', create_wc = False)
>>    replace_sbox_with_tarfile(sbox, 'upgrade_1_0.tar.bz2')
>>  
>>    url = sbox.repo_url
>>
>> % ../runpytest upgrade basic_upgrade_1_0 2>&1 | head
>> W: lt-svn: subversion/libsvn_subr/path.c:119: svn_path_join_internal: Assertion `svn_path_is_canonical_internal(base, pool)' failed.
>>
>> W: CMD: /home/danielsh/src/svn/t1/subversion/svn/svn upgrade 'svn-test-work/working_copies/foo+' --config-dir /home/danielsh/src/svn/t1/subversion/tests/cmdline/svn-test-work/local_tmp/config --password rayjandom --no-auth-cache --username jrandom terminated by signal 6
>
> It looks like the %2B is failing svn_uri_is_canonical.  Looking at
> svn_uri__char_validity in libsvn_subr/path.c the table says that '+'
> should not be % encoded.  The problem is that the Python testsuite uses
> urllib.pathname2url(self.repo_dir) and that has different ideas, it
> does % encode '+'.

Index: subversion/tests/cmdline/svntest/sandbox.py
===================================================================
--- subversion/tests/cmdline/svntest/sandbox.py	(revision 1520259)
+++ subversion/tests/cmdline/svntest/sandbox.py	(working copy)
@@ -57,7 +57,7 @@
     if not read_only:
       self.repo_dir = os.path.join(svntest.main.general_repo_dir, self.name)
       self.repo_url = (svntest.main.options.test_area_url + '/'
-                       + urllib.pathname2url(self.repo_dir))
+                       + urllib.quote(self.repo_dir, '/+'))
       self.add_test_path(self.repo_dir)
     else:
       self.repo_dir = svntest.main.pristine_greek_repos_dir

That works for '+'.  A real patch would need to ensure that the safe set
passed to urllib.quote matches the safe set required by Subversion.

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*

Re: upgrade_tests 7 FAIL when working directory has a "+" character

Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@apache.org> writes:

> On Wed, Sep 04, 2013 at 11:50:49PM -0400, James McCoy wrote:
>> This was with svn 1.7.9.  I haven't had a chance to test newer
>> versions, but looking through the code in trunk, it looks like it
>> would still be a problem.
>
> Confirmed, it reproduces in trunk:
>
> Index: subversion/tests/cmdline/upgrade_tests.py
> ===================================================================
> --- subversion/tests/cmdline/upgrade_tests.py   (revision 1520363)
> +++ subversion/tests/cmdline/upgrade_tests.py   (working copy)
> @@ -425,7 +425,7 @@ def simple_entries_replace(path, from_url, to_url)
>  def basic_upgrade_1_0(sbox):
>    "test upgrading a working copy created with 1.0.0"
>  
> -  sbox.build(create_wc = False)
> +  sbox.build(name='foo+', create_wc = False)
>    replace_sbox_with_tarfile(sbox, 'upgrade_1_0.tar.bz2')
>  
>    url = sbox.repo_url
>
> % ../runpytest upgrade basic_upgrade_1_0 2>&1 | head
> W: lt-svn: subversion/libsvn_subr/path.c:119: svn_path_join_internal: Assertion `svn_path_is_canonical_internal(base, pool)' failed.
>
> W: CMD: /home/danielsh/src/svn/t1/subversion/svn/svn upgrade 'svn-test-work/working_copies/foo+' --config-dir /home/danielsh/src/svn/t1/subversion/tests/cmdline/svn-test-work/local_tmp/config --password rayjandom --no-auth-cache --username jrandom terminated by signal 6
>

#0  0x00007ffff72cc475 in *__GI_raise (sig=<optimized out>)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x00007ffff72cf6f0 in *__GI_abort () at abort.c:92
#2  0x00007ffff72c5621 in *__GI___assert_fail (
    assertion=0x7ffff7d49210 "svn_path_is_canonical_internal(base, pool)", 
    file=<optimized out>, line=119, 
    function=0x7ffff7d498f0 "svn_path_join_internal") at assert.c:81
#3  0x00007ffff7d14a2e in svn_path_join_internal (
    base=0x7ffff3bc4bc0 "file:///home/pm/sw/subversion/obj/subversion/tests/cmdline/svn-test-work/repositories/foo%2B/trunk", component=0x7ffff3bc56d0 "iota", 
    pool=0x7ffff7e3e028) at ../src/subversion/libsvn_subr/path.c:119
#4  0x00007ffff7d16770 in svn_path_url_add_component2 (
    url=0x7ffff3bc4bc0 "file:///home/pm/sw/subversion/obj/subversion/tests/cmdline/svn-test-work/repositories/foo%2B/trunk", component=0x7ffff3bc56d0 "iota", 
    pool=0x7ffff7e3e028) at ../src/subversion/libsvn_subr/path.c:1085

It looks like the %2B is failing svn_uri_is_canonical.  Looking at
svn_uri__char_validity in libsvn_subr/path.c the table says that '+'
should not be % encoded.  The problem is that the Python testsuite uses
urllib.pathname2url(self.repo_dir) and that has different ideas, it
does % encode '+'.

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*

Re: upgrade_tests 7 FAIL when working directory has a "+" character

Posted by Daniel Shahaf <da...@apache.org>.
On Wed, Sep 04, 2013 at 11:50:49PM -0400, James McCoy wrote:
> This was with svn 1.7.9.  I haven't had a chance to test newer
> versions, but looking through the code in trunk, it looks like it
> would still be a problem.

Confirmed, it reproduces in trunk:

Index: subversion/tests/cmdline/upgrade_tests.py
===================================================================
--- subversion/tests/cmdline/upgrade_tests.py   (revision 1520363)
+++ subversion/tests/cmdline/upgrade_tests.py   (working copy)
@@ -425,7 +425,7 @@ def simple_entries_replace(path, from_url, to_url)
 def basic_upgrade_1_0(sbox):
   "test upgrading a working copy created with 1.0.0"
 
-  sbox.build(create_wc = False)
+  sbox.build(name='foo+', create_wc = False)
   replace_sbox_with_tarfile(sbox, 'upgrade_1_0.tar.bz2')
 
   url = sbox.repo_url

% ../runpytest upgrade basic_upgrade_1_0 2>&1 | head
W: lt-svn: subversion/libsvn_subr/path.c:119: svn_path_join_internal: Assertion `svn_path_is_canonical_internal(base, pool)' failed.

W: CMD: /home/danielsh/src/svn/t1/subversion/svn/svn upgrade 'svn-test-work/working_copies/foo+' --config-dir /home/danielsh/src/svn/t1/subversion/tests/cmdline/svn-test-work/local_tmp/config --password rayjandom --no-auth-cache --username jrandom terminated by signal 6