You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Alexander Kitaev <Al...@svnkit.com> on 2008/05/18 13:46:18 UTC

1.5.x segmentation fault on Repos to Repos copy

Hello,

The following command fails with segmentation fault:

svn cp --parents SRC_FILE_URL DST_DIR_URL -m "message"

SRC_FILE_URL - existing file
DST_DIR_URL - existing directory

Omitting "--parents" option makes above copy operation work as expected.

Bug is in libsvn_client/copy.c:801, where "dir" should be checked for 
null before using it in svn_ra_check_path call.

-- 
Alexander Kitaev,
TMate Software,
http://svnkit.com/ - Java [Sub]Versioning Library!

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.5.x segmentation fault on Repos to Repos copy

Posted by Karl Fogel <kf...@red-bean.com>.
I wrote:
> Before trying to reproduce with the script below, I wrote a regression
> test for this (see http://pastebin.ca/1021930).  When the test
> unexpectedly passed the first time, I wrote the script below...

I forgot that pastebin.ca doesn't do forever storage anymore.  Here's
the regression test patch, just so we don't lose it:

[[[
* subversion/tests/cmdline/copy_tests.py
  (unneeded_parents): New test.
  (test_list): Run it.
]]]

Index: subversion/tests/cmdline/copy_tests.py
===================================================================
--- subversion/tests/cmdline/copy_tests.py	(revision 31288)
+++ subversion/tests/cmdline/copy_tests.py	(working copy)
@@ -3819,6 +3819,57 @@
     svntest.main.safe_rmtree(other_wc_dir)
 
 
+def unneeded_parents(sbox):
+  "svn cp --parents FILE_URL DIR_URL"
+
+  # In this message...
+  #
+  #    http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=138738
+  #    From: Alexander Kitaev <Al...@svnkit.com>
+  #    To: dev@subversion.tigris.org
+  #    Subject: 1.5.x segmentation fault on Repos to Repos copy
+  #    Message-ID: <48...@svnkit.com>
+  #    Date: Sun, 18 May 2008 15:46:18 +0200
+  #
+  # ...Alexander Kitaev describes the bug:
+  #
+  #    svn cp --parents SRC_FILE_URL DST_DIR_URL -m "message"
+  #
+  #    SRC_FILE_URL - existing file
+  #    DST_DIR_URL - existing directory
+  # 
+  #    Omitting "--parents" option makes above copy operation work as
+  #    expected.
+  # 
+  #    Bug is in libsvn_client/copy.c:801, where "dir" should be
+  #    checked for null before using it in svn_ra_check_path call.
+  
+  sbox.build()
+  wc_dir = sbox.wc_dir
+
+  iota_url = sbox.repo_url + '/iota'
+  B_url = sbox.repo_url + '/A/B/'
+
+  # The --parents is unnecessary, but should still work (not segfault).
+  svntest.actions.run_and_verify_svn(None, None, [], 'cp', '--parents',
+                                     '-m', 'log msg', iota_url, B_url)
+
+  # Verify that it worked.
+  expected_output = svntest.wc.State(wc_dir, {
+    'A/B/iota' : Item(status='A '),
+    })
+  expected_disk = svntest.main.greek_state.copy()
+  expected_disk.add({
+    'A/B/iota'  : Item(contents="This is the file 'iota'.\n"),
+    })
+  expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
+  expected_status.add({
+    'A/B/iota'  : Item(status='  ', wc_rev=2),
+    })
+  svntest.actions.run_and_verify_update(
+    wc_dir, expected_output, expected_disk, expected_status)
+
+
 ########################################################################
 # Run the tests
 
@@ -3895,6 +3946,7 @@
               URI_encoded_repos_to_wc,
               allow_unversioned_parent_for_copy_src,
               replaced_local_source_for_incoming_copy,
+              unneeded_parents,
              ]
 
 if __name__ == '__main__':


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.5.x segmentation fault on Repos to Repos copy

Posted by Karl Fogel <kf...@red-bean.com>.
Alexander Kitaev <Al...@svnkit.com> writes:
> Here is the reproduction script (slightly modified version of yours):

Thank you.  Regression test committed in r31311, fix to follow shortly.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.5.x segmentation fault on Repos to Repos copy

Posted by Alexander Kitaev <Al...@svnkit.com>.
Hello Karl,

Here is the reproduction script (slightly modified version of yours):

#!/bin/sh

SVNDIR=/home/builder/usr/svn/bin/
SVN=${SVNDIR}/svn
SVNSERVE=${SVNDIR}/svnserve
SVNADMIN=${SVNDIR}/svnadmin

URL=file:///`pwd`/repos

rm -rf repos wc import-me

${SVNADMIN} create repos

echo "### Making a Greek Tree for import..."
mkdir import-me
mkdir import-me/src
mkdir import-me/dst
echo "This is the file 'file.txt'."        > import-me/src/file.txt
echo ""
echo "### Importing it..."
(cd import-me; ${SVN} import -q -m "Initial import." ${URL})
echo "### Done."
echo ""

${SVN} copy --parents ${URL}/src/file.txt ${URL}/dst -m "log msg"
echo "### File copied"
${SVN} ls -vR ${URL}/

And its output (Subversion from 1.5.x branch, revision 31307):

### Making a Greek Tree for import...

### Importing it...
### Done.

./test.sh: line 44:  5223 Segmentation fault      ${SVN} copy --parents 
${URL}/src/file.txt ${URL}/dst -m "log msg"
### File copied
       1 alex                  May 20 15:17 ./
       1 alex                  May 20 15:17 dst/
       1 alex                  May 20 15:17 src/
       1 alex               29 May 20 15:17 src/file.txt

