On 05.09.2013 21:20, Philip Martin wrote: > 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.
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. br...@wandisco.com
