{"thread":{"id":"17761","subject":"[PATCH] gitweb: fix wrong base URL when non-root DirectoryIndex","startedAt":"2009-02-12T21:11:12Z","lastAt":"2009-02-15T19:33:55Z","messageCount":13,"participants":["Giuseppe Bilotta","Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"104423","messageId":"1234473072-9901-1-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17761","inReplyTo":null,"subject":"[PATCH] gitweb: fix wrong base URL when non-root DirectoryIndex","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-12T21:11:12Z","receivedAt":"2009-02-12T21:11:12Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"From: Giuseppe Bilotta \n\nCGI::url() has some issues when rebuilding the script URL if the script\nis a DirectoryIndex.\n\nOne of these issue is the inability to strip PATH_INFO, which is why we\nhad to do it ourselves.\n\nAnother issue is that the resulting URL cannot be used for the \ntag: it works if we're the DirectoryIndex at the root level, but not\notherwise.\n\nWe fix this by building the proper base URL ourselves, and improve the\ndocumentation about the need to strip PATH_INFO manually while we're at\nit.\n---\n gitweb/gitweb.perl | 16 ++++++++++++----\n 1 files changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex c2c8080..48cfd5f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -27,15 +27,23 @@ our $version = \"++GIT_VERSION++\";\n our $my_url = $cgi->url();\n our $my_uri = $cgi->url(-absolute => 1);\n \n-# if we're called with PATH_INFO, we have to strip that\n-# from the URL to find our real URL\n-# we make $path_info global because it's also used later on\n+# When the script is used as DirectoryIndex, the URL does not contain the name\n+# of the script file itself, and $cgi->url() fails to strip PATH_INFO, so we\n+# have to do it ourselves. We make $path_info global because it's also used\n+# later on\n our $path_info = $ENV{\"PATH_INFO\"};\n if ($path_info) {\n \t$my_url =~ s,\\Q$path_info\\E$,,;\n \t$my_uri =~ s,\\Q$path_info\\E$,,;\n }\n \n+# Another issue with the script being the DirectoryIndex is that the resulting\n+# $my_url data is not the full script URL: this is good, because we want\n+# generated links to keep implying the script name if it wasn't explicitly\n+# indicated in the URL we're handling, but it means that $my_url cannot be used\n+# as base URL. Therefore, we have to build the base URL ourselves:\n+our $base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'};\n+\n # core git executable to use\n # this can just be \"git\" if your webserver has a sensible PATH\n our $GIT = \"++GIT_BINDIR++/git\";\n@@ -2908,7 +2916,7 @@ EOF\n \t# the stylesheet, favicon etc urls won't work correctly with path_info\n \t# unless we set the appropriate base URL\n \tif ($ENV{'PATH_INFO'}) {\n-\t\tprint \"\\n\";\n+\t\tprint \"\\n\";\n \t}\n \t# print out each stylesheet that exist, providing backwards capability\n \t# for those people who defined $stylesheet in a config file\n-- \n1.5.6.4\n"},{"id":"104439","messageId":"200902122303.37499.jnareb@gmail.com","threadId":"17761","inReplyTo":"1234473072-9901-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH] gitweb: fix wrong base URL when non-root DirectoryIndex","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-12T22:03:36Z","receivedAt":"2009-02-12T22:03:36Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 12 Feb 2009, Giuseppe Bilotta wrote:\n\n> From: Giuseppe Bilotta \n\nWhy different email? Was it intended, or was it an accident, and\nauthorship of this patch should be to\n\n Giuseppe Bilotta \n\n> \n> CGI::url() has some issues when rebuilding the script URL if the script\n> is a DirectoryIndex.\n> \n> One of these issue is the inability to strip PATH_INFO, which is why we\n> had to do it ourselves.\n> \n> Another issue is that the resulting URL cannot be used for the \n> tag: it works if we're the DirectoryIndex at the root level, but not\n> otherwise.\n> \n> We fix this by building the proper base URL ourselves, and improve the\n> documentation about the need to strip PATH_INFO manually while we're at\n> it.\n\nVery good. The only thing I wonder about is if we should add this info\nalso to gitweb/README, or if it is good as it is now.\n\nSignoff?\n\nI do not use gitweb as DirectoryIndex, but it looks sane, so:\nAcked-by: Jakub Narebski \n\n> ---\n> gitweb/gitweb.perl | 16 ++++++++++++----\n> 1 files changed, 12 insertions(+), 4 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index c2c8080..48cfd5f 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -27,15 +27,23 @@ our $version = \"++GIT_VERSION++\";\n> our $my_url = $cgi->url();\n> our $my_uri = $cgi->url(-absolute => 1);\n> \n> -# if we're called with PATH_INFO, we have to strip that\n> -# from the URL to find our real URL\n> -# we make $path_info global because it's also used later on\n> +# When the script is used as DirectoryIndex, the URL does not contain the name\n> +# of the script file itself, and $cgi->url() fails to strip PATH_INFO, so we\n> +# have to do it ourselves. We make $path_info global because it's also used\n> +# later on\n> our $path_info = $ENV{\"PATH_INFO\"};\n> if ($path_info) {\n> \t$my_url =~ s,\\Q$path_info\\E$,,;\n> \t$my_uri =~ s,\\Q$path_info\\E$,,;\n> }\n> \n> +# Another issue with the script being the DirectoryIndex is that the resulting\n> +# $my_url data is not the full script URL: this is good, because we want\n> +# generated links to keep implying the script name if it wasn't explicitly\n> +# indicated in the URL we're handling, but it means that $my_url cannot be used\n> +# as base URL. Therefore, we have to build the base URL ourselves:\n> +our $base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'};\n> +\n> # core git executable to use\n> # this can just be \"git\" if your webserver has a sensible PATH\n> our $GIT = \"++GIT_BINDIR++/git\";\n> @@ -2908,7 +2916,7 @@ EOF\n> \t# the stylesheet, favicon etc urls won't work correctly with path_info\n> \t# unless we set the appropriate base URL\n> \tif ($ENV{'PATH_INFO'}) {\n> -\t\tprint \"\\n\";\n> +\t\tprint \"\\n\";\n> \t}\n> \t# print out each stylesheet that exist, providing backwards capability\n> \t# for those people who defined $stylesheet in a config file\n> -- \n> 1.5.6.4\n> \n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"104488","messageId":"cb7bb73a0902122336k1726698ftef85fd4ea0e25085@mail.gmail.com","threadId":"17761","inReplyTo":"200902122303.37499.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: fix wrong base URL when non-root DirectoryIndex","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-13T07:36:54Z","receivedAt":"2009-02-13T07:36:54Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Thu, Feb 12, 2009 at 11:03 PM, Jakub Narebski wrote:\n> On Thu, 12 Feb 2009, Giuseppe Bilotta wrote:\n>\n>> From: Giuseppe Bilotta \n>\n> Why different email? Was it intended, or was it an accident, and\n> authorship of this patch should be to\n>\n> Giuseppe Bilotta \n\nI created this patch on a different computer and forgot to set the\nauthorship as usual. I'll resend.\n\n> Very good. The only thing I wonder about is if we should add this info\n> also to gitweb/README, or if it is good as it is now.\n\nI think there is no need to adjust the README, as in fact this fix is\nneeded to make sure that the instructions in README are correct even\nfor subpaths.\n\n> Signoff?\n\nWill add that too.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"104490","messageId":"1234510803-23268-1-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17761","inReplyTo":"200902122303.37499.jnareb@gmail.com","subject":"[PATCH] gitweb: fix wrong base URL when non-root DirectoryIndex","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-13T07:40:03Z","receivedAt":"2009-02-13T07:40:03Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"CGI::url() has some issues when rebuilding the script URL if the script\nis a DirectoryIndex.\n\nOne of these issue is the inability to strip PATH_INFO, which is why we\nhad to do it ourselves.\n\nAnother issue is that the resulting URL cannot be used for the \ntag: it works if we're the DirectoryIndex at the root level, but not\notherwise.\n\nWe fix this by building the proper base URL ourselves, and improve the\ndocumentation about the need to strip PATH_INFO manually while we're at\nit.\n\nSigned-off-by: Giuseppe Bilotta \n---\n gitweb/gitweb.perl | 16 ++++++++++++----\n 1 files changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex c2c8080..48cfd5f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -27,15 +27,23 @@ our $version = \"++GIT_VERSION++\";\n our $my_url = $cgi->url();\n our $my_uri = $cgi->url(-absolute => 1);\n \n-# if we're called with PATH_INFO, we have to strip that\n-# from the URL to find our real URL\n-# we make $path_info global because it's also used later on\n+# When the script is used as DirectoryIndex, the URL does not contain the name\n+# of the script file itself, and $cgi->url() fails to strip PATH_INFO, so we\n+# have to do it ourselves. We make $path_info global because it's also used\n+# later on\n our $path_info = $ENV{\"PATH_INFO\"};\n if ($path_info) {\n \t$my_url =~ s,\\Q$path_info\\E$,,;\n \t$my_uri =~ s,\\Q$path_info\\E$,,;\n }\n \n+# Another issue with the script being the DirectoryIndex is that the resulting\n+# $my_url data is not the full script URL: this is good, because we want\n+# generated links to keep implying the script name if it wasn't explicitly\n+# indicated in the URL we're handling, but it means that $my_url cannot be used\n+# as base URL. Therefore, we have to build the base URL ourselves:\n+our $base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'};\n+\n # core git executable to use\n # this can just be \"git\" if your webserver has a sensible PATH\n our $GIT = \"++GIT_BINDIR++/git\";\n@@ -2908,7 +2916,7 @@ EOF\n \t# the stylesheet, favicon etc urls won't work correctly with path_info\n \t# unless we set the appropriate base URL\n \tif ($ENV{'PATH_INFO'}) {\n-\t\tprint \"\\n\";\n+\t\tprint \"\\n\";\n \t}\n \t# print out each stylesheet that exist, providing backwards capability\n \t# for those people who defined $stylesheet in a config file\n-- \n1.5.6.5\n"},{"id":"104495","messageId":"200902130945.20601.jnareb@gmail.com","threadId":"17761","inReplyTo":"1234510803-23268-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH] gitweb: fix wrong base URL when non-root DirectoryIndex","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-13T08:45:19Z","receivedAt":"2009-02-13T08:45:19Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, 13 Feb 2009, Giuseppe Bilotta wrote:\n\n> CGI::url() has some issues when rebuilding the script URL if the script\n> is a DirectoryIndex.\n> \n> One of these issue is the inability to strip PATH_INFO, which is why we\n> had to do it ourselves.\n> \n> Another issue is that the resulting URL cannot be used for the \n> tag: it works if we're the DirectoryIndex at the root level, but not\n> otherwise.\n> \n> We fix this by building the proper base URL ourselves, and improve the\n> documentation about the need to strip PATH_INFO manually while we're at\n> it.\n> \n> Signed-off-by: Giuseppe Bilotta \n\nSounds good. I don't use gitweb as DirectoryIndex myself, but\nAcked-by: Jakub Narebski \n\n> ---\n> gitweb/gitweb.perl | 16 ++++++++++++----\n> 1 files changed, 12 insertions(+), 4 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index c2c8080..48cfd5f 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -27,15 +27,23 @@ our $version = \"++GIT_VERSION++\";\n> our $my_url = $cgi->url();\n> our $my_uri = $cgi->url(-absolute => 1);\n> \n> -# if we're called with PATH_INFO, we have to strip that\n> -# from the URL to find our real URL\n> -# we make $path_info global because it's also used later on\n> +# When the script is used as DirectoryIndex, the URL does not contain the name\n> +# of the script file itself, and $cgi->url() fails to strip PATH_INFO, so we\n> +# have to do it ourselves. We make $path_info global because it's also used\n> +# later on\n> our $path_info = $ENV{\"PATH_INFO\"};\n> if ($path_info) {\n> \t$my_url =~ s,\\Q$path_info\\E$,,;\n> \t$my_uri =~ s,\\Q$path_info\\E$,,;\n> }\n> \n> +# Another issue with the script being the DirectoryIndex is that the resulting\n> +# $my_url data is not the full script URL: this is good, because we want\n> +# generated links to keep implying the script name if it wasn't explicitly\n> +# indicated in the URL we're handling, but it means that $my_url cannot be used\n> +# as base URL. Therefore, we have to build the base URL ourselves:\n> +our $base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'};\n> +\n> # core git executable to use\n> # this can just be \"git\" if your webserver has a sensible PATH\n> our $GIT = \"++GIT_BINDIR++/git\";\n> @@ -2908,7 +2916,7 @@ EOF\n> \t# the stylesheet, favicon etc urls won't work correctly with path_info\n> \t# unless we set the appropriate base URL\n> \tif ($ENV{'PATH_INFO'}) {\n> -\t\tprint \"\\n\";\n> +\t\tprint \"\\n\";\n> \t}\n> \t# print out each stylesheet that exist, providing backwards capability\n> \t# for those people who defined $stylesheet in a config file\n> -- \n> 1.5.6.5\n> \n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"104570","messageId":"7vskmh3gtv.fsf@gitster.siamese.dyndns.org","threadId":"17761","inReplyTo":"200902130945.20601.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: fix wrong base URL when non-root DirectoryIndex","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-14T02:14:36Z","receivedAt":"2009-02-14T02:14:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n>> Signed-off-by: Giuseppe Bilotta \n>\n> Sounds good. I don't use gitweb as DirectoryIndex myself, but\n> Acked-by: Jakub Narebski \n>\n>> +# Another issue with the script being the DirectoryIndex is that the resulting\n>> +# $my_url data is not the full script URL: this is good, because we want\n>> +# generated links to keep implying the script name if it wasn't explicitly\n>> +# indicated in the URL we're handling, but it means that $my_url cannot be used\n>> +# as base URL. Therefore, we have to build the base URL ourselves:\n>> +our $base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'};\n\nBreaks t9500 with \n\n [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.\n\nPlease be more careful when giving an Ack, and more importantly please do\nnot send a patch that does not even pass the test suite by itself.\n"},{"id":"104576","messageId":"200902140342.26270.jnareb@gmail.com","threadId":"17761","inReplyTo":"7vskmh3gtv.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] gitweb: fix wrong base URL when non-root DirectoryIndex","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-14T02:42:24Z","receivedAt":"2009-02-14T02:42:24Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 14 Feb 2009, Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n>>> Signed-off-by: Giuseppe Bilotta \n>>\n>> Sounds good. I don't use gitweb as DirectoryIndex myself, but\n>> Acked-by: Jakub Narebski \n>>\n>>> +# Another issue with the script being the DirectoryIndex is that the resulting\n>>> +# $my_url data is not the full script URL: this is good, because we want\n>>> +# generated links to keep implying the script name if it wasn't explicitly\n>>> +# indicated in the URL we're handling, but it means that $my_url cannot be used\n>>> +# as base URL. Therefore, we have to build the base URL ourselves:\n>>> +our $base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'};\n> \n> Breaks t9500 with \n> \n> [Sat Feb 14 02:12:59 2009] gitweb.perl: Use of uninitialized value in\n> concatenation (.) or string at /pub/git/t/../gitweb/gitweb.perl line 45. \n> \n> Please be more careful when giving an Ack, and more importantly please do\n> not send a patch that does not even pass the test suite by itself.\n\nActually this is not a bug in _gitweb_, but in _test_ itself. \n\nIn t/t9500-gitweb-standalone-no-errors.sh we run gitweb.perl as\na standalone script (not from a web server), and we set _some_ of CGI\nenvironmental variables. Up till now we could get by using only most\nimportant ones: GATEWAY_INTERFACE (I'm not sure if needed), HTTP_ACCEPT\n(used to select Content-Type to use), REQUEST_METHOD (git_feed exits\nearly on HEAD request), and of course QUERY_STRING and PATH_INFO.\nThe required variable SCRIPT_NAME is simply not set...\n\nBTW. we could have set also SERVER_NAME, but it is not required by\ngitweb (gitweb can deal with situation when it is unset)...\n\nSo patch should be squashed with the following improvement to test\nsuite:\n\n-- >8 --\n\nAdditionally t/t9500-gitweb-standalone-no-errors.sh had to be modified\nto set SCRIPT_NAME variable (CGI standard states that it MUST be set,\nand now gitweb requires it if PATH_INFO is not empty, as is the case\nfor some of tests in t9500).\n\nSigned-off-by: Jakub Narebski \n---\n t/t9500-gitweb-standalone-no-errors.sh | 6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex 43cd6ee..7c6f70b 100755\n--- i/t/t9500-gitweb-standalone-no-errors.sh\n+++ w/t/t9500-gitweb-standalone-no-errors.sh\n@@ -43,9 +43,11 @@ gitweb_run () {\n \tGATEWAY_INTERFACE=\"CGI/1.1\"\n \tHTTP_ACCEPT=\"*/*\"\n \tREQUEST_METHOD=\"GET\"\n+\tSCRIPT_NAME=\"$TEST_DIRECTORY/../gitweb/gitweb.perl\"\n \tQUERY_STRING=\"\"$1\"\"\n \tPATH_INFO=\"\"$2\"\"\n-\texport GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD QUERY_STRING PATH_INFO\n+\texport GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD \\\n+\t\tSCRIPT_NAME QUERY_STRING PATH_INFO\n \n \tGITWEB_CONFIG=$(pwd)/gitweb_config.perl\n \texport GITWEB_CONFIG\n@@ -54,7 +56,7 @@ gitweb_run () {\n \t# written to web server logs, so we are not interested in that:\n \t# we are interested only in properly formatted errors/warnings\n \trm -f gitweb.log &&\n-\tperl -- \"$TEST_DIRECTORY/../gitweb/gitweb.perl\" \\\n+\tperl -- \"$SCRIPT_NAME\" \\\n \t\t>/dev/null 2>gitweb.log &&\n \tif grep \"^[[]\" gitweb.log >/dev/null 2>&1; then false; else true; fi\n \n\n-- \nJakub Narebski\nPoland\n"},{"id":"104613","messageId":"cb7bb73a0902140052h15fa60bfs8a0becab84c997c1@mail.gmail.com","threadId":"17761","inReplyTo":"200902140342.26270.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: fix wrong base URL when non-root DirectoryIndex","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-14T08:52:56Z","receivedAt":"2009-02-14T08:52:56Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Sat, Feb 14, 2009 at 3:42 AM, Jakub Narebski wrote:\n> On Sat, 14 Feb 2009, Junio C Hamano wrote:\n>> Jakub Narebski writes:\n>>\n>>>> Signed-off-by: Giuseppe Bilotta \n>>>\n>>> Sounds good. I don't use gitweb as DirectoryIndex myself, but\n>>> Acked-by: Jakub Narebski \n>>>\n>>>> +# Another issue with the script being the DirectoryIndex is that the resulting\n>>>> +# $my_url data is not the full script URL: this is good, because we want\n>>>> +# generated links to keep implying the script name if it wasn't explicitly\n>>>> +# indicated in the URL we're handling, but it means that $my_url cannot be used\n>>>> +# as base URL. Therefore, we have to build the base URL ourselves:\n>>>> +our $base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'};\n>>\n>> Breaks t9500 with\n>>\n>> [Sat Feb 14 02:12:59 2009] gitweb.perl: Use of uninitialized value in\n>> concatenation (.) or string at /pub/git/t/../gitweb/gitweb.perl line 45.\n>>\n>> Please be more careful when giving an Ack, and more importantly please do\n>> not send a patch that does not even pass the test suite by itself.\n\nSorry, my fault. For future reference for myself, is there a way to\nonly run the gitweb tests of the testsuite?\n\n> Actually this is not a bug in _gitweb_, but in _test_ itself.\n>\n> In t/t9500-gitweb-standalone-no-errors.sh we run gitweb.perl as\n> a standalone script (not from a web server), and we set _some_ of CGI\n> environmental variables. Up till now we could get by using only most\n> important ones: GATEWAY_INTERFACE (I'm not sure if needed), HTTP_ACCEPT\n> (used to select Content-Type to use), REQUEST_METHOD (git_feed exits\n> early on HEAD request), and of course QUERY_STRING and PATH_INFO.\n> The required variable SCRIPT_NAME is simply not set...\n\nThank you for the additional patch. One thing we should also probably\nadd to the test is the HTTP_IF_MODIFIED_SINCE. I will try to think of\na bunch of tests to run against feed production after I implement the\nreflog data check.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"104616","messageId":"7v63jdwekf.fsf@gitster.siamese.dyndns.org","threadId":"17761","inReplyTo":"cb7bb73a0902140052h15fa60bfs8a0becab84c997c1@mail.gmail.com","subject":"Re: [PATCH] gitweb: fix wrong base URL when non-root DirectoryIndex","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-14T09:30:40Z","receivedAt":"2009-02-14T09:30:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Giuseppe Bilotta writes:\n\n> For future reference for myself, is there a way to\n> only run the gitweb tests of the testsuite?\n\ncd t && sh ./t9500-gitweb-standalone-no-errors.sh [-i] [-v]\n\n> Thank you for the additional patch. One thing we should also probably\n> add to the test is the HTTP_IF_MODIFIED_SINCE. I will try to think of\n> a bunch of tests to run against feed production after I implement the\n> reflog data check.\n\nThanks.\n"},{"id":"104622","messageId":"200902141104.35042.jnareb@gmail.com","threadId":"17761","inReplyTo":"200902140342.26270.jnareb@gmail.com","subject":"[PATCH v3] gitweb: fix wrong base URL when non-root DirectoryIndex","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-14T10:04:34Z","receivedAt":"2009-02-14T10:04:34Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: Giuseppe Bilotta \n\nCGI::url() has some issues when rebuilding the script URL if the script\nis a DirectoryIndex.\n\nOne of these issue is the inability to strip PATH_INFO, which is why\nwe had to do it ourselves.\n\nAnother issue is that the resulting URL cannot be used for the \ntag: it works if we're the DirectoryIndex at the root level, but not\notherwise.\n\nWe fix this by building the proper base URL ourselves, and improve the\ncomment about the need to strip PATH_INFO manually while we're at it.\n\nAdditionally t/t9500-gitweb-standalone-no-errors.sh had to be modified\nto set SCRIPT_NAME variable (CGI standard states that it MUST be set,\nand now gitweb uses it if PATH_INFO is not empty, as is the case for\nsome of tests in t9500).\n\nSigned-off-by: Giuseppe Bilotta \nSigned-off-by: Jakub Narebski \n---\nOn Sat, 14 Feb 2009, Jakub Narebski wrote:\n\n> So patch should be squashed with the following improvement to test\n> suite:\n[...]\n\nHere it is. (And with [PATCH v3] :-)).\n\nAdditionally I tried to make it more robust, first by constructing\n$base_url _only_ when needed (when we strip PATH_INFO), and second\nby checking if SCRIPT_NAME is defined.\n\nPasses test (after its modification), but not testes itself!\n\nP.S. I wonder why Giuseppe's patch failed to apply, and failed\nto do fallback 3-way merge...\n\n Applying: gitweb: fix wrong base URL when non-root DirectoryIndex\n error: patch failed: gitweb/gitweb.perl:2908\n error: gitweb/gitweb.perl: patch does not apply\n fatal: sha1 information is lacking or useless (gitweb/gitweb.perl).\n Repository lacks necessary blobs to fall back on 3-way merge.\n Cannot fall back to three-way merge.\n Patch failed at 0001.\n\nOn top of v1.6.2-rc0-64-ge9cc02f, gitweb/gitweb.perl is 54108742...\n\n gitweb/gitweb.perl | 28 ++++++++++++++++++++++------\n t/t9500-gitweb-standalone-no-errors.sh | 6 ++++--\n 2 files changed, 26 insertions(+), 8 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 5410874..83c2ad7 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -27,13 +27,29 @@ our $version = \"++GIT_VERSION++\";\n our $my_url = $cgi->url();\n our $my_uri = $cgi->url(-absolute => 1);\n \n-# if we're called with PATH_INFO, we have to strip that\n-# from the URL to find our real URL\n-# we make $path_info global because it's also used later on\n+# Base URL for relative URLs in gitweb ($logo, $favicon, ...),\n+# needed and used only for URLs with nonempty PATH_INFO\n+our $base_url = $my_url;\n+\n+# When the script is used as DirectoryIndex, the URL does not contain the name\n+# of the script file itself, and $cgi->url() fails to strip PATH_INFO, so we\n+# have to do it ourselves. We make $path_info global because it's also used\n+# later on.\n+#\n+# Another issue with the script being the DirectoryIndex is that the resulting\n+# $my_url data is not the full script URL: this is good, because we want\n+# generated links to keep implying the script name if it wasn't explicitly\n+# indicated in the URL we're handling, but it means that $my_url cannot be used\n+# as base URL.\n+# Therefore, if we needed to strip PATH_INFO, then we know that we have\n+# to build the base URL ourselves:\n our $path_info = $ENV{\"PATH_INFO\"};\n if ($path_info) {\n-\t$my_url =~ s,\\Q$path_info\\E$,,;\n-\t$my_uri =~ s,\\Q$path_info\\E$,,;\n+\tif (($my_url =~ s,\\Q$path_info\\E$,, ||\n+\t $my_uri =~ s,\\Q$path_info\\E$,,) &&\n+\t defined $ENV{'SCRIPT_NAME'}) {\n+\t\t$base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'};\n+\t}\n }\n \n # core git executable to use\n@@ -2908,7 +2924,7 @@ EOF\n \t# the stylesheet, favicon etc urls won't work correctly with path_info\n \t# unless we set the appropriate base URL\n \tif ($ENV{'PATH_INFO'}) {\n-\t\tprint '\\n';\n+\t\tprint '\\n';\n \t}\n \t# print out each stylesheet that exist, providing backwards capability\n \t# for those people who defined $stylesheet in a config file\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex 43cd6ee..7c6f70b 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -43,9 +43,11 @@ gitweb_run () {\n \tGATEWAY_INTERFACE=\"CGI/1.1\"\n \tHTTP_ACCEPT=\"*/*\"\n \tREQUEST_METHOD=\"GET\"\n+\tSCRIPT_NAME=\"$TEST_DIRECTORY/../gitweb/gitweb.perl\"\n \tQUERY_STRING=\"\"$1\"\"\n \tPATH_INFO=\"\"$2\"\"\n-\texport GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD QUERY_STRING PATH_INFO\n+\texport GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD \\\n+\t\tSCRIPT_NAME QUERY_STRING PATH_INFO\n \n \tGITWEB_CONFIG=$(pwd)/gitweb_config.perl\n \texport GITWEB_CONFIG\n@@ -54,7 +56,7 @@ gitweb_run () {\n \t# written to web server logs, so we are not interested in that:\n \t# we are interested only in properly formatted errors/warnings\n \trm -f gitweb.log &&\n-\tperl -- \"$TEST_DIRECTORY/../gitweb/gitweb.perl\" \\\n+\tperl -- \"$SCRIPT_NAME\" \\\n \t\t>/dev/null 2>gitweb.log &&\n \tif grep \"^[[]\" gitweb.log >/dev/null 2>&1; then false; else true; fi\n \n-- \n1.6.1\n"},{"id":"104638","messageId":"cb7bb73a0902140629pa1ad038pe898aa0d804d0707@mail.gmail.com","threadId":"17761","inReplyTo":"200902141104.35042.jnareb@gmail.com","subject":"Re: [PATCH v3] gitweb: fix wrong base URL when non-root DirectoryIndex","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-14T14:29:49Z","receivedAt":"2009-02-14T14:29:49Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Sat, Feb 14, 2009 at 11:04 AM, Jakub Narebski wrote:\n>\n> P.S. I wonder why Giuseppe's patch failed to apply, and failed\n> to do fallback 3-way merge...\n\nBecause it depends on an additional quoting fix that I sent to the\nmailing list but that apparently didn't get through.\n\n> - print '\\n';\n> + print '\\n';\n\nThe quoting fix was here: double quotes are needed because otherwise\nthe \\n gets in literally. We can probably squash it with this patch.\nGimme a sec and I'll cook it up again.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"104750","messageId":"1234689516-14415-1-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17761","inReplyTo":"cb7bb73a0902140629pa1ad038pe898aa0d804d0707@mail.gmail.com","subject":"[PATCHv4] gitweb: fix wrong base URL when non-root DirectoryIndex","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-15T09:18:36Z","receivedAt":"2009-02-15T09:18:36Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"CGI::url() has some issues when rebuilding the script URL if the script\nis a DirectoryIndex.\n\nOne of these issue is the inability to strip PATH_INFO, which is why\nwe had to do it ourselves.\n\nAnother issue is that the resulting URL cannot be used for the \ntag: it works if we're the DirectoryIndex at the root level, but not\notherwise.\n\nWe fix this by building the proper base URL ourselves, and improve the\ncomment about the need to strip PATH_INFO manually while we're at it.\n\nAdditionally t/t9500-gitweb-standalone-no-errors.sh had to be modified\nto set SCRIPT_NAME variable (CGI standard states that it MUST be set,\nand now gitweb uses it if PATH_INFO is not empty, as is the case for\nsome of tests in t9500).\n\nSigned-off-by: Giuseppe Bilotta \nSigned-off-by: Jakub Narebski \n---\n\nThere was a bug in Jakub's rewritten logic for the PATH_INFO stripping:\nusing || instead of && meant that $my_uri was not being stripped if\n$my_url was, resulting in wrong relative links all around. Instead, if\nwe can strip PATH_INFO from $my_url then we can (and must) strip it from\n$my_uri too (&&), and then check for the definition of SCRIPT_NAME.\n\nThis patch also included a quoting patch that apparently didn't make it\nto the list: the tag was inserting a literal \\n instead of a\nnewline. Since that line got rewritten anyway, I thought it wasn't a big\ndeal to squash it in.\n\n gitweb/gitweb.perl | 28 ++++++++++++++++++++++------\n t/t9500-gitweb-standalone-no-errors.sh | 6 ++++--\n 2 files changed, 26 insertions(+), 8 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 8dffa3f..7c48181 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -27,13 +27,29 @@ our $version = \"++GIT_VERSION++\";\n our $my_url = $cgi->url();\n our $my_uri = $cgi->url(-absolute => 1);\n \n-# if we're called with PATH_INFO, we have to strip that\n-# from the URL to find our real URL\n-# we make $path_info global because it's also used later on\n+# Base URL for relative URLs in gitweb ($logo, $favicon, ...),\n+# needed and used only for URLs with nonempty PATH_INFO\n+our $base_url = $my_url;\n+\n+# When the script is used as DirectoryIndex, the URL does not contain the name\n+# of the script file itself, and $cgi->url() fails to strip PATH_INFO, so we\n+# have to do it ourselves. We make $path_info global because it's also used\n+# later on.\n+#\n+# Another issue with the script being the DirectoryIndex is that the resulting\n+# $my_url data is not the full script URL: this is good, because we want\n+# generated links to keep implying the script name if it wasn't explicitly\n+# indicated in the URL we're handling, but it means that $my_url cannot be used\n+# as base URL.\n+# Therefore, if we needed to strip PATH_INFO, then we know that we have\n+# to build the base URL ourselves:\n our $path_info = $ENV{\"PATH_INFO\"};\n if ($path_info) {\n-\t$my_url =~ s,\\Q$path_info\\E$,,;\n-\t$my_uri =~ s,\\Q$path_info\\E$,,;\n+\tif ($my_url =~ s,\\Q$path_info\\E$,, &&\n+\t $my_uri =~ s,\\Q$path_info\\E$,, &&\n+\t defined $ENV{'SCRIPT_NAME'}) {\n+\t\t$base_url = $cgi->url(-base => 1) . $ENV{'SCRIPT_NAME'};\n+\t}\n }\n \n # core git executable to use\n@@ -2908,7 +2924,7 @@ EOF\n \t# the stylesheet, favicon etc urls won't work correctly with path_info\n \t# unless we set the appropriate base URL\n \tif ($ENV{'PATH_INFO'}) {\n-\t\tprint '\\n';\n+\t\tprint \"\\n\";\n \t}\n \t# print out each stylesheet that exist, providing backwards capability\n \t# for those people who defined $stylesheet in a config file\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex 43cd6ee..7c6f70b 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -43,9 +43,11 @@ gitweb_run () {\n \tGATEWAY_INTERFACE=\"CGI/1.1\"\n \tHTTP_ACCEPT=\"*/*\"\n \tREQUEST_METHOD=\"GET\"\n+\tSCRIPT_NAME=\"$TEST_DIRECTORY/../gitweb/gitweb.perl\"\n \tQUERY_STRING=\"\"$1\"\"\n \tPATH_INFO=\"\"$2\"\"\n-\texport GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD QUERY_STRING PATH_INFO\n+\texport GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD \\\n+\t\tSCRIPT_NAME QUERY_STRING PATH_INFO\n \n \tGITWEB_CONFIG=$(pwd)/gitweb_config.perl\n \texport GITWEB_CONFIG\n@@ -54,7 +56,7 @@ gitweb_run () {\n \t# written to web server logs, so we are not interested in that:\n \t# we are interested only in properly formatted errors/warnings\n \trm -f gitweb.log &&\n-\tperl -- \"$TEST_DIRECTORY/../gitweb/gitweb.perl\" \\\n+\tperl -- \"$SCRIPT_NAME\" \\\n \t\t>/dev/null 2>gitweb.log &&\n \tif grep \"^[[]\" gitweb.log >/dev/null 2>&1; then false; else true; fi\n \n-- \n1.5.6.5\n"},{"id":"104806","messageId":"7v8wo7lckc.fsf@gitster.siamese.dyndns.org","threadId":"17761","inReplyTo":"cb7bb73a0902140629pa1ad038pe898aa0d804d0707@mail.gmail.com","subject":"Re: [PATCH v3] gitweb: fix wrong base URL when non-root DirectoryIndex","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-15T19:33:55Z","receivedAt":"2009-02-15T19:33:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Giuseppe Bilotta writes:\n\n> On Sat, Feb 14, 2009 at 11:04 AM, Jakub Narebski wrote:\n>>\n>> P.S. I wonder why Giuseppe's patch failed to apply, and failed\n>> to do fallback 3-way merge...\n>\n> Because it depends on an additional quoting fix that I sent to the\n> mailing list but that apparently didn't get through.\n>\n>> - print '\\n';\n>> + print '\\n';\n>\n> The quoting fix was here: double quotes are needed because otherwise\n> the \\n gets in literally. We can probably squash it with this patch.\n> Gimme a sec and I'll cook it up again.\n\nThanks, both.\n"}]}