HTTP/ConditionalGet.php : + encoding option to allow ETags to vary with encoding (issue 91)

HTTP/Encoder.php : Vary is always sent (issue 101)
Minify.php : Allow varying ETags based on encoding (issue 91)
lots of unit test updates

min/lib/HTTP/ConditionalGet.php

@@ -82,8 +82,11 @@ class HTTP_ConditionalGet {
      * 'lastModifiedTime': (int) if given, both ETag AND Last-Modified headers
      * will be sent with content. This is recommended.
      *
-     * 'eTag': (string) if given, this will be used as the ETag header rather
+     * 'encoding': (string) if set, the header "Vary: Accept-Encoding" will
-     * than values based on lastModifiedTime or contentHash.
+     * always be sent and a truncated version of the encoding will be appended
+     * to the ETag. E.g. "pub123456;gz". This will also trigger a more lenient 
+     * checking of the client's If-None-Match header, as the encoding portion of
+     * the ETag will be stripped before comparison.
      * 
      * 'contentHash': (string) if given, only the ETag header can be sent with
      * content (only HTTP1.1 clients can conditionally GET). The given string 
@@ -91,6 +94,10 @@ class HTTP_ConditionalGet {
      * resource changes (recommend md5()). This is not needed/used if 
      * lastModifiedTime is given.
      * 
+     * 'eTag': (string) if given, this will be used as the ETag header rather
+     * than values based on lastModifiedTime or contentHash. Also the encoding
+     * string will not be appended to the given value as described above.
+     * 
      * 'invalidate': (bool) if true, the client cache will be considered invalid
      * without testing. Effectively this disables conditional GET. 
      * (default false)
@@ -120,17 +127,28 @@ class HTTP_ConditionalGet {
                 $_SERVER['REQUEST_TIME'] + $spec['maxAge'] 
             );
         }
+        $etagAppend = '';
+        if (isset($spec['encoding'])) {
+            $this->_stripEtag = true;
+            $this->_headers['Vary'] = 'Accept-Encoding';
+            if ('' !== $spec['encoding']) {
+                if (0 === strpos($spec['encoding'], 'x-')) {
+                    $spec['encoding'] = substr($spec['encoding'], 2);
+                }
+                $etagAppend = ';' . substr($spec['encoding'], 0, 2);
+            }
+        }
         if (isset($spec['lastModifiedTime'])) {
             $this->_setLastModified($spec['lastModifiedTime']);
             if (isset($spec['eTag'])) { // Use it
                 $this->_setEtag($spec['eTag'], $scope);
             } else { // base both headers on time
-                $this->_setEtag($spec['lastModifiedTime'], $scope);
+                $this->_setEtag($spec['lastModifiedTime'] . $etagAppend, $scope);
             }
         } elseif (isset($spec['eTag'])) { // Use it
             $this->_setEtag($spec['eTag'], $scope);
         } elseif (isset($spec['contentHash'])) { // Use the hash as the ETag
-            $this->_setEtag($spec['contentHash'], $scope);
+            $this->_setEtag($spec['contentHash'] . $etagAppend, $scope);
         }
         $this->_headers['Cache-Control'] = "max-age={$maxAge}, {$scope}, must-revalidate";
         // invalidate cache if disabled, otherwise check
@@ -248,12 +266,11 @@ class HTTP_ConditionalGet {
     protected $_headers = array();
     protected $_lmTime = null;
     protected $_etag = null;
+    protected $_stripEtag = false;
     
     protected function _setEtag($hash, $scope)
     {
-        $this->_etag = '"' . $hash
+        $this->_etag = '"' . substr($scope, 0, 3) . $hash . '"';
-            . substr($scope, 0, 3)
-            . '"';
         $this->_headers['ETag'] = $this->_etag;
     }
 
@@ -286,18 +303,30 @@ class HTTP_ConditionalGet {
         if (!isset($_SERVER['HTTP_IF_NONE_MATCH'])) {
             return false;
         }
-        $cachedEtagList = get_magic_quotes_gpc()
+        $clientEtagList = get_magic_quotes_gpc()
             ? stripslashes($_SERVER['HTTP_IF_NONE_MATCH'])
             : $_SERVER['HTTP_IF_NONE_MATCH'];
-        $cachedEtags = split(',', $cachedEtagList);
+        $clientEtags = split(',', $clientEtagList);
-        foreach ($cachedEtags as $cachedEtag) {
+        
-            if (trim($cachedEtag) == $this->_etag) {
+        $compareTo = $this->normalizeEtag($this->_etag);
+        foreach ($clientEtags as $clientEtag) {
+            if ($this->normalizeEtag($clientEtag) === $compareTo) {
+                // respond with the client's matched ETag, even if it's not what
+                // we would've sent by default
+                $this->_headers['ETag'] = trim($clientEtag);
                 return true;
             }
         }
         return false;
     }
     
+    protected function normalizeEtag($etag) {
+        $etag = trim($etag);
+        return $this->_stripEtag
+            ? preg_replace('/;\\w\\w"$/', '"', $etag)
+            : $etag;
+    }
+
     protected function resourceNotModified()
     {
         if (!isset($_SERVER['HTTP_IF_MODIFIED_SINCE'])) {
@@ -308,6 +337,12 @@ class HTTP_ConditionalGet {
             // IE has tacked on extra data to this header, strip it
             $ifModifiedSince = substr($ifModifiedSince, 0, $semicolon);
         }
-        return ($ifModifiedSince == self::gmtDate($this->_lmTime));
+        if ($ifModifiedSince == self::gmtDate($this->_lmTime)) {
+            // Apache 2.2's behavior. If there was no ETag match, send the 
+            // non-encoded version of the ETag value.
+            $this->_headers['ETag'] = $this->normalizeEtag($this->_etag);
+            return true;
+        }
+        return false;
     }
 }

min/lib/HTTP/Encoder.php

@@ -8,6 +8,9 @@
 /**
  * Encode and send gzipped/deflated content
  *
+ * The "Vary: Accept-Encoding" header is sent. If the client allows encoding, 
+ * Content-Encoding and Content-Length are added.
+ *
  * <code>
  * // Send a CSS file, compressed if possible
  * $he = new HTTP_Encoder(array(
@@ -176,7 +179,7 @@ class HTTP_Encoder {
      * 
      * A syntax-aware scan is done of the Accept-Encoding, so the method must
      * be non 0. The methods are favored in order of deflate, gzip, then 
-     * compress. Yes, deflate is always smaller and faster!
+     * compress. deflate is always smallest and generally faster!
      * 
      * @param bool $allowCompress allow the older compress encoding
      * 
@@ -229,8 +232,8 @@ class HTTP_Encoder {
      * Then the appropriate gz_* function is called to compress the content. If
      * this fails, false is returned.
      * 
-     * If successful, the Content-Length header is updated, and Content-Encoding
+     * The header "Vary: Accept-Encoding" is added. If encoding is successful, 
-     * and Vary headers are added.
+     * the Content-Length header is updated, and Content-Encoding is also added.
      * 
      * @param int $compressionLevel given to zlib functions. If not given, the
      * class default will be used.
@@ -239,6 +242,7 @@ class HTTP_Encoder {
      */
     public function encode($compressionLevel = null)
     {
+        $this->_headers['Vary'] = 'Accept-Encoding';
         if (null === $compressionLevel) {
             $compressionLevel = self::$compressionLevel;
         }
@@ -260,7 +264,6 @@ class HTTP_Encoder {
         }
         $this->_headers['Content-Length'] = strlen($encoded);
         $this->_headers['Content-Encoding'] = $this->_encodeMethod[1];
-        $this->_headers['Vary'] = 'Accept-Encoding';
         $this->_content = $encoded;
         return true;
     }

min/lib/Minify.php

@@ -94,7 +94,8 @@ class Minify {
      * 'quiet' : set to true to have serve() return an array rather than sending
      * any headers/output (default false)
      * 
-     * 'encodeOutput' : to disable content encoding, set this to false (default true)
+     * 'encodeOutput' : set to false to disable content encoding, and not send
+     * the Vary header (default true)
      * 
      * 'encodeMethod' : generally you should let this be determined by 
      * HTTP_Encoder (leave null), but you can force a particular encoding
@@ -197,11 +198,29 @@ class Minify {
             self::$_options['maxAge'] = 0;
         }
         
+        // determine encoding
+        if (self::$_options['encodeOutput']) {
+            if (self::$_options['encodeMethod'] !== null) {
+                // controller specifically requested this
+                $contentEncoding = self::$_options['encodeMethod'];
+            } else {
+                // sniff request header
+                require_once 'HTTP/Encoder.php';
+                // depending on what the client accepts, $contentEncoding may be 
+                // 'x-gzip' while our internal encodeMethod is 'gzip'. Calling
+                // getAcceptedEncoding() with false leaves out compress as an option.
+                list(self::$_options['encodeMethod'], $contentEncoding) = HTTP_Encoder::getAcceptedEncoding(false);
+            }
+        } else {
+            self::$_options['encodeMethod'] = ''; // identity (no encoding)
+        }
+        
         // check client cache
         require_once 'HTTP/ConditionalGet.php';
         $cgOptions = array(
             'lastModifiedTime' => self::$_options['lastModifiedTime']
             ,'isPublic' => self::$_options['isPublic']
+            ,'encoding' => self::$_options['encodeMethod']
         );
         if (self::$_options['maxAge'] > 0) {
             $cgOptions['maxAge'] = self::$_options['maxAge'];
@@ -226,23 +245,6 @@ class Minify {
             unset($cg);
         }
         
-        // determine encoding
-        if (self::$_options['encodeOutput']) {
-            if (self::$_options['encodeMethod'] !== null) {
-                // controller specifically requested this
-                $contentEncoding = self::$_options['encodeMethod'];
-            } else {
-                // sniff request header
-                require_once 'HTTP/Encoder.php';
-                // depending on what the client accepts, $contentEncoding may be 
-                // 'x-gzip' while our internal encodeMethod is 'gzip'. Calling
-                // getAcceptedEncoding() with false leaves out compress as an option.
-                list(self::$_options['encodeMethod'], $contentEncoding) = HTTP_Encoder::getAcceptedEncoding(false);
-            }
-        } else {
-            self::$_options['encodeMethod'] = ''; // identity (no encoding)
-        }
-        
         if (self::$_options['contentType'] === self::TYPE_CSS
             && self::$_options['rewriteCssUris']) {
             reset($controller->sources);
@@ -303,6 +305,8 @@ class Minify {
             : self::$_options['contentType'];
         if (self::$_options['encodeMethod'] !== '') {
             $headers['Content-Encoding'] = $contentEncoding;
+        }
+        if (self::$_options['encodeOutput']) {
             $headers['Vary'] = 'Accept-Encoding';
         }
 

min_unit_tests/HTTP_ConditionalGet/2.php

@@ -1,6 +1,6 @@
 <?php
 
-require '../../config.php';
+set_include_path(get_include_path() . PATH_SEPARATOR . realpath(dirname(__FILE__) . '/../../min/lib'));
 require 'HTTP/ConditionalGet.php';
 
 // emulate regularly updating document

min_unit_tests/HTTP_ConditionalGet/3.php

@@ -1,6 +1,6 @@
 <?php
 
-require '../../config.php';
+set_include_path(get_include_path() . PATH_SEPARATOR . realpath(dirname(__FILE__) . '/../../min/lib'));
 require 'HTTP/ConditionalGet.php';
 
 // generate content first (not ideal)

min_unit_tests/HTTP_ConditionalGet/4.php

@@ -1,14 +1,18 @@
 <?php
 
-require '../../config.php';
+set_include_path(get_include_path() . PATH_SEPARATOR . realpath(dirname(__FILE__) . '/../../min/lib'));
 require 'HTTP/ConditionalGet.php';
 
 // emulate regularly updating document
 $every = 20;
 $lastModified = round(time()/$every)*$every - $every;
 
+require 'HTTP/Encoder.php';
+list($enc,) = HTTP_Encoder::getAcceptedEncoding();
+
 $cg = new HTTP_ConditionalGet(array(
     'lastModifiedTime' => $lastModified
+    ,'encoding' => $enc
 ));
 $cg->sendHeaders();
 if ($cg->cacheIsValid) {
@@ -31,7 +35,6 @@ $content = get_content(array(
     ,'explain' => $explain
 ));
 
-require 'HTTP/Encoder.php';
 $he = new HTTP_Encoder(array(
     'content' => get_content(array(
         'title' => $title

min_unit_tests/HTTP_ConditionalGet/5.php

@@ -1,6 +1,6 @@
 <?php
 
-require '../../config.php';
+set_include_path(get_include_path() . PATH_SEPARATOR . realpath(dirname(__FILE__) . '/../../min/lib'));
 require 'HTTP/ConditionalGet.php';
 
 // far expires

min_unit_tests/HTTP_ConditionalGet/index.php

@@ -1,6 +1,6 @@
 <?php
 
-require '../../config.php';
+set_include_path(get_include_path() . PATH_SEPARATOR . realpath(dirname(__FILE__) . '/../../min/lib'));
 require 'HTTP/ConditionalGet.php';
 
 // emulate regularly updating document

min_unit_tests/HTTP_Encoder/index.php

@@ -1,7 +1,7 @@
 <?php
 ini_set('display_errors', 'on');
 
-require '../../config.php';
+set_include_path(get_include_path() . PATH_SEPARATOR . realpath(dirname(__FILE__) . '/../../min/lib'));
 require 'HTTP/Encoder.php';
 
 if (!isset($_GET['test'])) {

min_unit_tests/test_HTTP_ConditionalGet.php

@@ -16,8 +16,9 @@ function test_HTTP_ConditionalGet()
             ,'inm' => null
             ,'ims' => $gmtTime
             ,'exp' => array(
-            	'Last-Modified' => $gmtTime
+                'Vary' => 'Accept-Encoding'
-                ,'ETag' => "\"{$lmTime}pri\""
+            	,'Last-Modified' => $gmtTime
+                ,'ETag' => "\"pri{$lmTime}\""
                 ,'Cache-Control' => 'max-age=0, private, must-revalidate'
                 ,'_responseCode' => 'HTTP/1.0 304 Not Modified'
                 ,'isValid' => true
@@ -28,20 +29,35 @@ function test_HTTP_ConditionalGet()
             ,'inm' => null
             ,'ims' => $gmtTime . ';'
             ,'exp' => array(
-            	'Last-Modified' => $gmtTime
+            	'Vary' => 'Accept-Encoding'
-                ,'ETag' => "\"{$lmTime}pri\""
+            	,'Last-Modified' => $gmtTime
+                ,'ETag' => "\"pri{$lmTime}\""
                 ,'Cache-Control' => 'max-age=0, private, must-revalidate'
                 ,'_responseCode' => 'HTTP/1.0 304 Not Modified'
                 ,'isValid' => true
             )
         )
         ,array(
-            'desc' => 'client has valid ETag'
+            'desc' => 'client has valid ETag (non-encoded version)'
-            ,'inm' => "\"badEtagFoo\", \"{$lmTime}pri\""
+            ,'inm' => "\"badEtagFoo\", \"pri{$lmTime}\""
             ,'ims' => null
             ,'exp' => array(
-                'Last-Modified' => $gmtTime
+                'Vary' => 'Accept-Encoding'
-                ,'ETag' => "\"{$lmTime}pri\""
+            	,'Last-Modified' => $gmtTime
+                ,'ETag' => "\"pri{$lmTime}\""
+                ,'Cache-Control' => 'max-age=0, private, must-revalidate'
+                ,'_responseCode' => 'HTTP/1.0 304 Not Modified'
+                ,'isValid' => true
+            )
+        )
+        ,array(
+            'desc' => 'client has valid ETag (gzip version)'
+            ,'inm' => "\"badEtagFoo\", \"pri{$lmTime};gz\""
+            ,'ims' => null
+            ,'exp' => array(
+                'Vary' => 'Accept-Encoding'
+            	,'Last-Modified' => $gmtTime
+                ,'ETag' => "\"pri{$lmTime};gz\""
                 ,'Cache-Control' => 'max-age=0, private, must-revalidate'
                 ,'_responseCode' => 'HTTP/1.0 304 Not Modified'
                 ,'isValid' => true
@@ -52,19 +68,21 @@ function test_HTTP_ConditionalGet()
             ,'inm' => null
             ,'ims' => null
             ,'exp' => array(
-                'Last-Modified' => $gmtTime
+                'Vary' => 'Accept-Encoding'
-                ,'ETag' => "\"{$lmTime}pri\""
+            	,'Last-Modified' => $gmtTime
+                ,'ETag' => "\"pri{$lmTime};gz\""
                 ,'Cache-Control' => 'max-age=0, private, must-revalidate'
                 ,'isValid' => false
             )
         )
         ,array(
             'desc' => 'client has invalid ETag'
-            ,'inm' => '"' . ($lmTime - 300) . 'pri"'
+            ,'inm' => '"pri' . ($lmTime - 300) . '"'
             ,'ims' => null
             ,'exp' => array(
-                'Last-Modified' => $gmtTime
+                'Vary' => 'Accept-Encoding'
-                ,'ETag' => "\"{$lmTime}pri\""
+            	,'Last-Modified' => $gmtTime
+                ,'ETag' => "\"pri{$lmTime};gz\""
                 ,'Cache-Control' => 'max-age=0, private, must-revalidate'
                 ,'isValid' => false
             )
@@ -74,8 +92,9 @@ function test_HTTP_ConditionalGet()
             ,'inm' => null
             ,'ims' => gmdate('D, d M Y H:i:s \G\M\T', $lmTime - 300)
             ,'exp' => array(
-                'Last-Modified' => $gmtTime
+                'Vary' => 'Accept-Encoding'
-                ,'ETag' => "\"{$lmTime}pri\""
+            	,'Last-Modified' => $gmtTime
+                ,'ETag' => "\"pri{$lmTime};gz\""
                 ,'Cache-Control' => 'max-age=0, private, must-revalidate'
                 ,'isValid' => false
             )
@@ -89,7 +108,7 @@ function test_HTTP_ConditionalGet()
         } else {
             $_SERVER['HTTP_IF_NONE_MATCH'] = get_magic_quotes_gpc()
                 ? addslashes($test['inm'])
-                : $test['inm'];;
+                : $test['inm'];
         }
         if (null === $test['ims']) {
             unset($_SERVER['HTTP_IF_MODIFIED_SINCE']);
@@ -100,6 +119,7 @@ function test_HTTP_ConditionalGet()
         
         $cg = new HTTP_ConditionalGet(array(
             'lastModifiedTime' => $lmTime
+            ,'encoding' => 'x-gzip'
         ));
         $ret = $cg->getHeaders();
         $ret['isValid'] = $cg->cacheIsValid;

min_unit_tests/test_HTTP_Encoder.php

@@ -128,6 +128,14 @@ function test_HTTP_Encoder()
                 , "(off by ". abs($ret - $test['exp']) . " bytes)\n\n";
         }
     }
+    
+    $_SERVER['HTTP_ACCEPT_ENCODING'] = 'identity';
+    $he = new HTTP_Encoder(array(
+        'content' => 'Hello'
+    ));
+    $he->encode();
+    $headers = $he->getHeaders();
+    assertTrue(isset($headers['Vary']), 'HTTP_Encoder : Vary always sent');    
 }
 
 test_HTTP_Encoder();

min_unit_tests/test_Minify.php

@@ -26,8 +26,9 @@ function test_Minify()
         ,'content' => '',
         'headers' => array(
             'Expires' => gmdate('D, d M Y H:i:s \G\M\T', $_SERVER['REQUEST_TIME'] + 1800),
+            'Vary' => 'Accept-Encoding',
             'Last-Modified' => gmdate('D, d M Y H:i:s \G\M\T', $lastModified),
-            'ETag' => "\"{$lastModified}pub\"",
+            'ETag' => "\"pub{$lastModified}\"",
             'Cache-Control' => 'max-age=1800, public, must-revalidate',
             '_responseCode' => 'HTTP/1.0 304 Not Modified',
         )
@@ -47,10 +48,9 @@ function test_Minify()
     }
 
     assertTrue(
-        ! class_exists('HTTP_Encoder', false)
+        ! class_exists('Minify_CSS', false)
-        && ! class_exists('Minify_CSS', false)
         && ! class_exists('Minify_Cache', false)
-        ,'Minify : encoding, cache, and minifier classes aren\'t loaded for 304s'
+        ,'Minify : cache, and minifier classes aren\'t loaded for 304s'
     );
 
     // Test minifying JS and serving with Expires header
@@ -67,8 +67,9 @@ function test_Minify()
         ,'content' => $content
         ,'headers' => array (
             'Expires' => gmdate('D, d M Y H:i:s \G\M\T', $tomorrow),
+            'Vary' => 'Accept-Encoding',
             'Last-Modified' => gmdate('D, d M Y H:i:s \G\M\T', $lastModified),
-            'ETag' => "\"{$lastModified}pub\"",
+            'ETag' => "\"pub{$lastModified}\"",
             'Cache-Control' => 'max-age=86400, public, must-revalidate',
             'Content-Length' => strlen($content),
             'Content-Type' => 'application/x-javascript; charset=UTF-8',
@@ -181,8 +182,9 @@ function test_Minify()
         ,'statusCode' => 200
         ,'content' => $expectedContent
         ,'headers' => array (
+            'Vary' => 'Accept-Encoding',
             'Last-Modified' => gmdate('D, d M Y H:i:s \G\M\T', $lastModified),
-            'ETag' => "\"{$lastModified}pub\"",
+            'ETag' => "\"pub{$lastModified}\"",
             'Cache-Control' => 'max-age=0, public, must-revalidate',
             'Content-Length' => strlen($expectedContent),
             'Content-Type' => 'text/css; charset=UTF-8',