Petr Baudis wrote: > Consider: > > http://repo.or.cz/?p=glibc-cvs.git;a=tree;h=2609cb0411389325f4ee2854cc7159756eb0671e;hb=2609cb0411389325f4ee2854cc7159756eb0671e > > (click on the funny =__ify file) > > We ought to handle anything in filenames and I actually see no reason why > we don't, modulo very little missing escaping that this patch hopefully > also fixes. > > I have also made esc_param() escape [?=&;]. Not escaping [&;] was downright > buggy and [?=] just feels better escaped. ;-) YMMV. > > Signed-off-by: Petr Baudis <pasky@xxxxxxx> This patch contains a few unrelated changes: * changing semantics of esc_param subroutine * adding some esc_html where it could be needed * removing $file_name and $file_parent validation About change to esc_param: we need current version of esc_param, perhaps to be named esc_url to be able to say esc_url($home_link) and soon esc_url($githelp_url). About adding esc_html where it could be needed: good change, see my comments below. About removing $file_name and $file_parent validation: those parameters have exactly the same textual restrictions as $project parameter - they are pathnames. > @@ -2439,7 +2429,7 @@ sub git_blame2 { > if ($ftype !~ "blob") { > die_error("400 Bad Request", "Object is not a blob"); > } > - open ($fd, "-|", git_cmd(), "blame", '-l', $file_name, $hash_base) > + open ($fd, "-|", git_cmd(), "blame", '-l', '--', $file_name, $hash_base) > or die_error(undef, "Open git-blame failed"); > git_header_html(); > my $formats_nav = Slightly unrelated change. Shouldn't it be open $fd, "-|", git_cmd(), "blame", '-l', $hash_base, "--", $file_name by the way? > @@ -3135,7 +3125,7 @@ sub git_blobdiff { > -type => 'text/plain', > -charset => 'utf-8', > -expires => $expires, > - -content_disposition => qq(inline; filename="${file_name}.patch")); > + -content_disposition => qq(inline; filename=") . quotemeta($file_name) . qq(.patch")); > > print "X-Git-Url: " . $cgi->self_url() . "\n\n"; > I'd check other places where we output Content-Disposition: header. At least one place needs similar quotemeta somewhere. > @@ -3585,7 +3575,7 @@ XML > if (!($line =~ m/^:([0-7]{6}) ([0-7]{6}) ([0-9a-fA-F]{40}) ([0-9a-fA-F]{40}) (.)([0-9]{0,3})\t(.*)$/)) { > next; > } > - my $file = validate_input(unquote($7)); > + my $file = esc_html(unquote($7)); > $file = decode("utf8", $file, Encode::FB_DEFAULT); > print "$file<br/>\n"; > } I'd say perhaps my $file = unquote($7); $file = decode("utf8", $file, Encode::FB_DEFAULT); print esc_html($file) . "<br/>\n"; -- Jakub Narebski Warsaw, Poland ShadeHawk on #git - 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