mrclay / minify

Combines. minifies, and serves CSS or Javascript files
BSD 3-Clause "New" or "Revised" License
3.01k stars 473 forks source link

Last-Modified Header Not Added to JS/CSS Files #181

Closed mrclay closed 9 years ago

mrclay commented 9 years ago

Originally reported on Google Code with ID 36

What version of Minify are you using? PHP version? 2.0.1

What steps will reproduce the problem?
1. Clear browser cache
2. request JS or CSS files generated by Minify
3. note that there is no Last-Modified header on the file

What is the expected output? What do you see instead?
I'm expecting to see a "304 Not Modified" response when requesting the same
CSS or JS file in subsequent requests.  As background, in FF and IE at
least future requests will not include an If-Modified-Since header unless
there was a Last-Modified header included in a previous response.  As a
result my browser is requesting and downloading the JS/CSS files on every
page request.

Please provide any additional information below.
If this is not the intended behavior, I noticed that the constructor for
HTTP/ConditionalGet.php is returning before checking for the
'lastModifiedTime' array element in the $spec variable passed to it.  It's
the presence of this variable that will trigger the Last-Modified header. 
From what I can see this code simply never executes.  See snippet below.

        if (isset($spec['setExpires'])) {
            if (is_numeric($spec['setExpires'])) {
                $spec['setExpires'] = self::gmtDate($spec['setExpires']);
            }
            $this->_headers = array(
                'Cache-Control' => $scope
                ,'Expires' => $spec['setExpires']
            );
            $this->cacheIsValid = false;
            return;
        }
        if (isset($spec['lastModifiedTime'])) {
            // base both headers on time
            $this->_setLastModified($spec['lastModifiedTime']);
            $this->_setEtag($spec['lastModifiedTime'], $scope);

Reported by vadibart on 2008-07-16 23:21:55

mrclay commented 9 years ago
Are you using the 'setExpires' option in serve()?

When using 'setExpires', Last-Modified and the check for If-Modified-Since are not

needed. After the Expires header is sent, under normal browsing the browser will not

re-request the file at all until the expiration time.

Only when you explicitly request the JS/CSS URL or refresh the HTML page will the 
browser re-request the file, and when it does it will not send a conditional GET, so

Last-Modified would be a waste.

You can verify this using Fiddler and navigating around on http://
www.miamianimalremoval.com/ (uses 'setExpires'). The /min/ URLs are not re-requested

unless you manually refresh a page, and when you do, IE7 sends Pragma: no-cache, so

it wouldn't send an If-Modified-Since header anyway.

If you want conditional GETs and 304 responses, don't use the 'setExpires' option.

Reported by mrclay.org on 2008-07-17 18:06:47

mrclay commented 9 years ago
After further testing and reading the code I realized this was probably the issue. 
Still, I'm not sure I agree with your statement that on refreshing the page the
browser won't do a conditional get.  In my FF2 and IE7 browsers I can see the
If-Modified-Since headers being sent for image requests that had a Last-Modified
header on a previous request.  It seems even if there is even partial support for
this in browsers having the Last-Modified in there could save some round-trips with
no adverse affects.

In either case, that would be in the realm of a feature request so I assume you can
just close out this erroneous bug report.  Thanks and sorry for the confusion.

Reported by vadibart on 2008-07-17 18:23:15

mrclay commented 9 years ago
So the requested feature would be: When 'setExpires' is used, Minify should still 
support conditional GETs?

My reasoning for not supporting conditional GET in this case is that it speeds up 
Minify by not loading ConditionalGet.php at all (in 2.0.2b).

My intuition is that, when far-off Expires headers are used, conditional GET 
requests are pretty rare, but I guess some testing is in order. If you have pages 
that invite refresh-a-thons (eBay bidding) and conditional GETs are sent, there'd be

a lot of opportunity for bandwidth savings.

Reported by mrclay.org on 2008-07-17 21:00:39

mrclay commented 9 years ago
Exactly.  I'm also wondering about pages with a meta-refresh (not sure what the
behavior is here but I'd bet it acts like a manual refresh) or AJAX-refreshed pages,
which are more and more common.

Reported by vadibart on 2008-07-17 21:23:36

mrclay commented 9 years ago
Created feature request:
http://code.google.com/p/minify/issues/detail?id=37

Invalidating this issue.

Reported by mrclay.org on 2008-07-17 23:09:18

mrclay commented 9 years ago
I don't know why I made a separate issue for this.

Reported by mrclay.org on 2008-11-27 15:45:43