Project

General

Profile

Actions

Defect #6054

closed

Error 500 on filenames with whitespace in git reposities

Added by Georg Lukas over 14 years ago. Updated about 14 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
SCM
Target version:
Start date:
2010-08-06
Due date:
% Done:

100%

Estimated time:
Resolution:
Fixed
Affected version:

Description

Getting a directory listing from GIT crashes Redmine with an internal server error when a file has whitespace in its name:

NoMethodError (private method `split' called for nil:NilClass):
    /lib/redmine/scm/adapters/git_adapter.rb:93:in `lastrev'
    /lib/redmine/scm/adapters/abstract_adapter.rb:193:in `call'
    /lib/redmine/scm/adapters/abstract_adapter.rb:193:in `shellout'
    /lib/redmine/scm/adapters/abstract_adapter.rb:191:in `popen'
    /lib/redmine/scm/adapters/abstract_adapter.rb:191:in `shellout'
    /lib/redmine/scm/adapters/abstract_adapter.rb:177:in `shellout'
    /lib/redmine/scm/adapters/git_adapter.rb:92:in `lastrev'
    /lib/redmine/scm/adapters/git_adapter.rb:77:in `entries'
    /lib/redmine/scm/adapters/git_adapter.rb:65:in `each_line'
    /lib/redmine/scm/adapters/git_adapter.rb:65:in `entries'
    ....

This is caused by inadequate quoting of path names in GitAdapter.lastrev(), which affects at least 0.8.4, but seems unfixed in 1.0rc and trunk as well. A patch fixing this issue is attached.


Files

redmine-allow-filenames-with-spaces-on-git.patch (983 Bytes) redmine-allow-filenames-with-spaces-on-git.patch Patch to fix shell quoting of filenames in GitAdapter.lastrev() Georg Lukas, 2010-08-06 11:20
Actions #1

Updated by Felix Schäfer over 14 years ago

Can't reproduce on trunk, see here http://orga.fachschaften.org/projects/sandbox-git/repository

What git version do you have installed?

Actions #2

Updated by Georg Lukas over 14 years ago

Sorry Felix, your link requires a login so I can't test. Do you have filenames with whitespace or repository path/name with whitespace? My problem only occured with the former.

I'm running Debian stable with:

$ git --version
git version 1.5.6.5

Actions #3

Updated by Felix Schäfer over 14 years ago

Sorry, I thought it was public (which it should have been), it is public now.

I have a file whitespace filename.txt in the repo, git version string is git version 1.7.1.

Actions #4

Updated by Georg Lukas over 14 years ago

Ah, now I see that it does not crash Redmine any more. However it does not display the revision and change information for that file. If you add another file without whitespace, I am pretty sure you will see its revision, author etc. columns. Could you please try to do so and compare the output with/without my patch?

The version of git on the other hand should not matter - it is solely an issue with file name quoting.

Actions #5

Updated by Felix Schäfer over 14 years ago

  • Affected version (unused) changed from 0.8.4 to devel
  • Affected version deleted (0.8.4)

Ah, I now see what you mean and can confirm the bug on trunk.

Could you see if other places in the git adapter are concerned and maybe even write a test for that? Thanks!

Actions #6

Updated by Georg Lukas over 14 years ago

From a quick glance the path quoting problem seems to affect GitAdapter.revisions() as well. Other subtle but bad things will probably happen in GitAdapter.entries() and GitAdapter.revisions() when file names start with whitespace.

The adapter might also freak out badly if the repository directory contains spaces in its path - I see no quoting at all for target('').

Regarding the test, I fear I can not help you much - I have exactly zero coding experience in Ruby.

Actions #7

Updated by Felix Schäfer over 14 years ago

  • Target version set to 1.0.2

This commit fixes the missing shell_quote s in the git_adapter: http://github.com/thegcat/redmine/commit/5d5c60d5d4d09aca61239f269fa8904328d9952e The tests in this commit will only work with the fixes in #6346.

jb grenot: could you commit both to trunk? Thanks.

Actions #8

Updated by Eric Davis about 14 years ago

  • Status changed from New to Resolved
  • Assignee set to Eric Davis
  • % Done changed from 0 to 100
  • Resolution set to Fixed

Committed in r4188. I found another issue if the file has leading or trailing whitespace, Felix said he will try to work on a fix (#6499).

Actions #9

Updated by Eric Davis about 14 years ago

  • Status changed from Resolved to Closed

Merged into 1.0-stable

Actions

Also available in: Atom PDF