Re: [PATCH] gitweb: check if-modified-since for feeds

[Jakub Narebski <jnareb@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.

> ---
>  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?

> +			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');

> +				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