{"thread":{"id":"31216","subject":"[PATCH] gitweb: URL-decode $my_url/$my_uri when stripping PATH_INFO","startedAt":"2012-08-09T02:29:26Z","lastAt":"2012-08-15T18:47:04Z","messageCount":4,"participants":["Jay Soffian","Junio C Hamano","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"196720","messageId":"1344479366-8957-1-git-send-email-jaysoffian@gmail.com","threadId":"31216","inReplyTo":null,"subject":"[PATCH] gitweb: URL-decode $my_url/$my_uri when stripping PATH_INFO","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2012-08-09T02:29:26Z","receivedAt":"2012-08-09T02:29:26Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"When gitweb is used as a DirectoryIndex, it attempts to strip\nPATH_INFO on its own, as $cgi->url() fails to do so.\n\nHowever, it fails to account for the fact that PATH_INFO has\nalready been URL-decoded by the web server, but the value\nreturned by $cgi->url() has not been. This causes the stripping\nto fail whenever the URL contains encoded characters.\n\nTo see this in action, setup gitweb as a DirectoryIndex and\nthen use it on a repository with a directory containing a\nspace in the name. Navigate to tree view, examine the gitweb\ngenerated html and you'll see a link such as:\n\n  <a href=\"/test.git/tree/HEAD:/directory with spaces\">directory with spaces</a>\n\nWhen clicked on, the browser will URL-encode this link, giving\na $cgi->url() of the form:\n\n   /test.git/tree/HEAD:/directory%20with%20spaces\n\nWhile PATH_INFO is:\n\n   /test.git/tree/HEAD:/directory with spaces\n\nFix this by calling unescape() on both $my_url and $my_uri before\nstripping PATH_INFO from them.\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\n gitweb/gitweb.perl | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 3d6a705388..7f8c1878d4 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -54,6 +54,11 @@ sub evaluate_uri {\n \t# to build the base URL ourselves:\n \tour $path_info = decode_utf8($ENV{\"PATH_INFO\"});\n \tif ($path_info) {\n+\t\t# $path_info has already been URL-decoded by the web server, but\n+\t\t# $my_url and $my_uri have not. URL-decode them so we can properly\n+\t\t# strip $path_info.\n+\t\t$my_url = unescape($my_url);\n+\t\t$my_uri = unescape($my_uri);\n \t\tif ($my_url =~ s,\\Q$path_info\\E$,, &&\n \t\t    $my_uri =~ s,\\Q$path_info\\E$,, &&\n \t\t    defined $ENV{'SCRIPT_NAME'}) {\n-- \n1.7.11.3\n"},{"id":"196733","messageId":"7vr4rgoz1u.fsf@alter.siamese.dyndns.org","threadId":"31216","inReplyTo":"1344479366-8957-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] gitweb: URL-decode $my_url/$my_uri when stripping PATH_INFO","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-09T15:38:53Z","receivedAt":"2012-08-09T15:38:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian <jaysoffian@gmail.com> writes:\n\n> When gitweb is used as a DirectoryIndex, it attempts to strip\n> PATH_INFO on its own, as $cgi->url() fails to do so.\n>\n> However, it fails to account for the fact that PATH_INFO has\n> already been URL-decoded by the web server, but the value\n> returned by $cgi->url() has not been. This causes the stripping\n> to fail whenever the URL contains encoded characters.\n>\n> To see this in action, setup gitweb as a DirectoryIndex and\n> then use it on a repository with a directory containing a\n> space in the name. Navigate to tree view, examine the gitweb\n> generated html and you'll see a link such as:\n>\n>   <a href=\"/test.git/tree/HEAD:/directory with spaces\">directory with spaces</a>\n>\n> When clicked on, the browser will URL-encode this link, giving\n> a $cgi->url() of the form:\n>\n>    /test.git/tree/HEAD:/directory%20with%20spaces\n>\n> While PATH_INFO is:\n>\n>    /test.git/tree/HEAD:/directory with spaces\n>\n> Fix this by calling unescape() on both $my_url and $my_uri before\n> stripping PATH_INFO from them.\n>\n> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>\n> ---\n\nThanks.  From a cursory look, with the help from the explanation in\nthe proposed commit log message, the change looks sensible.\n\nI wonder if a breakage like this is something we can catch in one of\nthe t95xx series of tests, though.\n\nJakub, Ack?\n\n>  gitweb/gitweb.perl | 5 +++++\n>  1 file changed, 5 insertions(+)\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 3d6a705388..7f8c1878d4 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -54,6 +54,11 @@ sub evaluate_uri {\n>  \t# to build the base URL ourselves:\n>  \tour $path_info = decode_utf8($ENV{\"PATH_INFO\"});\n>  \tif ($path_info) {\n> +\t\t# $path_info has already been URL-decoded by the web server, but\n> +\t\t# $my_url and $my_uri have not. URL-decode them so we can properly\n> +\t\t# strip $path_info.\n> +\t\t$my_url = unescape($my_url);\n> +\t\t$my_uri = unescape($my_uri);\n>  \t\tif ($my_url =~ s,\\Q$path_info\\E$,, &&\n>  \t\t    $my_uri =~ s,\\Q$path_info\\E$,, &&\n>  \t\t    defined $ENV{'SCRIPT_NAME'}) {\n"},{"id":"197051","messageId":"201208152015.49132.jnareb@gmail.com","threadId":"31216","inReplyTo":"7vr4rgoz1u.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] gitweb: URL-decode $my_url/$my_uri when stripping PATH_INFO","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-08-15T18:15:48Z","receivedAt":"2012-08-15T18:15:48Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 9 Aug 2012, Junio C Hamano wrote:\n> Jay Soffian <jaysoffian@gmail.com> writes:\n> \n> > When gitweb is used as a DirectoryIndex, it attempts to strip\n> > PATH_INFO on its own, as $cgi->url() fails to do so.\n> >\n> > However, it fails to account for the fact that PATH_INFO has\n> > already been URL-decoded by the web server, but the value\n> > returned by $cgi->url() has not been. This causes the stripping\n> > to fail whenever the URL contains encoded characters.\n> >\n> > To see this in action, setup gitweb as a DirectoryIndex and\n> > then use it on a repository with a directory containing a\n> > space in the name. Navigate to tree view, examine the gitweb\n> > generated html and you'll see a link such as:\n> >\n> >   <a href=\"/test.git/tree/HEAD:/directory with spaces\">directory with spaces</a>\n> >\n> > When clicked on, the browser will URL-encode this link, giving\n> > a $cgi->url() of the form:\n> >\n> >    /test.git/tree/HEAD:/directory%20with%20spaces\n> >\n> > While PATH_INFO is:\n> >\n> >    /test.git/tree/HEAD:/directory with spaces\n> >\n> > Fix this by calling unescape() on both $my_url and $my_uri before\n> > stripping PATH_INFO from them.\n> >\n> > Signed-off-by: Jay Soffian <jaysoffian@gmail.com>\n> > ---\n> \n> Thanks.  From a cursory look, with the help from the explanation in\n> the proposed commit log message, the change looks sensible.\n> \n> I wonder if a breakage like this is something we can catch in one of\n> the t95xx series of tests, though.\n\nNo, it is unfortunately not possible with current test infrastructure\nfor gitweb.  The gitweb_run from t/gitweb-lib.sh allows to set\nPATH_INFO and QUERY_STRING, but does not allow to set up URL.\n\nThat might change in the future...\n\n> Jakub, Ack?\n\nAcked-by: Jakub Narebski <jnareb@gmail.com>\n\nUf ut us bot too late...\n\n\n-- \nJakub Narebski\nPoland\n"},{"id":"197055","messageId":"7vipck56xj.fsf@alter.siamese.dyndns.org","threadId":"31216","inReplyTo":"201208152015.49132.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: URL-decode $my_url/$my_uri when stripping PATH_INFO","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-15T18:47:04Z","receivedAt":"2012-08-15T18:47:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> On Thu, 9 Aug 2012, Junio C Hamano wrote:\n>> Jay Soffian <jaysoffian@gmail.com> writes:\n>> \n>> > When gitweb is used as a DirectoryIndex, it attempts to strip\n>> > PATH_INFO on its own, as $cgi->url() fails to do so.\n>> >\n>> > However, it fails to account for the fact that PATH_INFO has\n>> > already been URL-decoded by the web server, but the value\n>> > returned by $cgi->url() has not been. This causes the stripping\n>> > to fail whenever the URL contains encoded characters.\n>> >\n>> > To see this in action, setup gitweb as a DirectoryIndex and\n>> > then use it on a repository with a directory containing a\n>> > space in the name. Navigate to tree view, examine the gitweb\n>> > generated html and you'll see a link such as:\n>> >\n>> >   <a href=\"/test.git/tree/HEAD:/directory with spaces\">directory with spaces</a>\n>> >\n>> > When clicked on, the browser will URL-encode this link, giving\n>> > a $cgi->url() of the form:\n>> >\n>> >    /test.git/tree/HEAD:/directory%20with%20spaces\n>> >\n>> > While PATH_INFO is:\n>> >\n>> >    /test.git/tree/HEAD:/directory with spaces\n>> >\n>> > Fix this by calling unescape() on both $my_url and $my_uri before\n>> > stripping PATH_INFO from them.\n>> >\n>> > Signed-off-by: Jay Soffian <jaysoffian@gmail.com>\n>> > ---\n>> \n>> Thanks.  From a cursory look, with the help from the explanation in\n>> the proposed commit log message, the change looks sensible.\n>> \n>> I wonder if a breakage like this is something we can catch in one of\n>> the t95xx series of tests, though.\n>\n> No, it is unfortunately not possible with current test infrastructure\n> for gitweb.  The gitweb_run from t/gitweb-lib.sh allows to set\n> PATH_INFO and QUERY_STRING, but does not allow to set up URL.\n>\n> That might change in the future...\n>\n>> Jakub, Ack?\n>\n> Acked-by: Jakub Narebski <jnareb@gmail.com>\n>\n> Uf ut us bot too late...\n\nThanks.\n"}]}