342744 - bz_locations should return absolute paths for mod_perl

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Max Kanat-Alexander]

During every time except checksetup, bz_locations should be able to use File::Spec to return absolute paths for all of its variables, instead of relative paths.

Why is this important?

Well, mod_perl doesn't actually know what directory it's working under. It's possible to hack it to do a chdir($0), but that will cause other problems in the future (for example, being able to only run one mod_perl Bugzilla per machine, when otherwise it would be possible to run two. Also, Bugzilla would be the *only* thing you could run in mod_perl, which would suck.)

This can be handled by never using relative paths in any CGI.

Comment 1

[Max Kanat-Alexander]

Attachment: v1

Okay, I figured out a very clever way to do this.

Under mod_perl, you specify what we used to call "libdir" inside of the httpd.conf. I'll show you this later, when I write the documentation.

Basically, this makes the paths absolute when we're in mod_perl, and relative when we're in mod_cgi.

I also fixed all the comments, now that we're actually using mod_perl.

Comment 2

[Max Kanat-Alexander]

Attachment: v2

Okay, this version actually works. It will be a tiny bit slower, but I don't think anybody will notice.

The previous version didn't work because "use lib" only gets called once per httpd child. So sometimes $INC[0] would be '.' and sometimes $INC[0] would be '/var/www/html/mod_perl'. That was breaking the mod_perl installation.

This one I've tested and it seems to work, although I've run into one bug. (I think the bug might be unrelated to this patch, though, since this patch does exactly what it's supposed to do.)

Comment 3

[Max Kanat-Alexander]

Attachment: v3

Okay, I finally came up with the most reliable way to do this. This works at compile time, at runtime, under mod_perl, and under mod_cgi. It works if we're inside of a module, or inside of a CGI.

Comment 4

[Frédéric Buclin]

Comment on attachment 227107 [details] [diff] [review]
v3


>+    my $libpath = abs_path(dirname($INC{'Bugzilla/Constants.pm'}) . '/..');

When symlinks are used (which is the case for my installations), abs_path() returns the real path. I don't know if that could be a problem or not. I prefer to let justdave or someone else review it.

Comment 5

[Dave Miller [:justdave]]

I don't like tacking /.. on the end, that just feels like a hack.  How about running dirname() on it twice or something?

Comment 6

[Max Kanat-Alexander]

Attachment: v4

Okay, now we just call dirname() twice and we leave out abs_path. The abs_path call could theoretically slow things down, and it's not actually needed since the correct path will always be in %INC.

Also, we no longer need to detaint, because %INC isn't tainted.

One thing is that I'd like to see this tested on Mac OS X and make sure that it works. File::Basename has some comments in it that seem to imply that what we're doing might not work on MacOS. (But it probably means the old MacOS, not Mac OS X.)

Here's the command-line script I use to test this:

#!/usr/bin/perl -wT
use strict;
use lib '.';
use Bugzilla::Constants;
use Data::Dumper;
print Dumper(bz_locations());

I just need somebody to run that on MacOS X with this patch applied and make sure that "Bugzilla/" doesn't show up in the paths that get printed out.

Comment 7

[Frédéric Buclin]

Comment on attachment 227550 [details] [diff] [review]
v4


>+    # On mod_cgi this will be a relative path. On mod_perl it will be an
>+    # absolute path.
>+    my $libpath = dirname(dirname($INC{'Bugzilla/Constants.pm'}));

Where in the spec is it written that it will return an absolute path when using mod_perl? And what kind of absolute path is returned when using symlinks?

Comment 8

[Max Kanat-Alexander]

(In reply to comment #7)
> Where in the spec is it written that it will return an absolute path when using
> mod_perl? And what kind of absolute path is returned when using symlinks?

  Okay, to understand this you have to understand what a mod_perl configuration file looks like for Bugzilla. One of the lines is:

PerlSwitches -I/var/www/html/mod_perl -w -T

  As you can see, there's the -I/var/www/html/mod_perl

  By definition, if we're inside Bugzilla::Constants, Bugzilla::Constants has been loaded into %INC. This means that whatever the path to Bugzilla/Constants.pm is, inside %INC, that's the *correct path* to the Bugzilla libraries.

  If you have a symlink in there, who cares? It doesn't matter at all. It will load it using the symlink path. It's just using the path that we specified in the -I line.

Comment 9

[Frédéric Buclin]

Comment on attachment 227550 [details] [diff] [review]
v4

Works when not using mod_perl, i.e. $libpath is correctly set to '.'. But I cannot test this patch using mod_perl. I trust mkanat for that. r=LpSolit

Comment 10

[Dave Miller [:justdave]]

Comment on attachment 227550 [details] [diff] [review]
v4

The only thing I worry about with this is what happens to the "use lib qw(.);" in all of the files when we're running under mod_perl?  What is the current directory at that point, and what happens if someone figures that out and drops a Bugzilla/* named module (or any other perl module we use for that matter) in that path?

But that's just thinking out loud, and something we might want to look at down the line...  I suspect if you have enough privs to set up mod_perl you have enough privs to sufficiently lock your system down otherwise. :)

Comment 11

[Max Kanat-Alexander]

(In reply to comment #10)
> (From update of attachment 227550 [details] [diff] [review] [edit])
> The only thing I worry about with this is what happens to the "use lib qw(.);"
> in all of the files when we're running under mod_perl? 

  The "use lib" is only effective the first time the page loads in that httpd child. After that @INC is reset.

> What is the current directory at that point,

  It seems to be the root directory, I think.

> and what happens if someone figures that out and drops
> a Bugzilla/* named module (or any other perl module we use for that matter) in
> that path?

  If it was a module that we pre-loaded at startup, then nothing will happen, because Apache only loads a module once. :-) Otherwise yes, there's a chance that Apache could load the wrong module.

> But that's just thinking out loud, and something we might want to look at down
> the line...  I suspect if you have enough privs to set up mod_perl you have
> enough privs to sufficiently lock your system down otherwise. :)

  Yeah, I would think so too. :-)

Comment 12

[Max Kanat-Alexander]

Checking in Bugzilla/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v  <--  Constants.pm
new revision: 1.42; previous revision: 1.41
done

Comment 13

[Max Kanat-Alexander]

Added to the release notes on bug 255155.

Comment 14

[Max Kanat-Alexander]

The correct bug number for those release notes is actually bug 349423.