Edgewall Software

Opened 5 years ago

Closed 5 years ago

#12274 closed defect (fixed)

Trap exceptions in Subversion backend when revision number is negative

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.0.10
Component: version control Version:
Severity: normal Keywords: svn
Cc: Branch:
Release Notes:

SubversionException from invalid revision number is trapped and a NoSuchChangeset exception is raised.

API Changes:
Internal Changes:


Invalid revisions such as a are trapped in the Subversion backend and a NoSuchChangeset exception is raised. However a negative revision number is not trapped and the SubversionException propagates. We should trap the exception and raise a NoSuchChangeset exception.

20:47:11 Trac[formatter] ERROR: Macro Include(source:proj1/workflow@-1) failed:
Traceback (most recent call last):
  File "/Users/rjollos/Documents/Workspace/th-dev/IncludeMacro/pve/lib/python2.7/site-packages/trac/wiki/formatter.py", line 789, in _macro_formatter
    return macro.ensure_inline(macro.process(args))
  File "/Users/rjollos/Documents/Workspace/th-dev/IncludeMacro/pve/lib/python2.7/site-packages/trac/wiki/formatter.py", line 358, in process
    text = self.processor(text)
  File "/Users/rjollos/Documents/Workspace/th-dev/IncludeMacro/pve/lib/python2.7/site-packages/trac/wiki/formatter.py", line 345, in _macro_processor
  File "/Users/rjollos/Documents/Workspace/th-dev/IncludeMacro/trunk/includemacro/macros.py", line 130, in expand_macro
  File "/Users/rjollos/Documents/Workspace/th-dev/IncludeMacro/trunk/includemacro/macros.py", line 202, in _get_source
    node = repos.get_node(path, rev)
  File "/Users/rjollos/Documents/Workspace/th-dev/IncludeMacro/pve/lib/python2.7/site-packages/trac/versioncontrol/cache.py", line 298, in get_node
    return self.repos.get_node(path, self.normalize_rev(rev))
  File "/Users/rjollos/Documents/Workspace/th-dev/IncludeMacro/pve/lib/python2.7/site-packages/tracopt/versioncontrol/svn/svn_fs.py", line 511, in get_node
    return SubversionNode(path, rev, self, self.pool)
  File "/Users/rjollos/Documents/Workspace/th-dev/IncludeMacro/pve/lib/python2.7/site-packages/tracopt/versioncontrol/svn/svn_fs.py", line 768, in __init__
    self.root = fs.revision_root(self.fs_ptr, rev, pool)
  File "/Users/rjollos/Documents/Workspace/th-dev/IncludeMacro/pve/lib/python2.7/site-packages/libsvn/fs.py", line 319, in svn_fs_revision_root
    return _fs.svn_fs_revision_root(*args)
SubversionException: 160006 - Invalid revision number '-1'

Attachments (0)

Change History (4)

comment:1 by Ryan J Ollos, 5 years ago

Release Notes: modified (diff)

Fixed on 1.0-stable in [14454] , merged to trunk in [14455].

comment:2 by Jun Omae, 5 years ago

I think we should raise NoSuchChangeset/NoSuchNode rather than TracError from SubversionRepository.get_node and SubversionNode.__init__.

  • tracopt/versioncontrol/svn/svn_fs.py

    diff --git a/tracopt/versioncontrol/svn/svn_fs.py b/tracopt/versioncontrol/svn/svn_fs.py
    index ed20a356d..6d0b7e756 100644
    a b class SvnCachedRepository(CachedRepository):  
    258258    def rev_db(self, rev):
    259259        return int(rev or 0)
     261    def normalize_rev(self, rev):
     262        return self.repos.normalize_rev(rev)
    262265class SubversionConnector(Component):
    class SubversionRepository(Repository):  
    429432        else:
    430433            try:
    431434                rev = int(rev)
    432                 if rev <= self.youngest_rev:
     435                if 0 <= rev <= self.youngest_rev:
    433436                    return rev
    434437            except (ValueError, TypeError):
    435438                pass
    class SubversionNode(Node):  
    768771            try:
    769772                self.root = fs.revision_root(self.fs_ptr, rev, pool)
    770773            except core.SubversionException, e:
    771                 raise TracError(e)
     774                raise NoSuchNode(path, rev, exception_to_unicode(e))
    772775        node_type = fs.check_path(self.root, self._scoped_path_utf8, pool)
    773776        if not node_type in _kindmap:
    774777            raise NoSuchNode(path, rev)
  • tracopt/versioncontrol/svn/tests/svn_fs.py

    diff --git a/tracopt/versioncontrol/svn/tests/svn_fs.py b/tracopt/versioncontrol/svn/tests/svn_fs.py
    index 4b69e7fea..d1484aec8 100644
    a b class NormalTests(object):  
    130130        self.assertEqual(HEAD, self.repos.normalize_rev(None))
    131131        self.assertEqual(11, self.repos.normalize_rev('11'))
    132132        self.assertEqual(11, self.repos.normalize_rev(11))
     133        self.assertRaises(NoSuchChangeset, self.repos.normalize_rev, -1)
     134        self.assertRaises(NoSuchChangeset, self.repos.normalize_rev, -42)
    134136    def test_rev_navigation(self):
    135137        self.assertEqual(1, self.repos.oldest_rev)
    class NormalTests(object):  
    169171        self.assertEqual(HEAD, node.created_rev)
    170172        self.assertEqual(datetime(2015, 6, 15, 14, 9, 13, 664490, utc),
    171173                         node.last_modified)
    172         self.assertRaises(TracError, self.repos.get_node, u'/', -1)
     174        self.assertRaises(NoSuchChangeset, self.repos.get_node, u'/', -1)
    173175        node = self.repos.get_node(u'/tête')
    174176        self.assertEqual(u'tête', node.name)
    175177        self.assertEqual(u'/tête', node.path)

comment:3 by Ryan J Ollos, 5 years ago

Thanks, I should have thought to use the NoSuchChangeset exception. I'll push the changes later today.

comment:4 by Ryan J Ollos, 5 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

comment:2 changes committed in [14469,14471], merged in [14470,14472].

Modify Ticket

Change Properties
Set your email in Preferences
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment

E-mail address and name can be saved in the Preferences .
Note: See TracTickets for help on using tickets.