Re: [PATCH] gitweb: fix wrong base URL when non-root DirectoryIndex

[Jakub Narebski <jnareb@gmail.com>]

On Sat, 14 Feb 2009, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>>> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
>>
>> Sounds good. I don't use gitweb as DirectoryIndex myself, but
>> Acked-by: Jakub Narebski <jnareb@gmail.com>
>>
>>> +# Another issue with the script being the DirectoryIndex is that the resulting
>>> +# $my_url data is not the full script URL: this is good, because we want
>>> +# generated links to keep implying the script name if it wasn't explicitly
>>> +# indicated in the URL we're handling, but it means that $my_url cannot be used
>>> +# as base URL. Therefore, we have to build the base URL ourselves:
>>> +our $base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'};
> 
> Breaks t9500 with 
> 
>     [Sat Feb 14 02:12:59 2009] gitweb.perl: Use of uninitialized value in
>     concatenation (.) or string at /pub/git/t/../gitweb/gitweb.perl line 45. 
> 
> Please be more careful when giving an Ack, and more importantly please do
> not send a patch that does not even pass the test suite by itself.

Actually this is not a bug in _gitweb_, but in _test_ itself. 

In t/t9500-gitweb-standalone-no-errors.sh we run gitweb.perl as
a standalone script (not from a web server), and we set _some_ of CGI
environmental variables.  Up till now we could get by using only most
important ones: GATEWAY_INTERFACE (I'm not sure if needed), HTTP_ACCEPT
(used to select Content-Type to use), REQUEST_METHOD (git_feed exits
early on HEAD request), and of course QUERY_STRING and PATH_INFO.
The required variable SCRIPT_NAME is simply not set...

BTW. we could have set also SERVER_NAME, but it is not required by
gitweb (gitweb can deal with situation when it is unset)...

So patch should be squashed with the following improvement to test
suite:

-- >8 --

Additionally t/t9500-gitweb-standalone-no-errors.sh had to be modified
to set SCRIPT_NAME variable (CGI standard states that it MUST be set,
and now gitweb requires it if PATH_INFO is not empty, as is the case
for some of tests in t9500).

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 t/t9500-gitweb-standalone-no-errors.sh |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 43cd6ee..7c6f70b 100755
--- i/t/t9500-gitweb-standalone-no-errors.sh
+++ w/t/t9500-gitweb-standalone-no-errors.sh
@@ -43,9 +43,11 @@ gitweb_run () {
 	GATEWAY_INTERFACE="CGI/1.1"
 	HTTP_ACCEPT="*/*"
 	REQUEST_METHOD="GET"
+	SCRIPT_NAME="$TEST_DIRECTORY/../gitweb/gitweb.perl"
 	QUERY_STRING=""$1""
 	PATH_INFO=""$2""
-	export GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD QUERY_STRING PATH_INFO
+	export GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD \
+		SCRIPT_NAME QUERY_STRING PATH_INFO
 
 	GITWEB_CONFIG=$(pwd)/gitweb_config.perl
 	export GITWEB_CONFIG
@@ -54,7 +56,7 @@ gitweb_run () {
 	# written to web server logs, so we are not interested in that:
 	# we are interested only in properly formatted errors/warnings
 	rm -f gitweb.log &&
-	perl -- "$TEST_DIRECTORY/../gitweb/gitweb.perl" \
+	perl -- "$SCRIPT_NAME" \
 		>/dev/null 2>gitweb.log &&
 	if grep "^[[]" gitweb.log >/dev/null 2>&1; then false; else true; fi
 

-- 
Jakub Narebski
Poland