Philip Martin <philip.mar...@wandisco.com> writes:
> Daniel Shahaf <danie...@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*
