[PATCH v2] Pull gitweb If-Modified-Since handling out into its own function and use for snapshots.
From: W. Trevor King <hidden>
Date: 2016-06-15 22:53:20
Subsystem:
the rest · Maintainer:
Linus Torvalds
The current gitweb only generates Last-Modified and handles If-Modified-Since headers for the git_feed action. This patch breaks the Last-Modified and If-Modified-Since handling code out from git_feed into a new function modified_since. This makes the code easy to reuse for other actions where it is appropriate and adds the code to do that to git_snapshot. Signed-off-by: W Trevor King <redacted> --- Thanks for the feedback, Jakub. I believe I've addressed your points in this revised patch. Changed since v1: - Fixed impersonal/personal commit message - Simplified modified_since() -> die_if_unmodified(). Now it only takes a $latest_epoch argument and we set fewer headers. It also jump straight to DONE_GITWEB if the page is unmodified. - Ran tests (which passed this time). Somehow I missed t/README earlier. I'd add tests for my patch, but I'd have to have commit timestamps from the test repository, and I don't understand the test framework well enough to know how to do that cleanly. Things that haven't changed:
quoted
@@ -7038,9 +7071,11 @@ sub git_snapshot { } $filename =~ s/(["\\])/\\$1/g; + my %latest_date = parse_date($co{'committer_epoch'}, $co{'committer_tz'});... This simply has no way of working, as %co variable you use here is not defined anywhere in git_snapshot() subroutine.
I defined it earlier:
quoted
@@ -7029,6 +7054,14 @@ sub git_snapshot { my ($name, $prefix) = snapshot_name($project, $hash); my $filename = "$name$known_snapshot_formats{$format}{'suffix'}"; + + my %co = parse_commit($hash) or die_error(404, "Unknown commit object"); + if (! modified_since($known_snapshot_formats{$format}{'type'}, + $co{'committer_epoch'},
While modified_since() (now die_if_unmodified()) handles the 304 case, we also want to set Last-Modified for 200s. That's what the latter code is trying to do.
What if there is no commit, for example if we are rewuesting snapshot of a tree by its SHA-1? We need to be able to deal with sich situation, if only by not handling Last-Modified / If-Modified-Since.
The parse_commit() call should handle these cases.
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a8b5fad..b944351 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl@@ -7003,6 +7003,28 @@ sub snapshot_name { return wantarray ? ($name, $name) : $name; } +sub die_if_unmodified { + my ($latest_epoch) = @_; + our $cgi; + + my $if_modified = $cgi->http('IF_MODIFIED_SINCE'); + if (defined $if_modified) { + my $since; + if (eval { require HTTP::Date; 1; }) { + $since = HTTP::Date::str2time($if_modified); + } elsif (eval { require Time::ParseDate; 1; }) { + $since = Time::ParseDate::parsedate($if_modified, GMT => 1); + } + if (defined $since && $latest_epoch <= $since) { + my %latest_date = parse_date($latest_epoch); + print $cgi->header( + -last_modified => $latest_date{'rfc2822'}, + -status => '304 Not Modified'); + goto DONE_GITWEB; + } + } +} + sub git_snapshot { my $format = $input_params{'snapshot_format'}; if (!@snapshot_fmts) {
@@ -7029,6 +7051,10 @@ sub git_snapshot { my ($name, $prefix) = snapshot_name($project, $hash); my $filename = "$name$known_snapshot_formats{$format}{'suffix'}"; + + my %co = parse_commit($hash) or die_error(404, "Unknown commit object"); + die_if_unmodified($co{'committer_epoch'}); + my $cmd = quote_command( git_cmd(), 'archive', "--format=$known_snapshot_formats{$format}{'format'}",
@@ -7038,9 +7064,11 @@ sub git_snapshot { } $filename =~ s/(["\\])/\\$1/g; + my %latest_date = parse_date($co{'committer_epoch'}, $co{'committer_tz'}); print $cgi->header( -type => $known_snapshot_formats{$format}{'type'}, -content_disposition => 'inline; filename="' . $filename . '"', + -last_modified => $latest_date{'rfc2822'}, -status => '200 OK'); open my $fd, "-|", $cmd
@@ -7820,24 +7848,8 @@ sub git_feed { if (defined($commitlist[0])) { %latest_commit = %{$commitlist[0]}; my $latest_epoch = $latest_commit{'committer_epoch'}; + die_if_unmodified($latest_epoch); %latest_date = parse_date($latest_epoch, $latest_commit{'comitter_tz'}); - my $if_modified = $cgi->http('IF_MODIFIED_SINCE'); - if (defined $if_modified) { - my $since; - if (eval { require HTTP::Date; 1; }) { - $since = HTTP::Date::str2time($if_modified); - } elsif (eval { require Time::ParseDate; 1; }) { - $since = Time::ParseDate::parsedate($if_modified, GMT => 1); - } - if (defined $since && $latest_epoch <= $since) { - print $cgi->header( - -type => $content_type, - -charset => 'utf-8', - -last_modified => $latest_date{'rfc2822'}, - -status => '304 Not Modified'); - return; - } - } print $cgi->header( -type => $content_type, -charset => 'utf-8', --
1.7.3.4
Attachments
- signature.asc [application/pgp-signature] 490 bytes