gitweb: refactor input parameters parse/validation
Since input parameters can be obtained both from CGI parameters and PATH_INFO, we would like most of the code to be agnostic about the way parameters were retrieved. We thus collect all the parameters into the new %input_params hash, delaying validation after the collection is completed. Although the kludge removal is minimal at the moment, it makes life much easier for future expansions such as more extensive PATH_INFO use or other form of input such as command-line support. Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com> Acked-by: Jakub Narebski <jnareb@gmail.com> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Giuseppe Bilotta committed
Oct 10, 2008 at 20:42 UTC
1b2d297e41dbd12c56646796c3d3bcf190f0d5d4
1 file changed
+178
-137
gitweb/gitweb.perl
+178
-137
@@ -29,7 +29,9 @@ our $my_uri = $cgi->url(-absolute => 1);
29
30
# if we're called with PATH_INFO, we have to strip that
31
# from the URL to find our real URL
32
-if (my $path_info = $ENV{"PATH_INFO"}) {
32
+# we make $path_info global because it's also used later on
33
+my $path_info = $ENV{"PATH_INFO"};
34
+if ($path_info) {
35
$my_url =~ s,\Q$path_info\E$,,;
36
$my_uri =~ s,\Q$path_info\E$,,;
37
}
@@ -428,34 +430,155 @@ $projects_list ||= $projectroot;
430
431
# ======================================================================
432
# input validation and dispatch
431
-our $action = $cgi->param('a');
433
+
434
+# input parameters can be collected from a variety of sources (presently, CGI
435
+# and PATH_INFO), so we define an %input_params hash that collects them all
436
+# together during validation: this allows subsequent uses (e.g. href()) to be
437
+# agnostic of the parameter origin
438
+
439
+my %input_params = ();
440
+
441
+# input parameters are stored with the long parameter name as key. This will
442
+# also be used in the href subroutine to convert parameters to their CGI
443
+# equivalent, and since the href() usage is the most frequent one, we store
444
+# the name -> CGI key mapping here, instead of the reverse.
445
+#
446
+# XXX: Warning: If you touch this, check the search form for updating,
447
+# too.
448
+
449
+my @cgi_param_mapping = (
450
+ project => "p",
451
+ action => "a",
452
+ file_name => "f",
453
+ file_parent => "fp",
454
+ hash => "h",
455
+ hash_parent => "hp",
456
+ hash_base => "hb",
457
+ hash_parent_base => "hpb",
458
+ page => "pg",
459
+ order => "o",
460
+ searchtext => "s",
461
+ searchtype => "st",
462
+ snapshot_format => "sf",
463
+ extra_options => "opt",
464
+ search_use_regexp => "sr",
465
+);
466
+my %cgi_param_mapping = @cgi_param_mapping;
467
+
468
+# we will also need to know the possible actions, for validation
469
+my %actions = (
470
+ "blame" => \&git_blame,
471
+ "blobdiff" => \&git_blobdiff,
472
+ "blobdiff_plain" => \&git_blobdiff_plain,
473
+ "blob" => \&git_blob,
474
+ "blob_plain" => \&git_blob_plain,
475
+ "commitdiff" => \&git_commitdiff,
476
+ "commitdiff_plain" => \&git_commitdiff_plain,
477
+ "commit" => \&git_commit,
478
+ "forks" => \&git_forks,
479
+ "heads" => \&git_heads,
480
+ "history" => \&git_history,
481
+ "log" => \&git_log,
482
+ "rss" => \&git_rss,
483
+ "atom" => \&git_atom,
484
+ "search" => \&git_search,
485
+ "search_help" => \&git_search_help,
486
+ "shortlog" => \&git_shortlog,
487
+ "summary" => \&git_summary,
488
+ "tag" => \&git_tag,
489
+ "tags" => \&git_tags,
490
+ "tree" => \&git_tree,
491
+ "snapshot" => \&git_snapshot,
492
+ "object" => \&git_object,
493
+ # those below don't need $project
494
+ "opml" => \&git_opml,
495
+ "project_list" => \&git_project_list,
496
+ "project_index" => \&git_project_index,
497
+);
498
+
499
+# finally, we have the hash of allowed extra_options for the commands that
500
+# allow them
501
+my %allowed_options = (
502
+ "--no-merges" => [ qw(rss atom log shortlog history) ],
503
+);
504
+
505
+# fill %input_params with the CGI parameters. All values except for 'opt'
506
+# should be single values, but opt can be an array. We should probably
507
+# build an array of parameters that can be multi-valued, but since for the time
508
+# being it's only this one, we just single it out
509
+while (my ($name, $symbol) = each %cgi_param_mapping) {
510
+ if ($symbol eq 'opt') {
511
+ $input_params{$name} = [ $cgi->param($symbol) ];
512
+ } else {
513
+ $input_params{$name} = $cgi->param($symbol);
514
+ }
515
+}
516
+
517
+# now read PATH_INFO and update the parameter list for missing parameters
518
+sub evaluate_path_info {
519
+ return if defined $input_params{'project'};
520
+ return if !$path_info;
521
+ $path_info =~ s,^/+,,;
522
+ return if !$path_info;
523
+
524
+ # find which part of PATH_INFO is project
525
+ my $project = $path_info;
526
+ $project =~ s,/+$,,;
527
+ while ($project && !check_head_link("$projectroot/$project")) {
528
+ $project =~ s,/*[^/]*$,,;
529
+ }
530
+ return unless $project;
531
+ $input_params{'project'} = $project;
532
+
533
+ # do not change any parameters if an action is given using the query string
534
+ return if $input_params{'action'};
535
+ $path_info =~ s,^\Q$project\E/*,,;
536
+
537
+ my ($refname, $pathname) = split(/:/, $path_info, 2);
538
+ if (defined $pathname) {
539
+ # we got "project.git/branch:filename" or "project.git/branch:dir/"
540
+ # we could use git_get_type(branch:pathname), but it needs $git_dir
541
+ $pathname =~ s,^/+,,;
542
+ if (!$pathname || substr($pathname, -1) eq "/") {
543
+ $input_params{'action'} = "tree";
544
+ $pathname =~ s,/$,,;
545
+ } else {
546
+ $input_params{'action'} = "blob_plain";
547
+ }
548
+ $input_params{'hash_base'} ||= $refname;
549
+ $input_params{'file_name'} ||= $pathname;
550
+ } elsif (defined $refname) {
551
+ # we got "project.git/branch"
552
+ $input_params{'action'} = "shortlog";
553
+ $input_params{'hash'} ||= $refname;
554
+ }
555
+}
556
+evaluate_path_info();
557
+
558
+our $action = $input_params{'action'};
559
if (defined $action) {
433
- if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
560
+ if (!validate_action($action)) {
561
die_error(400, "Invalid action parameter");
562
}
563
}
564
565
# parameters which are pathnames
439
-our $project = $cgi->param('p');
566
+our $project = $input_params{'project'};
567
if (defined $project) {
441
- if (!validate_pathname($project) ||
442
- !(-d "$projectroot/$project") ||
443
- !check_head_link("$projectroot/$project") ||
444
- ($export_ok && !(-e "$projectroot/$project/$export_ok")) ||
445
- ($strict_export && !project_in_list($project))) {
568
+ if (!validate_project($project)) {
569
undef $project;
570
die_error(404, "No such project");
571
}
572
}
573
451
-our $file_name = $cgi->param('f');
574
+our $file_name = $input_params{'file_name'};
575
if (defined $file_name) {
576
if (!validate_pathname($file_name)) {
577
die_error(400, "Invalid file parameter");
578
}
579
}
580
458
-our $file_parent = $cgi->param('fp');
581
+our $file_parent = $input_params{'file_parent'};
582
if (defined $file_parent) {
583
if (!validate_pathname($file_parent)) {
584
die_error(400, "Invalid file parent parameter");
@@ -463,44 +586,41 @@ if (defined $file_parent) {
586
}
587
588
# parameters which are refnames
466
-our $hash = $cgi->param('h');
589
+our $hash = $input_params{'hash'};
590
if (defined $hash) {
591
if (!validate_refname($hash)) {
592
die_error(400, "Invalid hash parameter");
593
}
594
}
595
473
-our $hash_parent = $cgi->param('hp');
596
+our $hash_parent = $input_params{'hash_parent'};
597
if (defined $hash_parent) {
598
if (!validate_refname($hash_parent)) {
599
die_error(400, "Invalid hash parent parameter");
600
}
601
}
602
480
-our $hash_base = $cgi->param('hb');
603
+our $hash_base = $input_params{'hash_base'};
604
if (defined $hash_base) {
605
if (!validate_refname($hash_base)) {
606
die_error(400, "Invalid hash base parameter");
607
}
608
}
609
487
-my %allowed_options = (
488
- "--no-merges" => [ qw(rss atom log shortlog history) ],
489
-);
490
-
491
-our @extra_options = $cgi->param('opt');
492
-if (defined @extra_options) {
493
- foreach my $opt (@extra_options) {
494
- if (not exists $allowed_options{$opt}) {
495
- die_error(400, "Invalid option parameter");
496
- }
497
- if (not grep(/^$action$/, @{$allowed_options{$opt}})) {
498
- die_error(400, "Invalid option parameter for this action");
499
- }
610
+our @extra_options = @{$input_params{'extra_options'}};
611
+# @extra_options is always defined, since it can only be (currently) set from
612
+# CGI, and $cgi->param() returns the empty array in array context if the param
613
+# is not set
614
+foreach my $opt (@extra_options) {
615
+ if (not exists $allowed_options{$opt}) {
616
+ die_error(400, "Invalid option parameter");
617
+ }
618
+ if (not grep(/^$action$/, @{$allowed_options{$opt}})) {
619
+ die_error(400, "Invalid option parameter for this action");
620
}
621
}
622
503
-our $hash_parent_base = $cgi->param('hpb');
623
+our $hash_parent_base = $input_params{'hash_parent_base'};
624
if (defined $hash_parent_base) {
625
if (!validate_refname($hash_parent_base)) {
626
die_error(400, "Invalid hash parent base parameter");
@@ -508,23 +628,23 @@ if (defined $hash_parent_base) {
628
}
629
630
# other parameters
511
-our $page = $cgi->param('pg');
631
+our $page = $input_params{'page'};
632
if (defined $page) {
633
if ($page =~ m/[^0-9]/) {
634
die_error(400, "Invalid page parameter");
635
}
636
}
637
518
-our $searchtype = $cgi->param('st');
638
+our $searchtype = $input_params{'searchtype'};
639
if (defined $searchtype) {
640
if ($searchtype =~ m/[^a-z]/) {
641
die_error(400, "Invalid searchtype parameter");
642
}
643
}
644
525
-our $search_use_regexp = $cgi->param('sr');
645
+our $search_use_regexp = $input_params{'search_use_regexp'};
646
527
-our $searchtext = $cgi->param('s');
647
+our $searchtext = $input_params{'searchtext'};
648
our $search_regexp;
649
if (defined $searchtext) {
650
if (length($searchtext) < 2) {
@@ -533,86 +653,11 @@ if (defined $searchtext) {
653
$search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;
654
}
655
536
-# now read PATH_INFO and use it as alternative to parameters
537
-sub evaluate_path_info {
538
- return if defined $project;
539
- my $path_info = $ENV{"PATH_INFO"};
540
- return if !$path_info;
541
- $path_info =~ s,^/+,,;
542
- return if !$path_info;
543
- # find which part of PATH_INFO is project
544
- $project = $path_info;
545
- $project =~ s,/+$,,;
546
- while ($project && !check_head_link("$projectroot/$project")) {
547
- $project =~ s,/*[^/]*$,,;
548
- }
549
- # validate project
550
- $project = validate_pathname($project);
551
- if (!$project ||
552
- ($export_ok && !-e "$projectroot/$project/$export_ok") ||
553
- ($strict_export && !project_in_list($project))) {
554
- undef $project;
555
- return;
556
- }
557
- # do not change any parameters if an action is given using the query string
558
- return if $action;
559
- $path_info =~ s,^\Q$project\E/*,,;
560
- my ($refname, $pathname) = split(/:/, $path_info, 2);
561
- if (defined $pathname) {
562
- # we got "project.git/branch:filename" or "project.git/branch:dir/"
563
- # we could use git_get_type(branch:pathname), but it needs $git_dir
564
- $pathname =~ s,^/+,,;
565
- if (!$pathname || substr($pathname, -1) eq "/") {
566
- $action ||= "tree";
567
- $pathname =~ s,/$,,;
568
- } else {
569
- $action ||= "blob_plain";
570
- }
571
- $hash_base ||= validate_refname($refname);
572
- $file_name ||= validate_pathname($pathname);
573
- } elsif (defined $refname) {
574
- # we got "project.git/branch"
575
- $action ||= "shortlog";
576
- $hash ||= validate_refname($refname);
577
- }
578
-}
579
-evaluate_path_info();
580
-
656
# path to the current git repository
657
our $git_dir;
658
$git_dir = "$projectroot/$project" if $project;
659
660
# dispatch
586
-my %actions = (
587
- "blame" => \&git_blame,
588
- "blobdiff" => \&git_blobdiff,
589
- "blobdiff_plain" => \&git_blobdiff_plain,
590
- "blob" => \&git_blob,
591
- "blob_plain" => \&git_blob_plain,
592
- "commitdiff" => \&git_commitdiff,
593
- "commitdiff_plain" => \&git_commitdiff_plain,
594
- "commit" => \&git_commit,
595
- "forks" => \&git_forks,
596
- "heads" => \&git_heads,
597
- "history" => \&git_history,
598
- "log" => \&git_log,
599
- "rss" => \&git_rss,
600
- "atom" => \&git_atom,
601
- "search" => \&git_search,
602
- "search_help" => \&git_search_help,
603
- "shortlog" => \&git_shortlog,
604
- "summary" => \&git_summary,
605
- "tag" => \&git_tag,
606
- "tags" => \&git_tags,
607
- "tree" => \&git_tree,
608
- "snapshot" => \&git_snapshot,
609
- "object" => \&git_object,
610
- # those below don't need $project
611
- "opml" => \&git_opml,
612
- "project_list" => \&git_project_list,
613
- "project_index" => \&git_project_index,
614
-);
615
-
661
if (!defined $action) {
662
if (defined $hash) {
663
$action = git_get_type($hash);
@@ -642,35 +687,12 @@ sub href (%) {
687
# default is to use -absolute url() i.e. $my_uri
688
my $href = $params{-full} ? $my_url : $my_uri;
689
645
- # XXX: Warning: If you touch this, check the search form for updating,
646
- # too.
647
-
648
- my @mapping = (
649
- project => "p",
650
- action => "a",
651
- file_name => "f",
652
- file_parent => "fp",
653
- hash => "h",
654
- hash_parent => "hp",
655
- hash_base => "hb",
656
- hash_parent_base => "hpb",
657
- page => "pg",
658
- order => "o",
659
- searchtext => "s",
660
- searchtype => "st",
661
- snapshot_format => "sf",
662
- extra_options => "opt",
663
- search_use_regexp => "sr",
664
- );
665
- my %mapping = @mapping;
666
-
690
$params{'project'} = $project unless exists $params{'project'};
691
692
if ($params{-replay}) {
670
- while (my ($name, $symbol) = each %mapping) {
693
+ while (my ($name, $symbol) = each %cgi_param_mapping) {
694
if (!exists $params{$name}) {
672
- # to allow for multivalued params we use arrayref form
673
- $params{$name} = [ $cgi->param($symbol) ];
695
+ $params{$name} = $input_params{$name};
696
}
697
}
698
}
@@ -689,8 +711,8 @@ sub href (%) {
711
712
# now encode the parameters explicitly
713
my @result = ();
692
- for (my $i = 0; $i < @mapping; $i += 2) {
693
- my ($name, $symbol) = ($mapping[$i], $mapping[$i+1]);
714
+ for (my $i = 0; $i < @cgi_param_mapping; $i += 2) {
715
+ my ($name, $symbol) = ($cgi_param_mapping[$i], $cgi_param_mapping[$i+1]);
716
if (defined $params{$name}) {
717
if (ref($params{$name}) eq "ARRAY") {
718
foreach my $par (@{$params{$name}}) {
@@ -710,6 +732,25 @@ sub href (%) {
732
## ======================================================================
733
## validation, quoting/unquoting and escaping
734
735
+sub validate_action {
736
+ my $input = shift || return undef;
737
+ return undef unless exists $actions{$input};
738
+ return $input;
739
+}
740
+
741
+sub validate_project {
742
+ my $input = shift || return undef;
743
+ if (!validate_pathname($input) ||
744
+ !(-d "$projectroot/$input") ||
745
+ !check_head_link("$projectroot/$input") ||
746
+ ($export_ok && !(-e "$projectroot/$input/$export_ok")) ||
747
+ ($strict_export && !project_in_list($input))) {
748
+ return undef;
749
+ } else {
750
+ return $input;
751
+ }
752
+}
753
+
754
sub validate_pathname {
755
my $input = shift || return undef;
756
@@ -4121,7 +4162,7 @@ sub git_search_grep_body {
4162
## actions
4163
4164
sub git_project_list {
4124
- my $order = $cgi->param('o');
4165
+ my $order = $input_params{'order'};
4166
if (defined $order && $order !~ m/none|project|descr|owner|age/) {
4167
die_error(400, "Unknown order parameter");
4168
}
@@ -4149,7 +4190,7 @@ sub git_project_list {
4190
}
4191
4192
sub git_forks {
4152
- my $order = $cgi->param('o');
4193
+ my $order = $input_params{'order'};
4194
if (defined $order && $order !~ m/none|project|descr|owner|age/) {
4195
die_error(400, "Unknown order parameter");
4196
}
@@ -4697,7 +4738,7 @@ sub git_snapshot {
4738
my @supported_fmts = gitweb_check_feature('snapshot');
4739
@supported_fmts = filter_snapshot_fmts(@supported_fmts);
4740
4700
- my $format = $cgi->param('sf');
4741
+ my $format = $input_params{'snapshot_format'};
4742
if (!@supported_fmts) {
4743
die_error(403, "Snapshots not allowed");
4744
}