Re: [PATCH 1/5] gitweb: Cleanup input validation and error messages

From: Junio C Hamano <junkio@cox.net>
Date: 2006-08-05 10:15:31
Jakub Narebski <jnareb@gmail.com> writes:

>  our $action = $cgi->param('a');
>  if (defined $action) {
>  	if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
> -		undef $action;
> -		die_error(undef, "Invalid action parameter.");
> +		die_error(undef, "Invalid action parameter $action");
>  	}

Doesn't this make us parrot what the browser threw at us without
escaping back for HTML (iow, die_error does not seem to escape
$error)?

>  our $project = ($cgi->param('p') || $ENV{'PATH_INFO'});
> -if (defined $project) {
> -	$project =~ s|^/||; $project =~ s|/$||;
> -	$project = validate_input($project);
> -	if (!defined($project)) {
> -		die_error(undef, "Invalid project parameter.");
> +$project =~ s|^/||; $project =~ s|/$||;

Unrelated change but it is probably safe.

> +if (defined $project || $project) {
> +	if (!validate_input($project)) {
> +		die_error(undef, "Invalid project parameter $project");
>  	}

Because undef is not true, "|| $project" here does not make much
sense to me.  Even if you meant to say "&&" to exclude empty
string or "0", wouldn't validate_input() take care of them?
Same input parrotting comment applies here and everywhere.

> -	$rss_link = "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
> -	            "$my_uri?" . esc_param("p=$project;a=rss") . "\" type=\"application/rss+xml\"/>";

The reason of removal is...?  Ah, you inlined it.  It was not
clear from the proposed commit log message.

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Received on Sat Aug 05 10:16:10 2006

This archive was generated by hypermail 2.1.8 : 2006-08-05 10:16:40 EST