{"thread":{"id":"20710","subject":"[RFC/PATCH 1/3] gitweb: make suspenders more useful","startedAt":"2009-08-23T21:53:42Z","lastAt":"2009-08-24T03:09:36Z","messageCount":5,"participants":["Mark Rada","Sam Vilain"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"121591","messageId":"4A91BA66.5050108@mailservices.uwaterloo.ca","threadId":"20710","inReplyTo":null,"subject":"[RFC/PATCH 1/3] gitweb: make suspenders more useful","fromName":"Mark Rada","fromEmail":"marada@uwaterloo.ca","sentAt":"2009-08-23T21:53:42Z","receivedAt":"2009-08-23T21:53:42Z","isPatch":true,"sender":{"key":"marada@uwaterloo.ca","avatar":"https://avatars.githubusercontent.com/u/38430?v=4"},"body":"In the first block of checks to validate a snapshot request, the last\ncheck is never executed because the second last check is a superset of\nthe last check.\n\nThis change will switch the order of the last two checks, it has the\nadvantage of giving clients a more specific reason why they cannot get\na specific snapshot format instead of giving them the more generic\nresponse.\n\nSigned-off-by: Mark Rada <marada@uwaterloo.ca>\n---\n gitweb/gitweb.perl |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 4a42f61..7068db2 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5174,10 +5174,10 @@ sub git_snapshot {\n \t\tdie_error(400, \"Invalid snapshot format parameter\");\n \t} elsif (!exists($known_snapshot_formats{$format})) {\n \t\tdie_error(400, \"Unknown snapshot format\");\n-\t} elsif (!grep($_ eq $format, @snapshot_fmts)) {\n-\t\tdie_error(403, \"Unsupported snapshot format\");\n \t} elsif ($known_snapshot_formats{$format}{'disabled'}) {\n \t\tdie_error(403, \"Snapshot format not allowed\");\n+\t} elsif (!grep($_ eq $format, @snapshot_fmts)) {\n+\t\tdie_error(403, \"Unsupported snapshot format\");\n \t}\n \n \tif (!defined $hash) {\n-- \nMark A Rada (ferrous26)\nmarada@uwaterloo.ca\n"},{"id":"121594","messageId":"4A91BE50.7070103@vilain.net","threadId":"20710","inReplyTo":"4A91BA66.5050108@mailservices.uwaterloo.ca","subject":"Re: [RFC/PATCH 1/3] gitweb: make suspenders more useful","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2009-08-23T22:10:24Z","receivedAt":"2009-08-23T22:10:24Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"Mark Rada wrote:\n> Subject: [RFC/PATCH 1/3] gitweb: make suspenders more useful\n\nSuspenders?  Really?\n\nSam\n"},{"id":"121599","messageId":"4A91C512.7000707@mailservices.uwaterloo.ca","threadId":"20710","inReplyTo":"4A91BE50.7070103@vilain.net","subject":"Re: [RFC/PATCH 1/3] gitweb: make suspenders more useful","fromName":"Mark Rada","fromEmail":"marada@uwaterloo.ca","sentAt":"2009-08-23T22:39:14Z","receivedAt":"2009-08-23T22:39:14Z","isPatch":true,"sender":{"key":"marada@uwaterloo.ca","avatar":"https://avatars.githubusercontent.com/u/38430?v=4"},"body":"On 23/08/09 6:10 PM, Sam Vilain wrote:\n> Mark Rada wrote:\n>> Subject: [RFC/PATCH 1/3] gitweb: make suspenders more useful\n> \n> Suspenders?  Really?\n> \n> Sam\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n\nContext:\n\nOn 21/08/09 5:43 PM, Junio C Hamano wrote:\n> Mark A Rada <marada@uwaterloo.ca> writes:\n> \n>> Unless I missed a case, the tests show that the extra condition check\n>> that was added in the &git_snapshot routine is never actually executed,\n>> because a disabled snapshot format is not added to @snapshot_fmts, which\n>> is checked first.\n>>\n>> snippet:\n>> 5178     } elsif (!grep($_ eq $format, @snapshot_fmts)) {\n>> 5179         die_error(403, \"Unsupported snapshot format\");\n>> 5180     } elsif ($known_snapshot_formats{$format}{'disabled'}) {\n>> 5181         die_error(403, \"Snapshot format not allowed\");\n>> 5182     }\n>> 5183\n> \n> True; filter_snapshot_fmts looks at 'disabled' first.\n> \n> I do not mind keeping these two lines as belt-and-suspender, though.\n> \n\n\nYup...I couldn't think of an appropriate title and I expect this\npatch to be edited (for another reason) or thrown out anyways.\n\n\n\n-- \nMark A Rada (ferrous26)\nmarada@uwaterloo.ca\n"},{"id":"121600","messageId":"4A91C74D.1080908@vilain.net","threadId":"20710","inReplyTo":"4A91C512.7000707@mailservices.uwaterloo.ca","subject":"Re: [RFC/PATCH 1/3] gitweb: make suspenders more useful","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2009-08-23T22:48:45Z","receivedAt":"2009-08-23T22:48:45Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"Mark Rada wrote:\n>>> Subject: [RFC/PATCH 1/3] gitweb: make suspenders more useful\n>>>       \n>> Suspenders?  Really?\n>>     \n> Context:\n>\n> On 21/08/09 5:43 PM, Junio C Hamano wrote:\n>   \n>> I do not mind keeping these two lines as belt-and-suspender, though.\n>>     \n> Yup...I couldn't think of an appropriate title and I expect this\n> patch to be edited (for another reason) or thrown out anyways.\n>   \n\nPlease don't do that.  Use of metaphors makes the code harder to follow,\nespecially for non-native speakers.  In this case you confused me, a\nBritish/NZ native speaker because you only used half of the idiom; so\nall I was left with was a word which in the English where I speak refers\nto a piece of clothing worn most often by prostitutes and people at the\nRocky Horror Picture Show.\n\nCan I suggest \"snapshot error handling\" as a more neutral term...\nSam\n"},{"id":"121605","messageId":"4A920470.1090709@mailservices.uwaterloo.ca","threadId":"20710","inReplyTo":"4A91C74D.1080908@vilain.net","subject":"[RFC/PATCH 1/3] gitweb: improve snapshot error handling","fromName":"Mark Rada","fromEmail":"marada@uwaterloo.ca","sentAt":"2009-08-24T03:09:36Z","receivedAt":"2009-08-24T03:09:36Z","isPatch":true,"sender":{"key":"marada@uwaterloo.ca","avatar":"https://avatars.githubusercontent.com/u/38430?v=4"},"body":"Ok, this is just a resend of the first patch in my set from\nearlier, but with some of the commit message reworded, namely\nthe subject (as suggested by Sam Vilain).\n\n--\nMark Rada (ferrous26)\nmarada@uwaterloo.ca\n\n\n--->8---\nIn the second block of checks to validate a snapshot request, the last\ncheck is never executed because the second last check is a superset of\nthe last check.\n\nThis change will switch the order of the last two checks, it has the\nadvantage of giving clients a more specific reason why they cannot get\na snapshot format if the format they have chosen is disabled.\n\nSigned-off-by: Mark Rada <marada@uwaterloo.ca>\n---\n gitweb/gitweb.perl |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 4a42f61..7068db2 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5174,10 +5174,10 @@ sub git_snapshot {\n \t\tdie_error(400, \"Invalid snapshot format parameter\");\n \t} elsif (!exists($known_snapshot_formats{$format})) {\n \t\tdie_error(400, \"Unknown snapshot format\");\n-\t} elsif (!grep($_ eq $format, @snapshot_fmts)) {\n-\t\tdie_error(403, \"Unsupported snapshot format\");\n \t} elsif ($known_snapshot_formats{$format}{'disabled'}) {\n \t\tdie_error(403, \"Snapshot format not allowed\");\n+\t} elsif (!grep($_ eq $format, @snapshot_fmts)) {\n+\t\tdie_error(403, \"Unsupported snapshot format\");\n \t}\n \n \tif (!defined $hash) {\n-- \n1.6.4\n"}]}