Summary | SVN integration is not robust and has poor error reporting |
Queue | Horde Framework Packages |
Queue Version | HEAD |
Type | Bug |
State | Resolved |
Priority | 1. Low |
Owners | |
Requester | meyer (at) mesw (dot) de |
Created | 02/28/2007 (6656 days ago) |
Due | |
Updated | 03/29/2007 (6627 days ago) |
Assigned | 02/28/2007 (6656 days ago) |
Resolved | 03/29/2007 (6627 days ago) |
Milestone | |
Patch | No |
State ⇒ Resolved
New Attachment: chora_new_setting.patch
to the user.
New Attachment: vc_new_setting.patch
State ⇒ Feedback
requesting is, that instead of reusing the cvsps_home path, introduce
a new configuration variable in Chora, and for backward compatibility
reasons fall back to Util::getTempDir() if the value is not set.
Priority ⇒ 1. Low
Type ⇒ Bug
Summary ⇒ SVN integration is not robust and has poor error reporting
Queue ⇒ Horde Framework Packages
New Attachment: vc.patch
State ⇒ Unconfirmed
installs Chora and configures it to use SVN but the content of the
repositories are not shown, yet there are no error messages and no
messages in the logs and there does not seem to be an apparent
misconfiguration.
The attached patch tries to improve this situation by making the
following changes:
(1)
In VC::getPath(), the path of the SVN (or any other) binary is only
returned if php determines it is executable. This was introduced
without explanation in log message or comment in version 1.7:
http://cvs.horde.org/diff.php?r1=1.6&r2=1.7&f=framework%2FVC%2FVC.php
This is wrong for two reasons:
First, when a PHP basedir restriction to the htdocs/ directory is
active (common on many webservers), PHP's built-in function
"is_executable" cannot access files in, say, /usr/bin although it can
execute this file. Infact, the only way to see if the file can be
executed is to try to execute it. Thus, the use of "is_executable"
here forces the user to extend the basedir restriction to /usr/bin,
which is a bad idea from a security point of view.
Second, the error return value "false" is never checked within
"svn.php" and therefore wrong commands will be executed. E.g. when the
main module list of the SVN repository is supposed to be shown, Chora
will build a string of the form "/usr/bin/svn ls /path/to/repository".
If "/usr/bin/svn" is not found, "ls /path/to/repository" will be
executed instead. This will succeed (=> no error message), but the
returned output will not match the expected output (=> modules list
will be empty). Thus, the user has no chance to find out what is wrong
than to debug the code.
Therefore the patch restores the code which was present in rev 1.6 of
VC.php. This will always try to execute the SVN binary. If the SVN
binary is not found, a sensible error message will be shown.
(2)
A common configuration on web servers (e.g. the default configuration
on SUSE 9.x) is that the apache process runs as root but the web pages
are handled by the "wwwroot" user which has low privileges. Since the
apache process is started as root, its HOME dir will point to "/root".
In this case, svn, when started by the "wwwroot" user will look for
the presence of "/root/.svn" and fail because it obviously is denied
access. This behaviour can be prevented by explicitely setting the
"--config-dir" directive to a directory the current user has access to
(say, something like "/tmp"). The attached patch automates this
behaviour.