Re: [PATCHv3] gitweb: make static files accessible with PATH_INFO

From: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Date: Jan 28, 2009, 00:43 UTC
Message-ID: <cb7bb73a0901271643g7c3a8c42qff6025187ab3c081@mail.gmail.com>
In-Reply-To: <200901280114.59388.jnareb@gmail.com>

On Wed, Jan 28, 2009 at 1:14 AM, Jakub Narebski <jnareb@gmail.com> wrote:

Show 23 quoted lines

> On Tue, 27 Jan 2009, Giuseppe Bilotta wrote:
>
>> When PATH_INFO is defined, static files such as the defalt CSS or the
>                                                      default
>
>> shortcut icon are not accessible beyond the summary page (e.g. in
>> shortlog or commit view).
>>
>> Fix this by adding a <base> tag pointing to the script base URL.
>
> By the way, I have thought that it would conflict with use path_info
> for 'blob_plain' action to have links work in document... but I forgot
> that then we do not use gitweb HTML header...
>
>>
>> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
>> ---
>> Of course, last time I forgot that the BASE href is supposed to be
>> absolute. While Opera apparently has no problem with it being relative,
>> other browsers such as Firefox are stricter about it.
>
> Errrr... I think you are talking about _full_ vs. _absolute_, not
> _absolute_ vs. _relative_, see below.

No, I actually mean absolute vs relative in the URI sense, not in the Perl/CGI sense.

http://www.example.com/ is absolute, / is relative

Show 14 quoted lines

>>  gitweb/gitweb.perl |    8 ++++++++
>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 931db4f..411b1f6 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -2901,6 +2901,14 @@ sub git_header_html {
>>  <meta name="robots" content="index, nofollow"/>
>>  <title>$title</title>
>>  EOF
>> +# the stylesheet, favicon etc urls won't work correctly with path_info unless we set the appropriate base URL
>
> Errr... could you please break this line to not have it overly long?

Ah yes, sorry.

Show 33 quoted lines

>> +     if ($ENV{'PATH_INFO'}) {
>> +             my $base = $my_url;
>
> Hmmm...
>
>  our $my_url = $cgi->url(); # = $cgi->url(-full);
>  our $my_uri = $cgi->url(-absolute => 1);
>> +             my $sname = $ENV{'SCRIPT_NAME'};
>> +             $base =~ s,\Q$sname\E$,,;
>> +             $base .= "/";
>
> I don't think that is required; neither of $my_url and $my_uri ends
> with '/' after stripping path info:
>
>  our $path_info = $ENV{"PATH_INFO"};
>  if ($path_info) {
>        $my_url =~ s,\Q$path_info\E$,,;
>        $my_uri =~ s,\Q$path_info\E$,,;
>  }
>
> and if BASE is a document, then relative URLs are resolved using
> dirname of BASE, I guess, as
>  http://www.w3.org/TR/html401/struct/links.html#edef-BASE
> contains in example:
>  <BASE href="http://www.aviary.com/products/intro.html">
>
> See also RFC1808 (Relative Uniform Resource Locators), section
> 4.  Resolving Relative URLs:
>
>   Step 6: The last segment of the base URL's path (anything
>           following the rightmost slash "/", or the entire path if no
>           slash is present) is removed and the embedded URL's path is
>           appended in its place.[...]

Ah, good point, I had missed this part, so we can actually keep the script name in the url. Good.

Show 5 quoted lines

> Besides, if you strip SCRIPT_NAME, then you are left with document
> root; this means that if git-logo.png etc. are in the same directory
> as gitweb.cgi, they won't be found.  For example for me it doesn't
> work correctly (I have git-logo.png along gitweb.cgi, which is in
> /cgi-bin/gitweb/... and thanks to symlinks also in /gitweb/).

They won't be found only if you have gitweb.cgi as directory index. In that case you obviously need a rewrite rule at the server level anyway. The one I use is

RewriteEngine On RewriteCond %{REQUEST_FILENAME} !-f RewriteRule ^.* gitweb.cgi/$0 [L,PT]

tha means 'server static files and turn anything else into pathinfo for gitweb.cgi'

We probably want to document that. Notice that it's an issue regardless of this particular patch.

> By the way, according to documentation $cgi->url() should *not*
> contain path_info; you have to use $cgi->url(-path_info=>1) for
> that... strange.

I think there's a bug in CGI.pm when the script is the directory index.

Show 5 quoted lines

>> +             print "<base href=\"$base\"/>\n";
>
> Just in case, to be compatible with both XHML and HTML, we should use
>
> +               print "<base href=\"$base\" />\n";

> ...if not for the fact that surrounding code doesn't use this way...

That's the reason why I kept it that way, yes.

-- 
Giuseppe "Oblomov" Bilotta