Looks like there is one more condition to reproduce the problem - dst 
URL should has no more segments count than source one.

Alexander Kitaev,
TMate Software,
http://svnkit.com/ - Java [Sub]Versioning Library!

Karl Fogel wrote:
> Karl Fogel <kf...@red-bean.com> writes:
>> Alexander Kitaev <Al...@svnkit.com> writes:
>>> The following command fails with segmentation fault:
>>>
>>> [...]
>> Could you please give a formal reproduction recipe (that is, a script)?
>> I was unable to reproduce this over any RA layer, using the script
>> below.  
> 
> By the way, I was working with r31288 of trunk.
> 
> -Karl
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.5.x segmentation fault on Repos to Repos copy

Posted by Karl Fogel <kf...@red-bean.com>.
Karl Fogel <kf...@red-bean.com> writes:
> Alexander Kitaev <Al...@svnkit.com> writes:
>> The following command fails with segmentation fault:
>>
>> [...]
>
> Could you please give a formal reproduction recipe (that is, a script)?
> I was unable to reproduce this over any RA layer, using the script
> below.  

By the way, I was working with r31288 of trunk.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.5.x segmentation fault on Repos to Repos copy

Posted by Karl Fogel <kf...@red-bean.com>.
Alexander Kitaev <Al...@svnkit.com> writes:
> The following command fails with segmentation fault:
>
> svn cp --parents SRC_FILE_URL DST_DIR_URL -m "message"
>
> SRC_FILE_URL - existing file
> DST_DIR_URL - existing directory
>
> Omitting "--parents" option makes above copy operation work as expected.
>
> Bug is in libsvn_client/copy.c:801, where "dir" should be checked for
> null before using it in svn_ra_check_path call.

Could you please give a formal reproduction recipe (that is, a script)?
I was unable to reproduce this over any RA layer, using the script
below.  I agree this code in libsvn_client/copy.c looks suspicious:

   dir = svn_path_is_child(top_url, svn_path_dirname(pair->dst, pool),
                           pool);
   SVN_ERR(svn_ra_check_path(ra_session, dir, SVN_INVALID_REVNUM, &kind,
                             iterpool));

Nevertheless, the bug did not occur for me.

Before trying to reproduce with the script below, I wrote a regression
test for this (see http://pastebin.ca/1021930).  When the test
unexpectedly passed the first time, I wrote the script below.

---------------------------------------------------------------------
#!/bin/sh

# The next line is the only line you should need to adjust.
SVNDIR=/home/kfogel/src/subversion

SVN=${SVNDIR}/subversion/svn/svn
SVNSERVE=${SVNDIR}/subversion/svnserve/svnserve
SVNADMIN=${SVNDIR}/subversion/svnadmin/svnadmin

# Select an access method.  If svn://, the svnserve setup is
# handled automagically by this script; but if http://, then
# you'll have to configure it yourself first.
# 
# URL=http://localhost/unneeded-parents/repos
# URL=svn://localhost/repos
URL=file:///`pwd`/repos

rm -rf repos wc import-me

${SVNADMIN} create repos

# These are for svnserve only.
echo "[general]" > repos/conf/svnserve.conf
echo "anon-access = write" >> repos/conf/svnserve.conf
echo "auth-access = write" >> repos/conf/svnserve.conf

# The server will only be contacted if $URL is svn://foo, of course.
${SVNSERVE} --pid-file svnserve-pid -d -r `pwd`
# And put the kill command in a file, in case need to run it manually.
echo "kill -9 `cat svnserve-pid`" > k
chmod a+rwx k

echo "### Making a Greek Tree for import..."
mkdir import-me
mkdir import-me/trunk
mkdir import-me/tags
mkdir import-me/branches
mkdir import-me/trunk/A
mkdir import-me/trunk/A/B/
mkdir import-me/trunk/A/C/
mkdir import-me/trunk/A/D/
mkdir import-me/trunk/A/B/E/
mkdir import-me/trunk/A/B/F/
mkdir import-me/trunk/A/D/G/
mkdir import-me/trunk/A/D/H/
echo "This is the file 'iota'."        > import-me/trunk/iota
echo "This is the file 'A/mu'."        > import-me/trunk/A/mu
echo "This is the file 'A/B/lambda'."  > import-me/trunk/A/B/lambda
echo "This is the file 'A/B/E/alpha'." > import-me/trunk/A/B/E/alpha
echo "This is the file 'A/B/E/beta'."  > import-me/trunk/A/B/E/beta
echo "This is the file 'A/D/gamma'."   > import-me/trunk/A/D/gamma
echo "This is the file 'A/D/G/pi'."    > import-me/trunk/A/D/G/pi
echo "This is the file 'A/D/G/rho'."   > import-me/trunk/A/D/G/rho
echo "This is the file 'A/D/G/tau'."   > import-me/trunk/A/D/G/tau
echo "This is the file 'A/D/H/chi'."   > import-me/trunk/A/D/H/chi
echo "This is the file 'A/D/H/omega'." > import-me/trunk/A/D/H/omega
echo "This is the file 'A/D/H/psi'."   > import-me/trunk/A/D/H/psi
echo "### Done."
echo ""
echo "### Importing it..."
(cd import-me; ${SVN} import -q -m "Initial import." ${URL})
echo "### Done."
echo ""

${SVN} copy --parents ${URL}/trunk/iota ${URL}/trunk/A/B/iota -m "log msg"
echo ""
${SVN} ls -vR ${URL}/

# Put kill command in a file, in case need to run it manually.
echo "kill -9 `cat svnserve-pid`" > k
chmod a+rwx k
./k

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org