Thread (7 messages) flat view 7 messages, 2 authors, 2016-06-15

Re: [PATCH 1/3] gitweb: Add option to force version match

From: Jakub Narebski <hidden>
Date: 2016-06-15 22:49:55

"John 'Warthog9' Hawley" [off-list ref] writes:
This adds $git_versions_must_match variable, which is set to true,
checks that we are running on the same version of git that we
shipped with, and if not throw '500 Internal Server Error' error.
What is checked is the version of gitweb (embedded in building
gitweb.cgi), against version of runtime git binary used.

Gitweb can usually run with a mismatched git install.  This is more
here to give an obvious warning as to whats going on vs. silently
failing.
Were you bitten by mismatch between git version and gitweb version?
Just how serious is that (hypothetical?) situation?

Should it be warning, or erroring out?
By default this feature is turned on.
I think last time this patch stalled on discussion whether this
feature should be turned on by default, where it can break some
setups; or be turned off by default, where it is much less useful
protecting against errors.
Signed-off-by: John 'Warthog9' Hawley <warthog9@kernel.org>
Signed-off-by: Jakub Narebski <redacted>
---
 gitweb/README      |    4 ++++
 gitweb/gitweb.perl |   27 +++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 0 deletions(-)
For me, if $git_versions_must_match is to be on by default, I would
prefer also update to t/gitweb-lib.sh, so hat I don't have to
recompile git to test new version of gitweb...  
quoted hunk ↗ jump to hunk
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8d7e4c5..215a4e9 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -232,6 +232,9 @@ our %avatar_size = (
 	'double'  => 32
 );
 
+# If it is true, exit if gitweb version and git binary version don't match
+our $git_versions_must_match = 1;
+
 # Used to set the maximum load that we will still respond to gitweb queries.
 # If server load exceed this value then return "503 server busy" error.
 # If gitweb cannot determined server load, it is taken to be 0.
@@ -649,6 +652,29 @@ sub check_loadavg {
 	}
 }
 
+sub check_versionmatch {
+	# Throw an error if git versions does not match, if $git_versions_must_match is true.
+	if ($git_versions_must_match &&
+	    $git_version ne $version) {
+		my $admin_contact =
+			defined $ENV{'SERVER_ADMIN'} ? ", $ENV{'SERVER_ADMIN'}," : '';
+		my $err_msg = <<EOT;
+<h1 align="center">*** Warning ***</h1>
+<p>
+This version of gitweb was compiled for <b>@{[esc_html($version)]}</b>,
+however git version <b>@{[esc_html($git_version)]}</b> was found on server,
+and administrator requested strict version checking.
+</p>
+<p>
+Please contact the server administrator${admin_contact} to either configure
+gitweb to allow mismatched versions, or update git or gitweb installation.
+</p>
+EOT
+		die_error(500, 'Internal server error', $err_msg);
+	}
+
+}
Nice.
quoted hunk ↗ jump to hunk
+
 # ======================================================================
 # input validation and dispatch
 
@@ -1075,6 +1101,7 @@ sub run_request {
 	evaluate_uri();
 	evaluate_gitweb_config();
 	check_loadavg();
+	check_versionmatch();
Shouldn't it be before check_loadavg()?

-- 
Jakub Narebski
Poland
ShadeHawk on #git
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help