[PATCH] gitweb: new cgi parameter: opt

From

Miklos Vajna <vmiklos@frugalware.org>

Date

Jul 12, 2007, 18:39 UTC

Message-ID

<20070712183927.GS19386@genesis.frugalware.org>

In-Reply-To

<200707121211.32813.jnareb@gmail.com>

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

Hello,

Na Thu, Jul 12, 2007 at 12:11:32PM +0200, Jakub Narebski <jnareb@gmail.com> pisal(a):

> 'log', 'shortlog' and 'history' actions, but it can be easily extended to allow
> Micronit: it is unwritten (as of yet) requirement to word wrap commit
> message at 80 columns or less.

OK, though I think 79 is valid for "80 or less" :-)

> By the way, there is t9500-gitweb-standalone-no-errors.sh test script to
> check if gitweb doesn't give any Perl warnings or errors. Please try to
> use it; it should at least find errors about undefined values and such.
> But it has the disadvantage of requiring git to be build (compiled),
> even if theoretically testing gitweb doesn't require it.
>
> You would see something like
>
>   Argument "nomerges" isn't numeric in numeric eq (==)
>
> when running this test with --debug, I think.

As far as i see the test passes without any such warning ATM.

> First, you don't need inner () parentheses to delimit/create list, as
> anonymous array reference constructor [] works like it. So it could be
> written simply as
>
>   +my %options = (
>   +   "--no-merges" => ['rss', 'atom', 'log', 'shortlog', 'history'],
>   +);
>
> Second, instead of quoting each word by hand, we can use handy Perl
> quoting operator, qw(), i.e. 'word list' operator, like below.
> See perlop(1), "Quote and Quote-like Operators" subsection
>
>   +my %options = (
>   +   "--no-merges" => [ qw(rss atom log shortlog history) ],
>   +);

Fixed.

> history view. So I'd use
>
>   +our @options = $cgi->param('option');
>
> instead. This would make option validation bit harder, but I think not that
> harder. But it would also make using extra options easier: just @options
> instead of (defined $option ? $option : ()).

Done.

> I'd also use @extra_options, or @act_opts instead of @options as
> a variable name to be more descriptive.

Changed. Also renamed %options to %allowed_options.

> I'm also not sure if invalid option parameter for action should return
> error, or be simply ignored. This allow to hand-edit URL, changing for
> example action from 'commitdiff' to 'commit', not worrying about spurious
> parameters.

> By the way, gitweb uses shortened names for paramaters. Perhaps 'opt'
> or 'op' instead of 'options' here and in href subroutine (below)?

Changed to 'opt'.

 gitweb/gitweb.perl |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 27580b5..13134fe 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -386,6 +386,23 @@ if (defined $hash_base) {
 	}
 }
 
+my %allowed_options = (
+	"--no-merges" => [ qw(rss atom log shortlog history) ],
+);
+
+our @extra_options = $cgi->param('opt');
+if (defined @extra_options) {
+	foreach(@extra_options)
+	{
+		if (not grep(/^$_$/, keys %allowed_options)) {
+			die_error(undef, "Invalid option parameter");
+		}
+		if (not grep(/^$action$/, @{$allowed_options{$_}})) {
+			die_error(undef, "Invalid option parameter for this action");
+		}
+	}
+}
+
 our $hash_parent_base = $cgi->param('hpb');
 if (defined $hash_parent_base) {
 	if (!validate_refname($hash_parent_base)) {
@@ -537,6 +554,7 @@ sub href(%) {
 		action => "a",
 		file_name => "f",
 		file_parent => "fp",
+		extra_options => "opt",
 		hash => "h",
 		hash_parent => "hp",
 		hash_base => "hb",
@@ -1773,6 +1791,7 @@ sub parse_commits {
 		($arg ? ($arg) : ()),
 		("--max-count=" . $maxcount),
 		("--skip=" . $skip),
+		@extra_options,
 		$commit_id,
 		"--",
 		($filename ? ($filename) : ())

-- 
1.5.3.rc0.39.g46f7-dirty