{"thread":{"id":"18876","subject":"Re: [PATCH/RESEND] gitweb: Fix snapshots requested via PATH_INFO","startedAt":"2009-04-15T06:40:48Z","lastAt":"2009-04-20T10:41:07Z","messageCount":7,"participants":["Junio C Hamano","Giuseppe Bilotta","Holger Weiß","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"111338","messageId":"7vvdp6xvv3.fsf@gitster.siamese.dyndns.org","threadId":"18876","inReplyTo":"20090414104648.GA36554444@CIS.FU-Berlin.DE","subject":"Re: [PATCH/RESEND] gitweb: Fix snapshots requested via PATH_INFO","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-15T06:40:48Z","receivedAt":"2009-04-15T06:40:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Holger Weiß writes:\n\n> Fix the detection of the requested snapshot format, which failed for\n> PATH_INFO URLs since the references to the hashes which describe the\n> supported snapshot formats weren't dereferenced appropriately.\n>\n> Signed-off-by: Holger Weiß \n> ---\n> I guess this one got lost. Without this patch, snapshots won't work if\n> Gitweb is configured to generate PATH_INFO URLs. (Original Message-ID:\n> <20090331161636.GV30233737@CIS.FU-Berlin.DE>).\n\nThe patch looks obviously correct; \"our %known_snapshort_formats\" maps a\nname to a hashref, but the current code makes a nonsense assignment,\nessentialy doing ($fmt, %opt) = ($name, $hashref), but what would I\nknow... I am not using gitweb actively.\n\nThese lines come from 1ec2fb5 (gitweb: retrieve snapshot format from\nPATH_INFO, 2008-11-02) by Guiseppe.\n\nJudging from the \"git shortlog -n -s --grep=PATH_INFO gitweb\" output, I\nthink I should have heard from either Guiseppe and Jakub by now if this\npatch is desired. Pinging them...\n\n> gitweb/gitweb.perl | 4 ++--\n> 1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 33ef190..3f99361 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -688,10 +688,10 @@ sub evaluate_path_info {\n> \t\t# extensions. Allowed extensions are both the defined suffix\n> \t\t# (which includes the initial dot already) and the snapshot\n> \t\t# format key itself, with a prepended dot\n> -\t\twhile (my ($fmt, %opt) = each %known_snapshot_formats) {\n> +\t\twhile (my ($fmt, $opt) = each %known_snapshot_formats) {\n> \t\t\tmy $hash = $refname;\n> \t\t\tmy $sfx;\n> -\t\t\t$hash =~ s/(\\Q$opt{'suffix'}\\E|\\Q.$fmt\\E)$//;\n> +\t\t\t$hash =~ s/(\\Q$opt->{'suffix'}\\E|\\Q.$fmt\\E)$//;\n> \t\t\tnext unless $sfx = $1;\n> \t\t\t# a valid suffix was found, so set the snapshot format\n> \t\t\t# and reset the hash parameter\n> -- \n> 1.6.2.1\n"},{"id":"111348","messageId":"cb7bb73a0904150233wfa72b1fn85692f81880f6848@mail.gmail.com","threadId":"18876","inReplyTo":"7vvdp6xvv3.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH/RESEND] gitweb: Fix snapshots requested via PATH_INFO","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-04-15T09:33:48Z","receivedAt":"2009-04-15T09:33:48Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Wed, Apr 15, 2009 at 8:40 AM, Junio C Hamano wrote:\n> Holger Weiß writes:\n>\n>> Fix the detection of the requested snapshot format, which failed for\n>> PATH_INFO URLs since the references to the hashes which describe the\n>> supported snapshot formats weren't dereferenced appropriately.\n>>\n>> Signed-off-by: Holger Weiß \n>> ---\n>> I guess this one got lost.  Without this patch, snapshots won't work if\n>> Gitweb is configured to generate PATH_INFO URLs.  (Original Message-ID:\n>> <20090331161636.GV30233737@CIS.FU-Berlin.DE>).\n>\n> The patch looks obviously correct; \"our %known_snapshort_formats\" maps a\n> name to a hashref, but the current code makes a nonsense assignment,\n> essentialy doing ($fmt, %opt) = ($name, $hashref), but what would I\n> know...  I am not using gitweb actively.\n\nMy gitweb over at http://git.oblomov.eu/ supports snapshots with\nPATH_INFO just fine even without the need for this patch. Could this\nbe a perl version issue? My apache is using mod_perl version 2.0.4,\nand I have a perl 5.10 on my system.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"111349","messageId":"20090415100955.GA33221014@CIS.FU-Berlin.DE","threadId":"18876","inReplyTo":"cb7bb73a0904150233wfa72b1fn85692f81880f6848@mail.gmail.com","subject":"Re: [PATCH/RESEND] gitweb: Fix snapshots requested via PATH_INFO","fromName":"Holger Weiß","fromEmail":"holger@zedat.fu-berlin.de","sentAt":"2009-04-15T10:09:55Z","receivedAt":"2009-04-15T10:09:55Z","isPatch":true,"sender":{"key":"holger@zedat.fu-berlin.de","avatar":"https://avatars.githubusercontent.com/u/71850?v=4"},"body":"* Giuseppe Bilotta [2009-04-15 11:33]:\n> On Wed, Apr 15, 2009 at 8:40 AM, Junio C Hamano wrote:\n> > Holger Weiß writes:\n> >> Fix the detection of the requested snapshot format, which failed for\n> >> PATH_INFO URLs since the references to the hashes which describe the\n> >> supported snapshot formats weren't dereferenced appropriately.\n> >>\n> >> Signed-off-by: Holger Weiß \n> >> ---\n> >> I guess this one got lost.  Without this patch, snapshots won't work if\n> >> Gitweb is configured to generate PATH_INFO URLs.  (Original Message-ID:\n> >> <20090331161636.GV30233737@CIS.FU-Berlin.DE>).\n> >\n> > The patch looks obviously correct; \"our %known_snapshort_formats\" maps a\n> > name to a hashref, but the current code makes a nonsense assignment,\n> > essentialy doing ($fmt, %opt) = ($name, $hashref), but what would I\n> > know...  I am not using gitweb actively.\n> \n> My gitweb over at http://git.oblomov.eu/ supports snapshots with\n> PATH_INFO just fine even without the need for this patch.\n\nReally? If I try to download a snapshot from your site, I get an empty\ntarball (and the server appends an additional \".tar.gz\" suffix to the\nfilename within the \"Content-disposition\" header). For example:\n\n$ wget -q -O - http://git.oblomov.eu/git/snapshot/v1.6.2.3.tar.gz | gzip -d | tar -t | wc -l\n0\n\nThat's the bug which is fixed by my patch.\n\nHolger\n"},{"id":"111351","messageId":"cb7bb73a0904150429l7c9910aer847464add60ee345@mail.gmail.com","threadId":"18876","inReplyTo":"20090415100955.GA33221014@CIS.FU-Berlin.DE","subject":"Re: [PATCH/RESEND] gitweb: Fix snapshots requested via PATH_INFO","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-04-15T11:29:27Z","receivedAt":"2009-04-15T11:29:27Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"2009/4/15 Holger Weiß :\n> * Giuseppe Bilotta [2009-04-15 11:33]:\n\n>> My gitweb over at http://git.oblomov.eu/ supports snapshots with\n>> PATH_INFO just fine even without the need for this patch.\n>\n> Really?  If I try to download a snapshot from your site, I get an empty\n> tarball (and the server appends an additional \".tar.gz\" suffix to the\n> filename within the \"Content-disposition\" header).  For example:\n>\n> $ wget -q -O - http://git.oblomov.eu/git/snapshot/v1.6.2.3.tar.gz | gzip -d | tar -t | wc -l\n> 0\n>\n> That's the bug which is fixed by my patch.\n\nThis gets weirder and weirder. I'm seeing the same behaviour with your\npatch. I think I busted something the upgrade I was running this\nmorning. Even git archive on the command line gives me an empty tar.\n\nI have no idea what happened. I rebuilt and reinstalled git and now\nit's working again, and I see the problem too, and yes your patch\nfixes it.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"111383","messageId":"200904151934.10253.jnareb@gmail.com","threadId":"18876","inReplyTo":"7vvdp6xvv3.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH/RESEND] gitweb: Fix snapshots requested via PATH_INFO","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-04-15T17:34:08Z","receivedAt":"2009-04-15T17:34:08Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 15 April 2009, Junio C Hamano wrote:\n> Holger Weiß writes:\n> \n> > Fix the detection of the requested snapshot format, which failed for\n> > PATH_INFO URLs since the references to the hashes which describe the\n> > supported snapshot formats weren't dereferenced appropriately.\n> >\n> > Signed-off-by: Holger Weiß \n> > ---\n> > I guess this one got lost. Without this patch, snapshots won't work if\n> > Gitweb is configured to generate PATH_INFO URLs. (Original Message-ID:\n> > <20090331161636.GV30233737@CIS.FU-Berlin.DE>).\n> \n> The patch looks obviously correct; \"our %known_snapshort_formats\" maps a\n> name to a hashref, but the current code makes a nonsense assignment,\n> essentialy doing ($fmt, %opt) = ($name, $hashref), but what would I\n> know... I am not using gitweb actively.\n> \n> These lines come from 1ec2fb5 (gitweb: retrieve snapshot format from\n> PATH_INFO, 2008-11-02) by Guiseppe.\n> \n> Judging from the \"git shortlog -n -s --grep=PATH_INFO gitweb\" output, I\n> think I should have heard from either Guiseppe and Jakub by now if this\n> patch is desired. Pinging them...\n\nThis change looks correct, and is very much desired. Thanks for\ncatching this.\n\nBy the way, if there was check added for full path_info snapshot URL in\nexisting t/t9500-gitweb-standalone-no-errors.sh it would caught this\nbug thanks to the\n \"Odd number of elements in hash assignment ...\"\nwarning that Perl throws in this case. \n\n> > gitweb/gitweb.perl | 4 ++--\n> > 1 files changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> > index 33ef190..3f99361 100755\n> > --- a/gitweb/gitweb.perl\n> > +++ b/gitweb/gitweb.perl\n> > @@ -688,10 +688,10 @@ sub evaluate_path_info {\n> > \t\t# extensions. Allowed extensions are both the defined suffix\n> > \t\t# (which includes the initial dot already) and the snapshot\n> > \t\t# format key itself, with a prepended dot\n> > -\t\twhile (my ($fmt, %opt) = each %known_snapshot_formats) {\n> > +\t\twhile (my ($fmt, $opt) = each %known_snapshot_formats) {\n> > \t\t\tmy $hash = $refname;\n> > \t\t\tmy $sfx;\n> > -\t\t\t$hash =~ s/(\\Q$opt{'suffix'}\\E|\\Q.$fmt\\E)$//;\n> > +\t\t\t$hash =~ s/(\\Q$opt->{'suffix'}\\E|\\Q.$fmt\\E)$//;\n> > \t\t\tnext unless $sfx = $1;\n> > \t\t\t# a valid suffix was found, so set the snapshot format\n> > \t\t\t# and reset the hash parameter\n> > -- \n> > 1.6.2.1\n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"111703","messageId":"m3iqkzps96.fsf@localhost.localdomain","threadId":"18876","inReplyTo":"200904151934.10253.jnareb@gmail.com","subject":"Re: [PATCH/RESEND] gitweb: Fix snapshots requested via PATH_INFO","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-04-20T09:52:48Z","receivedAt":"2009-04-20T09:52:48Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"I'm sorry for resend, but I forgot to quote non-ASCII in 'Cc:'\nand vger anti-SPAM filter rejected message...\n\nJakub Narebski writes:\n> On Wed, 15 April 2009, Junio C Hamano wrote:\n>> Holger Weiß writes:\n>> \n>>> Fix the detection of the requested snapshot format, which failed for\n>>> PATH_INFO URLs since the references to the hashes which describe the\n>>> supported snapshot formats weren't dereferenced appropriately.\n>>>\n>>> Signed-off-by: Holger Weiß \n>>> ---\n>>> I guess this one got lost. Without this patch, snapshots won't work if\n>>> Gitweb is configured to generate PATH_INFO URLs. (Original Message-ID:\n>>> <20090331161636.GV30233737@CIS.FU-Berlin.DE>).\n>> \n>> The patch looks obviously correct; \"our %known_snapshort_formats\" maps a\n>> name to a hashref, but the current code makes a nonsense assignment,\n>> essentialy doing ($fmt, %opt) = ($name, $hashref), but what would I\n>> know... I am not using gitweb actively.\n>> \n>> These lines come from 1ec2fb5 (gitweb: retrieve snapshot format from\n>> PATH_INFO, 2008-11-02) by Guiseppe.\n>> \n>> Judging from the \"git shortlog -n -s --grep=PATH_INFO gitweb\" output, I\n>> think I should have heard from either Guiseppe and Jakub by now if this\n>> patch is desired. Pinging them...\n> \n> This change looks correct, and is very much desired. Thanks for\n> catching this.\n\nPing! This is quite straighforward bugfix for a new feature...\n\n> By the way, if there was check added for full path_info snapshot URL in\n> existing t/t9500-gitweb-standalone-no-errors.sh it would caught this\n> bug thanks to the\n> \"Odd number of elements in hash assignment ...\"\n> warning that Perl throws in this case. \n\n... or are you waiting for test case?\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"111711","messageId":"7v7i1fip4s.fsf@gitster.siamese.dyndns.org","threadId":"18876","inReplyTo":"m3iqkzps96.fsf@localhost.localdomain","subject":"Re: [PATCH/RESEND] gitweb: Fix snapshots requested via PATH_INFO","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-20T10:41:07Z","receivedAt":"2009-04-20T10:41:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> Ping! This is quite straighforward bugfix for a new feature...\n>\n>> By the way, if there was check added for full path_info snapshot URL in\n>> existing t/t9500-gitweb-standalone-no-errors.sh it would caught this\n>> bug thanks to the\n>> \"Odd number of elements in hash assignment ...\"\n>> warning that Perl throws in this case. \n>\n> ... or are you waiting for test case?\n\nYeah, I somehow misinterpreted that you are saying you will follow up with\nan additional test to cover this, and while I was waiting I got busy and\nforgot.\n"}]}