Re: [PATCH] gitweb: fix error when highlight is enabled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Orgad Shaneh <orgads@xxxxxxxxx> writes:

> Use of uninitialized value in substitution iterator at gitweb.cgi line 1560

This is not just about squelching an error message, but more
importantly, attempting to fix an information lossage, no?

The statement captures each control character in the string to $1,
then matches a class of known/safe control chars against that
control character we just have seen. If matches, it just wants to
use that control character, otherwise it wants to apply quot_cec()
on that control character.  It forgets that "$1" is reset
immediately when =~ matches with the class of known/safe control
chars, and your version attempts to fix it by recapturing it.

What if you are looking at a non-safe control, say "\001"?  It is
matched and is captured by ([[;cntrl:]]), making $1 -eq "\001", and
then the replacement side of s///e operator, tries to match and
capture it with ([\t\n\r]), but it does *not* match.

What does that "$1" you are feeding quot_cec() contain at that
point?  I _think_ "$1" is left intact when the inner match fails and
you are correctly feeding "\001" to quot_cec(), but it is not
immediately obvious.  Perl regexp, especially s///e, is a yucky
language X-<.

I wonder if there is a better way to express what goes inside the
replacement side of this s///e construct in a more obvious way. The
updated one may be correct but it looks too subtle to my taste..

> Signed-off-by: Orgad Shaneh <orgads@xxxxxxxxx>
> ---
>  gitweb/gitweb.perl |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 0f207f2..862b9cd 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1556,7 +1556,7 @@ sub sanitize {
>  	return undef unless defined $str;
>  
>  	$str = to_utf8($str);
> -	$str =~ s|([[:cntrl:]])|($1 =~ /[\t\n\r]/ ? $1 : quot_cec($1))|eg;
> +	$str =~ s|([[:cntrl:]])|($1 =~ /([\t\n\r])/ ? $1 : quot_cec($1))|eg;
>  	return $str;
>  }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]