626: post-review: svn wrapper inappropriately escapes some filenames in diffs

timw.a******@gmai***** (Google Code) (Is this you? Claim this profile.)
Feb. 17, 2010
979 
What's the URL of the page containing the problem?


What steps will reproduce the problem?
1. Ensure you have a file with spaces in your repository
2. Change that file and use post-review to post a diff of that file (after
applying the fix from issue 625)
3. Attempt to view the diff in RB

What is the expected output?
The diff should be viewable.

What do you see instead?
The file '/repo/some%20file' (r1) could not be found in the repository:
File not found: revision 1, path '/repo/some%20file'

What operating system are you using? What browser?
Linux (Ubuntu 8.04 32-bit), Firefox 3.0

Please provide any additional information below.
The diff contains paths that are URL-escaped. 'svn diff' does not produce
such paths, and the RB svn diff parsing code does not handle such paths.

The fix can be done one or two different ways:
1. fix post-review to not URL-escape svn diffs (necessary for compatibility
with patch)
2. fix RB's svn diff parser to check for already-escaped paths and leave
them alone (instead of re-escaping them)

These fixes (and the attached patches) are orthogonal and were not tested
against malicious input.
server-approach.patch 
Index: scmtools/svn.py
===================================================================
--- scmtools/svn.py	(revision 1477)
+++ scmtools/svn.py	(working copy)
@@ -77,18 +77,26 @@
 
         try:
             normpath = self.__normalize_path(path)
+            normrev  = self.__normalize_revision(revision)
 
             # SVN expects to have URLs escaped. Take care to only
             # escape the path part of the URL.
             if self.client.is_url(normpath):
+                old_normpath = normpath
                 pathtuple = urlparse.urlsplit(normpath)
                 normpath = urlparse.urlunsplit((pathtuple[0],
                                                 pathtuple[1],
                                                 urllib.quote(pathtuple[2]),
                                                 '',''))
+                try:
+                    self.client.propget("svn:keywords", normpath, normrev)
+                except ClientError, e:
+                    # HACK: if
postreview-unescape.patch 
Index: contrib/tools/post-review
===================================================================
--- contrib/tools/post-review	(revision 1477)
+++ contrib/tools/post-review	(working copy)
@@ -13,6 +13,7 @@
 from optparse import OptionParser
 from tempfile import mkstemp
 from urlparse import urljoin, urlparse
+import urllib
 
 
 ###
@@ -710,7 +711,7 @@
                     info = self.svn_info(file)
                     url  = info["URL"]
                     root = info["Repository Root"]
-                    path = url[len(root):]
+                    path = urllib.unquote(url[len(root):])
 
                     line = front + " " + path + rest
david
#1 david
  • +Component-RBTools
    +Component-DiffParser
david
#2 david
david
#3 david 
This has been fixed.
  • +Fixed