{"thread":{"id":"32457","subject":"[PATCH] gitweb: fix error when highlight is enabled","startedAt":"2012-12-26T07:54:16Z","lastAt":"2012-12-26T09:55:46Z","messageCount":2,"participants":["Orgad Shaneh","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"205489","messageId":"1356508456-17454-1-git-send-email-orgads@gmail.com","threadId":"32457","inReplyTo":null,"subject":"[PATCH] gitweb: fix error when highlight is enabled","fromName":"Orgad Shaneh","fromEmail":"orgads@gmail.com","sentAt":"2012-12-26T07:54:16Z","receivedAt":"2012-12-26T07:54:16Z","isPatch":true,"sender":{"key":"orgads@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1246544?v=4"},"body":"Use of uninitialized value in substitution iterator at gitweb.cgi line 1560\n\nSigned-off-by: Orgad Shaneh <orgads@gmail.com>\n---\n gitweb/gitweb.perl |    2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0f207f2..862b9cd 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1556,7 +1556,7 @@ sub sanitize {\n \treturn undef unless defined $str;\n \n \t$str = to_utf8($str);\n-\t$str =~ s|([[:cntrl:]])|($1 =~ /[\\t\\n\\r]/ ? $1 : quot_cec($1))|eg;\n+\t$str =~ s|([[:cntrl:]])|($1 =~ /([\\t\\n\\r])/ ? $1 : quot_cec($1))|eg;\n \treturn $str;\n }\n \n-- \n1.7.10.4\n"},{"id":"205491","messageId":"7vpq1xp271.fsf@alter.siamese.dyndns.org","threadId":"32457","inReplyTo":"1356508456-17454-1-git-send-email-orgads@gmail.com","subject":"Re: [PATCH] gitweb: fix error when highlight is enabled","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-26T09:55:46Z","receivedAt":"2012-12-26T09:55:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Orgad Shaneh <orgads@gmail.com> writes:\n\n> Use of uninitialized value in substitution iterator at gitweb.cgi line 1560\n\nThis is not just about squelching an error message, but more\nimportantly, attempting to fix an information lossage, no?\n\nThe statement captures each control character in the string to $1,\nthen matches a class of known/safe control chars against that\ncontrol character we just have seen. If matches, it just wants to\nuse that control character, otherwise it wants to apply quot_cec()\non that control character.  It forgets that \"$1\" is reset\nimmediately when =~ matches with the class of known/safe control\nchars, and your version attempts to fix it by recapturing it.\n\nWhat if you are looking at a non-safe control, say \"\\001\"?  It is\nmatched and is captured by ([[;cntrl:]]), making $1 -eq \"\\001\", and\nthen the replacement side of s///e operator, tries to match and\ncapture it with ([\\t\\n\\r]), but it does *not* match.\n\nWhat does that \"$1\" you are feeding quot_cec() contain at that\npoint?  I _think_ \"$1\" is left intact when the inner match fails and\nyou are correctly feeding \"\\001\" to quot_cec(), but it is not\nimmediately obvious.  Perl regexp, especially s///e, is a yucky\nlanguage X-<.\n\nI wonder if there is a better way to express what goes inside the\nreplacement side of this s///e construct in a more obvious way. The\nupdated one may be correct but it looks too subtle to my taste..\n\n> Signed-off-by: Orgad Shaneh <orgads@gmail.com>\n> ---\n>  gitweb/gitweb.perl |    2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 0f207f2..862b9cd 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -1556,7 +1556,7 @@ sub sanitize {\n>  \treturn undef unless defined $str;\n>  \n>  \t$str = to_utf8($str);\n> -\t$str =~ s|([[:cntrl:]])|($1 =~ /[\\t\\n\\r]/ ? $1 : quot_cec($1))|eg;\n> +\t$str =~ s|([[:cntrl:]])|($1 =~ /([\\t\\n\\r])/ ? $1 : quot_cec($1))|eg;\n>  \treturn $str;\n>  }\n"}]}