Giuseppe Bilotta wrote:
> On Tue, Oct 7, 2008 at 12:57 PM, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
>> On Fri, 3 Oct 2008, Giuseppe Bilotta wrote:
>>> + # find which part of PATH_INFO is project
>>> + my $project = $path_info;
>>
>> Hmmm... now $project is local (lexically) here.
>
> Yes, itt's only used temporarily here, to see if a proper $project
> can be defined. It gets redefined outside. It just made sense to name
> it like this 8-)
Well, if $project is local in evaluate_path_info(), so could be
$path_info...
>>> + $project =~ s,/+$,,;
>>> + while ($project && !check_head_link("$projectroot/$project")) {
>>> + $project =~ s,/*[^/]*$,,;
>>> + }
>>> + # validate project
>>> + $project = validate_project($project);
>>
>> I'm not sure if it is worth worrying over, but I think you repeat
>> check_head_link() check here.
>>
>> [After examining code further]. But I think you do double validation;
>> once you do it here, and once you do it copying to global variables
>> such as $action or $project, and double checking check_head_link()
>> won't be easy to avoid; fortunately it is cheap filesystem-level check
>> (might be slow only when stat is extremely slow, and is not cached).
>
> I know. This is actually the reason why I had interleaved path_info
> definition and global validation in my previous version of the patch.
> The big issue here is that path_info evaluation _needs_ (partial)
> validation.
>
> A possible alternative could be to only put validated parameters into
> %input_params. This would completely separate the validation for cgi
> and path_info (modulo shared subs).
>
> Of course, the check_head_link would still be repeated inside
> evaluate_path_info, but the other params could skip a double
> validation.
Wouldn't it be simpler and as good solution to just leave validation
off evaluate_path_info() (well, of course except check_head_link() test),
and allow it to be validated when assigning global 'params' variables?
check_head_link() would be repeated for path_info links, but that
should not affect performance much.
--
Jakub Narebski
Poland
--
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
