Re: [PATCH] gitweb: check if-modified-since for feeds
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Jan 26, 2009, 02:18 UTC
- Message-ID
- <200901260318.05301.jnareb@gmail.com>
- In-Reply-To
- <1232923370-4427-2-git-send-email-giuseppe.bilotta@gmail.com>
Should be "[PATCH 2/2]" or similar, just in case.
On Sun, 25 Jun 2009, Giuseppe Bilotta wrote:
> Offering Last-modified header
And skipping generating the body if client uses 'HEAD' request to get only Last-Modified header.
> for feeds is only half the work: we should > also check that same date against If-modified-since, and bail out early > with 304 Not Modified.
Lacks signoff.
Show 23 quoted lines
> ---
> gitweb/gitweb.perl | 20 +++++++++++++++++++-
> 1 files changed, 19 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 8c49c75..0a5d229 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -6015,7 +6015,25 @@ sub git_feed {
> }
> if (defined($commitlist[0])) {
> %latest_commit = %{$commitlist[0]};
> - %latest_date = parse_date($latest_commit{'committer_epoch'});
> + my $latest_epoch = $latest_commit{'committer_epoch'};
> + %latest_date = parse_date($latest_epoch);
> + 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);
> + }I'd really like to fallback on hand-parsing, as we have to parse date in well defined HTTP-date format (RFC-1123, update to RFC-822), which I think is what we send in Last-Modified header (or is it RFC-2822?).
But that might be too much work. I like the checking for modules, and the fallback cascade, but could you explain why in this order?
Show 6 quoted lines
> + if (defined $since && $latest_epoch <= $since) {
> + print $cgi->header(
> + -type => $content_type,
> + -charset => 'utf-8',
> + -last_modified => $latest_date{'rfc2822'},
> + -status => 304);I think we spell HTTP status messages in full (even if it is hidden in die_error subroutine), i.e.
+ -status => '304 Not Modified');
Show 10 quoted lines
> + return; > + } > + } > print $cgi->header( > -type => $content_type, > -charset => 'utf-8', > -- > 1.5.6.5 > >
P.S. It would be nice to have this mechanism (responding to cache-control headers such as If-Modified-Since) for all of gitweb, but I guess it is most critical for feeds, which are _polled_.
-- Jakub Narebski Poland