Re: [PATCH v7 3/3] gitweb: add If-Modified-Since handling to git_snapshot().

From: W. Trevor King <wking@drexel.edu>
Date: Mar 28, 2012, 18:37 UTC
Message-ID: <20120328183712.GA5992@odin.tremily.us>
In-Reply-To: <201203282011.32148.jnareb@gmail.com>

On Wed, Mar 28, 2012 at 07:11:31PM +0100, Jakub Narebski wrote:

Show 15 quoted lines

> W. Trevor King wrote:
> >  	print $cgi->header(
> >  		-type => $known_snapshot_formats{$format}{'type'},
> >  		-content_disposition => 'inline; filename="' . $filename . '"',
> > +		-last_modified => (%co ? $latest_date{'rfc2822'} : ()),
> >  		-status => '200 OK');
> 
> I'm sorry to be bearer of bad news, but this is still incorrect.
> It should be:
> 
>    	print $cgi->header(
>    		-type => $known_snapshot_formats{$format}{'type'},
>    		-content_disposition => 'inline; filename="' . $filename . '"',
>   +		%co ? (-last_modified => $latest_date{'rfc2822'}) : (),
>    		-status => '200 OK');

Grr. Thanks. I'm getting lots of rebase practice on this patch set, but I'm still missing things…

Show 8 quoted lines

> And it was not caught by test because CGI.pm can output the last modified
> header as "Last-modified" (RFC 2616, sec 4.2 states "Field names are
> case-insensitive"), so the last check should be
> 
>   +	! grep -i "Last-Modified" gitweb.output
> 
> Hmmm... why we use gitweb.output and not gitweb.headers?  Is it consistency
> with earlier tests?

Yes, but I can switch to `gitweb.headers` if you'd like. Should I adjust all the header tests in t9501 to use `gitweb.headers` and `grep -i`? It should probably be a separate patch for the tests that existed before my i-m-s additions.

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy