subvertpy wrapper orders paths incorrectly and crashes when pushing

Issue #461 new
matthew hussey
created an issue

Hi,

When pushing changes to subversion I was getting a file not found crash within subvertpy. The problem appears to be caused by the path traversal approach used.

If I have the following directory structure:

  • MyApplication/
    • MyApplication.sln
    • MyApplication/
      • MyApplication.csproj

This is a strange directory stucture because you have a solution file alongside a directory with the same name, but when using Visual Studio and a solution with only one project this is common, and the default configuration.

The problem happens when MyApplication.sln and MyApplication.csproj are both edited in one revision. At line 417 of subvertpy_wrapper.py the paths of modified files are collected together, their parent directories collected and then sorted. However, the sorting order is wrong and causes the visitdir function to fail.

The sorted order is:

[
    'MyApplication',
    'MyApplication/MyApplication',
    'MyApplication/MyApplication.sln',
    'MyApplication/MyApplication/MyApplication.csproj'
]

This means that the Directory Editor for MyApplication is active and then moves to the Directory Editor for MyApplication/MyApplication.

The next visitdir looks at MyApplication/MyApplication.sln and because it is not within MyApplication/MyApplication it returns with the index to MyApplication.

Then MyApplication/MyApplication/MyApplication.csproj is looked at in the context of the MyApplication Directory Editor, which is one level up from where it should be.

This then causes svn to try to open the file ...svn/MyApplication/MyApplication.csproj instead of svn/MyApplication/MyApplication/MyApplication.csproj.

The problem seems to stem from the sorted function putting '/' after '.' in its sort. The correct sort should be:

[
    'MyApplication',
    'MyApplication/MyApplication',
    'MyApplication/MyApplication/MyApplication.csproj',
    'MyApplication/MyApplication.sln'
]

but I imagine that there are likely to be other quirks too.

My config: Windows 7 Professional. Hg 4.0.1, SVN 1.9.5

Comments (5)

  1. Augie Fackler repo owner

    What version of Subversion do you have on the server? I suspect this is a corner case that's tripping up some assertion about editor drive ordering in either the svn client or server and things are exploding.

    I wish they hadn't relaxed this and thought it was backwards compatible.

  2. matthew hussey reporter

    I have no idea. It's not my server and I have no control over it. The SVN client on my PC is the latest version.

    I think it's the ordering of the sort and the way that the subvertpy wrapper iterates through them rather than a problem with SVN but I'm not certain. When the wrapper processes the MyApplication.csproj file it is against a directory editor that is looking at the parent directory and therefore fails to find the file. I can fix this by hardcoding the order in your wrapper code but obviously that is not a proper work around. I am currently trying to replicate this using hgsubversion on a sourceforge svn repo.

  3. matthew hussey reporter

    Adding a custom sort that makes sure that directories are ordered then the files within them is so far working for me. I've hacked it in and only tested in partially but it seems to be working so far. I thought I'd share incase it may help.

    from collections import namedtuple
    from os.path import dirname, isdir, normpath
    
    DirectoryRecord = namedtuple('DirectoryRecord', ['original_name', 'files'])
    
    def sorted_paths(paths):
        directories, files = [], []
        [directories.append(path) if isdir(path) else files.append(path) for path in paths]
    
        dir_map = dict([(normpath(directory), DirectoryRecord(directory, [])) for directory in directories])
        [dir_map[normpath(dirname(file))].files.append(file) for file in files]
    
        ordered_paths = []
        for directory in sorted(dir_map.keys()):
            ordered_paths.append(dir_map[directory].original_name)
            ordered_paths.extend(sorted(dir_map[directory].files))
    
        return ordered_paths
    

    in the subvertypy_wrapper:

        def commit(self, paths, message, file_data, base_revision, addeddirs,
                   deleteddirs, props, copies):
            """Commits the appropriate targets from revision in editor's store.
    
            Return the committed revision as a common.Revision instance.
            """
            def commitcb(rev, date, author):
                r = common.Revision(rev, author, message, date)
                committedrev.append(r)
    
            committedrev = []
            revprops = { properties.PROP_REVISION_LOG: message }
            # revprops.update(props)
            commiteditor = self.remote.get_commit_editor(revprops, commitcb)
    
            paths = set(paths)
            paths.update(addeddirs)
            paths.update(deleteddirs)
    
            # ensure that all parents are visited too; this may be slow
            for path in paths.copy():
                for i in xrange(path.count('/'), -1, -1):
                    p = path.rsplit('/', i)[0]
                    if p in paths:
                        continue
                    paths.add(p)
            paths = sorted_paths(paths)
    

    The final line is the only change.

  4. Augie Fackler repo owner

    That actually looks correct to my eyes, and if it fixes your problem, can I get you to commit that and contribute it? I'll even entertain a pull request for this, since it's so critical and I'd like to make the process as easy for you as possible.

    Thanks!

  5. Log in to comment