3558: RB doesn't handle paths containing spaces correctly
- Fixed
- Review Board
| matthia********@gmai***** (Google Code) (Is this you? Claim this profile.) | |
| Oct. 23, 2014 |
What version are you running? Reviewboard 2.0.6 RBTools 0.6.2 What's the URL of the page containing the problem? http://<host>/reviewboard/r/<requestnr>/diff/ What steps will reproduce the problem? 1. Have a git Repository with paths containing spaces. 2. Generate a post-commit review request using rbt post 3. View the review requests diff What is the expected output? File list and header of individual filediffs show full path. What do you see instead? In the file list I see the part of the path _after_ the space as path and beyond that the part of the path before the space, smaller, grey and italic, prepended by "Was ". Seems, a path containing one space character is interpreted as a rename. In the header of an individual filediff, only the part before the (probably first) space is shown. What operating system are you using? What browser? Reviewboard and RBTools run on an up-to-date Ubuntu 12.04. I tested Chrome 37 (stable) and Firefox 32.0. Please provide any additional information below. Apparently, the path is already wrong when entered into the database. In the corresponding entries in diffviewer_filediff, the part of the path before the space is in the column source_file and the part after the space in the column dest_file. As a result, the source_revision entries are the same for every file in that set of file_diffs. They seem to be the hash of the first part of the path, which of course is not a git object. Also I only tested paths containing at most one space.
Determination of source revision doesn't always fail, as you can see in rb02.png. Weird.
-
+ 
+ 
-
- NeedInfo + New -
+ EasyFix + Component-SCMTools
Hi everyone,
I've been looking into this issue quickly,
The problem comes from this code :
diff_line = self.lines[linenum].split()
try:
# Need to remove the "a/" and "b/" prefix
file_info.origFile = GIT_DIFF_PREFIX.sub(b"", diff_line[-2])
file_info.newFile = GIT_DIFF_PREFIX.sub(b"", diff_line[-1])
Parsing this line :
diff --git a/File name with spaces.pdf b/File name with spaces.pdf
This will result in origFile = "with" and newFile = "spaces.pdf"
I was thinking about doing something like using regex
/a(.*)/b
/b(.*)$
But if you path looks like 'somefolder b/your file', this will fail again.
Other solution is to update file_info.origFile & file_info.newFile with regex as you read the file
Binary files /dev/null and b/File name with spaces.pdf differ <= 'Binary files /dev/null and b/(.*) differ'
But this mean adding code (with regex) for every condition in
if self._is_new_file(linenum):
elif self._is_deleted_file(linenum):
elif self._is_mode_change(linenum):
elif self._is_moved_file(linenum):
elif self._is_copied_file(linenum):
If anyone thinks of a better way to deal with that, please let me know :)