Uploaded image for project: 'Subversion'
  1. Subversion
  2. SVN-4421

diff --summarize of a child of a newly added child causes crash

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.7.x
    • Fix Version/s: 1.7.14
    • Component/s: libsvn_ra_neon
    • Labels:
      None

      Description

      svnadmin create repo
      svn co $REPO_URL wc
      svn mkdir --parents wc/a/b
      svn ci -mm wc
      svn diff --summarize -c 1 $REPO_URL/a/b
      
      $REPO_URL must be a http:// URL (file:// and svn:// work fine).
      
      I have confirmed that it doesn't matter if we use neon or serf http-libraries or 1.7.7, 1.7.13 and 1.8.3 servers in this case.
      
      Based on what I've found so far while the crash is triggered by the fix for issue #4408 we made but it's actually caused by a 
      different issue that's specific to just http.  
      
      For instance using a 1.7.7 client (which doesn't have the issue #4408 fix) we'll see:
      
      $ svn diff --summarize -c 1 file:///Users/breser/summarize-crash/repo/a/b
      A       file:///Users/breser/summarize-crash/repo/a/b/a/b
      
      $ svn diff --summarize -c 1 svn://127.0.0.1/a/b
      A       svn://127.0.0.1/a/b/a/b
      
      $ svn diff --summarize -c 1 http://127.0.0.1:8081/svn/a/b
      A       http://127.0.0.1:8081/svn/a/b/b
      
      Note how in the http case only b is repeated and in the other cases the a/b is repeated.  What's happening here is because 
      neither a or its child b exist in one side of the diff the diff has to be rooted higher up until it finds a path that exists on both 
      sides, in this case the root of the repository.
      
      While the diff is rooted higher we still only ask for the diff of the changes of the target the user requested.  When we alter the 
      root so that it no longer matches the users target, we receive the part of the path contained in the users target as part of the path 
      being reported for changes.  If we don't handle this we end up with duplicate path segments.  This was fixed by the changes 
      made for issue #4408.
      
      We fixed issue #4408 by skipping the common ancestor of the users target and the reported path.  However, in the above case 
      for some reason rather than reporting the full path under our expected root (the repository root) we're just getting b.
      
      Since "b" is not a ancestor of "a/b" our skip_ancestor call returns NULL and we end up with the path in the diff summary being set 
      to NULL.  This results in a crash when the command line client code tries to produce the path that will be outputed (i.e. concating 
      the users target URL).
      
      Based on the above I believe this to be an independent bug.  It just wasn't obvious until we fixed the other issue.
      
      This works properly with a 1.8.x client because the new implementation of the diff --summarize feature does not have this issue.
      

        Attachments

          Activity

            People

            • Assignee:
              breser Ben Reser
              Reporter:
              breser Ben Reser
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: