Thread (4 messages) flat view 4 messages, 3 authors, 2016-06-15

Re: [RFCv3 1/2] gitweb: add patch view

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:43

Giuseppe Bilotta [off-list ref] writes:
quoted hunk
The manually-built email format in commitdiff_plain output is not
appropriate for feeding git-am, because of two limitations:
 * when a range of commits is specified, commitdiff_plain publishes a
   single patch with the message from the first commit, instead of a
   patchset,
 * in either case, the patch summary is replicated both as email subject
   and as first line of the email itself, resulting in a doubled summary
   if the output is fed to git-am.

We thus create a new view that can be fed to git-am directly by exposing
the output of git format-patch directly. This allows patch exchange and
submission via gitweb.

A configurable limit is imposed on the number of commits which will be
included in a patchset, to prevent DoS attacks on the server. Setting
the limit to 0 will disable the patch view, setting it to a negative
number will remove the limit.

Signed-off-by: Giuseppe Bilotta <redacted>
---
 gitweb/gitweb.perl |   41 ++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 40 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2738643..c9abfcf 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -329,6 +329,13 @@ our %feature = (
 	'ctags' => {
 		'override' => 0,
 		'default' => [0]},
+
+	# The maximum number of patches in a patchset generated in patch
+	# view. Set this to 0 or undef to disable patch view, or to a
+	# negative number to remove any limit.
+	'patches' => {
+		'override' => 1,
+		'default' => [16]},
 );
Looking at the existing entries in the %feature hash, it seems that it is
our tradition that a new feature starts as disabled and not overridable
(see 'ctags' in the context above).
 sub git_commitdiff {
 	my $format = shift || 'html';
+
+	my $patch_max = gitweb_check_feature('patches');
+	if ($format eq 'patch') {
+		die_error(403, "Patch view not allowed") unless $patch_max;
+	}
+
Should you have to pay overhead for the check-feature call even when
the $format is not "patch"?
quoted hunk
@@ -5396,6 +5410,7 @@ sub git_commitdiff {
 	}
 	# we need to prepare $formats_nav before almost any parameter munging
 	my $formats_nav;
+
Noise.
quoted hunk
@@ -5532,6 +5552,15 @@ sub git_commitdiff {
 			print to_utf8($line) . "\n";
 		}
 		print "---\n\n";
+	} elsif ($format eq 'patch') {
+		my $filename = basename($project) . "-$hash.patch";
+
+		print $cgi->header(
+			-type => 'text/plain',
+			-charset => 'utf-8',
+			-expires => $expires,
+			-content_disposition => 'inline; filename="' . "$filename" . '"');
+		# TODO add X-Git-Tag/X-Git-Url headers in a sensible way
A stupid question.  Are you talking about sending these X-Foo as extra
HTTP headers?  What good would they do (iow what will they be used for by
the receiving browser/wget)?

Other than that the patch seems quite straightforward and was a pleasant
read.  Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help