3558: RB doesn't handle paths containing spaces correctly

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?

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.
#1 chipx86
Can you attach a screenshot for this?
  • +NeedInfo
#2 matthia********@gmai***** (Google Code) (Is this you? Claim this profile.)
Determination of source revision doesn't always fail, as you can see in rb02.png. Weird.
#3 david
  • -NeedInfo
  • +EasyFix
#4 gheaec*******@gmai***** (Google Code) (Is this you? Claim this profile.)
Hi everyone,

I've been looking into this issue quickly,
The problem comes from this code : 

        diff_line = self.lines[linenum].split()

            # 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 


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 :)
#5 david
Fixed in release-2.0.x (de3bbf7). This will ship in 2.0.10
  • -New