Petr Baudis wrote: > Consider: > > http://repo.or.cz/?p=glibc-cvs.git;a=tree;h=2609cb0411389325f4ee2854cc7159756eb0671e;hb=2609cb0411389325f4ee2854cc7159756eb0671e > > (click on the funny =__ify file) Aaargh? Why this name? > 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. First of all, before introduction href() subroutine we escaped (using esc_param) the _whole_ generated query string. Now we need to escape only value part (as we can assume that parameter names are correct; they are taken from limited pre-defined set). I'd rather have new esc_param() or esc_param_value() quote like escape subroutine from CGI::Util, with the esception of _not_ escaping '/' (it makes funny bookmark, and lot less readable query string), and rename current esc_param() to esc_query_string() or esc_params(). Perhaps we should have also esc_arg() for things like title attribute of <a> (link) element (or other element) and filename="..." part of Content-disposition: HTTP header. By the way, the validate_input() should be split into separate subroutines: validate_ref() for validating hash, hash_base, hash_parent, hash_parent_base, and validate_path() for validating project, file_name and file_parent parameters. We should _never_ use esc_html except during the output, or just before output. It certainly shouldn't take place in parse_* subroutine (or in the fake parse like in git_blobdiff)! P.S. gitweb was tested I think with filenames wirh spaces. Perhaps we should add some other test file to gitweb/test, with '=', '@', '$', '?', '*', ' ', '\n' etc. -- 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