Merge pull request #1 from nexcess/devel

update Devel
nexcess · Sep 3, 2015 · 9610093 · 9610093
2 parents ab2657c + cdbf11c
commit 9610093
Show file tree

Hide file tree

Showing 19 changed files with 899 additions and 99 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -408,3 +408,8 @@ Magento CE 1.8+ or EE 1.13+, see [these instructions](https://github.com/nexcess
   * [#860] Removing ref to nonexistent file from modman (@cgrice  )
   * [#596] update docs to make an important notice on Crawler IP Addresses setting under Caching Options (@arosenhagen)
   * [#878] Explicit cache bypass for progress sections (@astorm)
+  * [#844] Fix for HTTPS ESI URLs (@jeroenvermeulen)
+  * [#865] Implemented load balancing support (@jeroenvermeulen)
+
+### RELEASE-0.6.5
+  * Varnish 4 (@aheadley @aricwatson)
diff --git a/TECHNICAL_NOTES.md b/TECHNICAL_NOTES.md
@@ -39,6 +39,10 @@ Even with the addition of ESI, a client still needs a way to get their session c
 
 It sounds simple enough, but Varnish doesn't actually have any easy way to generate unique session tokens. At first, this seemed like a wash but then I remembered that you can actually write straight C code in the VCL for more advanced functionality, such as generating the session token! So that's what Turpentine does: a small C function is included in the VCL to [generate uuids](https://github.com/nexcess/magento-turpentine/blob/master/app/code/community/Nexcessnet/Turpentine/misc/uuid.c), then that function is used to [make a session cookie](https://github.com/nexcess/magento-turpentine/blob/master/app/code/community/Nexcessnet/Turpentine/misc/version-3.vcl#L58) if no session cookie was sent in the request, and the cookie is [passed back to the client](https://github.com/nexcess/magento-turpentine/blob/master/app/code/community/Nexcessnet/Turpentine/misc/version-3.vcl#L340) in the response so it is sent with future requests.
 
+#### Inline C as of Varnish 4.0
+
+As of Varnish 4.0 the inline C functionality is disabled by default. To enable inline C you need to modify Varnish's startup config and add `-p vcc_allow_inline_c=on` to Varnish's startup command.
+
 ### Serializing Registry Data
 
 To include the registry data required for ESI block rendering in the ESI URL, Turpentine simply takes a list of the needed registry keys (provided in the layout file) for the block and serializes it using PHP's native [`serialize`](http://us3.php.net/serialize) function which turns stores the data in a string which we can then include in the URL as a simple GET parameter. However, a registry key can be associated with any data type, including whole objects. While `serialize` will typically work on objects there are some edge cases (such as XML documents) that cannot be serialized. In order to accommodate these documents, Turpentine considers some objects to be "[complex registry data](https://github.com/nexcess/magento-turpentine/blob/master/app/code/community/Nexcessnet/Turpentine/Model/Observer/Esi.php#L441)" and handles those separately from the standard PHP types like string, int, simple classes, etc. Complex registry data is considered to be Magento "models" that correspond to database records and have `getId` methods (typically things like products, categories, etc). Then Turpentine can simply serialize the model's class and ID, then load the model with the ID instead of serializing the entire object. This neatly sidesteps the whole issue of finding a way to serialize objects that are unserializable.
@@ -58,7 +62,7 @@ This change effectively broke Turpentine for those Magento versions, and finding
   1. Because the form key is located inside HTML attributes (`href="http://example.com/example/action/form_key/<esi:include src="http://example.com/getFormKey/"/>/"` for example), by default Varnish doesn't see the ESI include tag which completely breaks the buttons and forms that use the form key.
   2. How does Magento know when to generate the actual form key, and not the ESI tag that is replaced by the form key. For example, the form key ESI is needed when generating the "Add to Cart" button, but the actual form key is needed both for both the ESI request to pull in the form key, and when that button is clicked so that it can be compared to the form key that was submitted.
 
-The first is solved by a Varnish config change. Adding `-p esi_syntax=0x2` to Varnish's startup command tells it to look for any ESI tags, even if they're not in properly structured HTML syntax (like the example). The second was more tricky, at first it seemed a static list of actions that use the form key would be needed (like the original fix) that could be used to tell the mage/core session class when to generate the form key, and when to generate the form key ESI tag. However, the solution was staring me in the face: the request itself tells you when you need the actual form key, as it's only needed when the request includes the form key in the GET params or POST data to do the comparison between the actual form key, and the form key sent with the request. Thus we can just [check](https://github.com/nexcess/magento-turpentine/blob/master/app/code/local/Mage/Core/Model/Session.php#L53) if the form key was sent in the request and either generate the real form key, or the ESI tag.
+The first is solved by a Varnish config change. Adding `-p esi_syntax=0x2` (`-p feature=+esi_ignore_other_elements` as of Varnish 4.0) to Varnish's startup command tells it to look for any ESI tags, even if they're not in properly structured HTML syntax (like the example). The second was more tricky, at first it seemed a static list of actions that use the form key would be needed (like the original fix) that could be used to tell the mage/core session class when to generate the form key, and when to generate the form key ESI tag. However, the solution was staring me in the face: the request itself tells you when you need the actual form key, as it's only needed when the request includes the form key in the GET params or POST data to do the comparison between the actual form key, and the form key sent with the request. Thus we can just [check](https://github.com/nexcess/magento-turpentine/blob/master/app/code/local/Mage/Core/Model/Session.php#L53) if the form key was sent in the request and either generate the real form key, or the ESI tag.
 
 ## Closing Time
 

diff --git a/app/code/community/Nexcessnet/Turpentine/Block/Core/Messages.php b/app/code/community/Nexcessnet/Turpentine/Block/Core/Messages.php
@@ -198,16 +198,17 @@ protected function _toHtml() {
             } else {
                 $this->_loadMessages();
                 $this->_loadSavedMessages();
-                $html = $this->_real_toHtml();
+                if ( count($this->getMessages()) ) {
+                    $html = $this->_real_toHtml();
+                } else {
+                    // Prevent returning an empty <ul></ul>
+                    $html = '';
+                }
             }
         } else {
             $html = $this->_real_toHtml();
         }
         $this->_directCall = false;
-
-        if (count($this->getMessages()) == 0) {
-            return '';
-        }
         return $html;
     }
 

diff --git a/app/code/community/Nexcessnet/Turpentine/Helper/Esi.php b/app/code/community/Nexcessnet/Turpentine/Helper/Esi.php
@@ -357,7 +357,10 @@ public function getFormKeyEsiUrl() {
             $this->getEsiScopeParam()       => 'global',
             $this->getEsiCacheTypeParam()   => 'private',
         );
-        return Mage::getUrl( 'turpentine/esi/getFormKey', $urlOptions );
+        $esiUrl = Mage::getUrl( 'turpentine/esi/getFormKey', $urlOptions );
+        // setting [web/unsecure/base_url] can be https://... but ESI can never be HTTPS
+        $esiUrl = preg_replace( '|^https://|i', 'http://', $esiUrl );
+        return $esiUrl;
     }
 
     /**

diff --git a/app/code/community/Nexcessnet/Turpentine/Model/Config/Select/LoadBalancing.php b/app/code/community/Nexcessnet/Turpentine/Model/Config/Select/LoadBalancing.php
@@ -0,0 +1,34 @@
+<?php
+
+/** 
+ * Nexcess.net Turpentine Extension for Magento
+ * Copyright (C) 2012  Nexcess.net L.L.C.
+ * 
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ * 
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ * 
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */ 
+
+class Nexcessnet_Turpentine_Model_Config_Select_LoadBalancing {
+    /**
+     * @return array
+     */
+    public function toOptionArray() {
+        $helper = Mage::helper('turpentine');
+        return array(
+            array('value'=>'no', 'label'=>$helper->__('No, use only one backend server')),
+            array('value'=>'yes', 'label'=>$helper->__('Yes, use load balancing')),
+            array('value'=>'yes_admin', 'label'=>$helper->__('Yes, with separate settings for Admin')),
+        );
+    }
+}
diff --git a/app/code/community/Nexcessnet/Turpentine/Model/Config/Select/Version.php b/app/code/community/Nexcessnet/Turpentine/Model/Config/Select/Version.php
@@ -1,30 +1,31 @@
 <?php
 
-/** 
+/**
  * Nexcess.net Turpentine Extension for Magento
  * Copyright (C) 2012  Nexcess.net L.L.C.
- * 
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
  * (at your option) any later version.
- * 
+ *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- * 
+ *
  * You should have received a copy of the GNU General Public License along
  * with this program; if not, write to the Free Software Foundation, Inc.,
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- */ 
+ */
 
 class Nexcessnet_Turpentine_Model_Config_Select_Version {
     public function toOptionArray() {
         $helper = Mage::helper('turpentine');
         return array(
             array( 'value' => '2.1', 'label' => $helper->__( '2.1.x' ) ),
             array( 'value' => '3.0', 'label' => $helper->__( '3.0.x' ) ),
+	        array( 'value' => '4.0', 'label' => $helper->__( '4.0.x' ) ),
             array( 'value' => 'auto', 'label' => $helper->__( 'Auto' ) ),
         );
     }

diff --git a/app/code/community/Nexcessnet/Turpentine/Model/Observer/Ban.php b/app/code/community/Nexcessnet/Turpentine/Model/Observer/Ban.php
@@ -299,8 +299,14 @@ public function banCacheType( $eventObject ) {
      */
     public function banProductReview( $eventObject ) {
         $patterns = array();
+        /* @var $review \Mage_Review_Model_Review*/
         $review = $eventObject->getObject();
-        $products = $review->getProductCollection()->getItems();
+
+        /* @var $productCollection \Mage_Review_Model_Resource_Review_Product_Collection*/
+        $productCollection = $review->getProductCollection();
+
+        $products = $productCollection->addEntityFilter((int)$review->getEntityPkValue())->getItems();
+
         $productIds = array_unique( array_map(
             create_function( '$p', 'return $p->getEntityId();' ),
             $products ) );

diff --git a/app/code/community/Nexcessnet/Turpentine/Model/Observer/Esi.php b/app/code/community/Nexcessnet/Turpentine/Model/Observer/Esi.php
@@ -286,6 +286,10 @@ public function injectEsi( $eventObject ) {
             }
 
             $esiUrl = Mage::getUrl( 'turpentine/esi/getBlock', $urlOptions );
+            if( $esiOptions[$methodParam] == 'esi' ) {
+                // setting [web/unsecure/base_url] can be https://... but ESI can never be HTTPS
+                $esiUrl = preg_replace( '|^https://|i', 'http://', $esiUrl );
+            }
             $blockObject->setEsiUrl( $esiUrl );
             // avoid caching the ESI template output to prevent the double-esi-
             // include/"ESI processing not enabled" bug

diff --git a/app/code/community/Nexcessnet/Turpentine/Model/Varnish/Admin.php b/app/code/community/Nexcessnet/Turpentine/Model/Varnish/Admin.php
@@ -138,22 +138,39 @@ protected function _testEsiSyntaxParam( $socket ) {
         $result = false;
 
         if( $helper->csrfFixupNeeded() ) {
-            $value = $socket->param_show( 'esi_syntax' );
-            if( preg_match( '~(\d)\s+\[bitmap\]~', $value['text'], $match ) ) {
-                $value = hexdec( $match[1] );
-                if( $value & self::MASK_ESI_SYNTAX ) { //bitwise intentional
-                    // setting is correct, all is fine
+            if ( $socket->getVersion()==='4.0' ) {
+                $paramName = 'feature';
+                $value = $socket->param_show( $paramName );
+                $value = explode("\n", $value['text']);
+                if ( isset($value[1]) && strpos($value[1], '+esi_ignore_other_elements')!==false ) {
                     $result = true;
                 } else {
-                    $session->addWarning( 'Varnish <em>esi_syntax</em> param is ' .
+                    $session->addWarning( 'Varnish <em>feature</em> param is ' .
                         'not set correctly, please see <a target="_blank" href="' .
                         self::URL_ESI_SYNTAX_FIX . '">these instructions</a> ' .
                         'to fix this warning.' );
                 }
             } else {
+                $paramName = 'esi_syntax';
+                $value = $socket->param_show( $paramName );
+                if( preg_match( '~(\d)\s+\[bitmap\]~', $value['text'], $match ) ) {
+                    $value = hexdec( $match[1] );
+                    if( $value & self::MASK_ESI_SYNTAX ) { //bitwise intentional
+                        // setting is correct, all is fine
+                        $result = true;
+                    } else {
+                        $session->addWarning( 'Varnish <em>esi_syntax</em> param is ' .
+                            'not set correctly, please see <a target="_blank" href="' .
+                            self::URL_ESI_SYNTAX_FIX . '">these instructions</a> ' .
+                            'to fix this warning.' );
+                    }
+                }
+            }
+
+            if ( $result===false ) {
                 // error
                 Mage::helper( 'turpentine/debug' )->logWarn(
-                    'Failed to parse param.show output to check esi_syntax value' );
+                    sprintf('Failed to parse param.show output to check %s value', $paramName ) );
                 $result = true;
             }
         } else {

diff --git a/app/code/community/Nexcessnet/Turpentine/Model/Varnish/Admin/Socket.php b/app/code/community/Nexcessnet/Turpentine/Model/Varnish/Admin/Socket.php
@@ -84,10 +84,16 @@ class Nexcessnet_Turpentine_Model_Varnish_Admin_Socket {
     // and used
     const CLI_CMD_LENGTH_LIMIT  = 8192;
 
+    /**
+     * Regexp to detect the varnish version number
+     * @var string
+     */
+    const REGEXP_VARNISH_VERSION = '/^varnish\-(?P<vmajor>\d)\.(?P<vminor>\d)\.(?P<vsub>\d) revision (?P<vhash>[0-9a-f]+)$/';
+
     /**
      * VCL config versions, should match config select values
      */
-    static protected $_VERSIONS = array( '2.1', '3.0' );
+    static protected $_VERSIONS = array( '2.1', '3.0', '4.0' );
 
     /**
      * Varnish socket connection
@@ -257,8 +263,8 @@ public function isConnected() {
      * @return string
      */
     public function getVersion() {
-        if( is_null( $this->_version ) ) {
-            $this->_version = $this->_determineVersion();
+        if ( !$this->isConnected() ) {
+            $this->_connect();
         }
         return $this->_version;
     }
@@ -331,15 +337,37 @@ protected function _connect() {
             $challenge = substr( $banner['text'], 0, 32 );
             $response = hash( 'sha256', sprintf( "%s\n%s%s\n", $challenge,
                 $this->_authSecret, $challenge ) );
-            $this->_command( 'auth', self::CODE_OK, $response );
-        } else if( $banner['code'] !== self::CODE_OK ) {
+            $banner = $this->_command( 'auth', self::CODE_OK, $response );
+        }
+
+        if( $banner['code'] !== self::CODE_OK ) {
             Mage::throwException( 'Varnish admin authentication failed: ' .
                 $banner['text'] );
         }
 
+        if ($this->_version == null) { // If autodetecting
+            $this->_version = $this->_determineVersion($banner['text']);
+        }
+
         return $this->isConnected();
     }
 
+    protected function _determineVersion($bannerText) {
+        $bannerText = array_filter(explode("\n", $bannerText));
+        if ( count($bannerText)<6 ) {
+            // Varnish 2.0 does not spit out a banner on connect
+            Mage::throwException('Varnish versions before 2.1 are not supported');
+        }
+        if ( count($bannerText)<7 ) {
+            // Varnish before 3.0 does not spit out a version number
+            return '2.1';
+        } elseif ( preg_match(self::REGEXP_VARNISH_VERSION, $bannerText[4], $matches)===1 ) {
+            return $matches['vmajor'] . '.' . $matches['vminor'];
+        } else {
+            Mage::throwException('Unable to detect varnish version');
+        }
+    }
+
     /**
      * Close the connection (if we're connected)
      *
@@ -367,9 +395,17 @@ protected function _write( $data ) {
         $dataLength = strlen( $data );
         if( $dataLength >= self::CLI_CMD_LENGTH_LIMIT ) {
             $cliBufferResponse = $this->param_show( 'cli_buffer' );
-            if( preg_match( '~^cli_buffer\s+(\d+)\s+\[bytes\]~',
-                    $cliBufferResponse['text'], $match ) ) {
+            $regexp = '~^cli_buffer\s+(\d+)\s+\[bytes\]~';
+            if ( $this->getVersion()==='4.0' ) {
+                // Varnish4 supports "16k" style notation
+                $regexp = '~^cli_buffer\s+Value is:\s+(\d+)([k|m|g]{1})?\s+\[bytes\]~';
+            }
+            if( preg_match( $regexp, $cliBufferResponse['text'], $match ) ) {
                 $realLimit = (int)$match[1];
+                if ( isset($match[2]) ) {
+                    $factors = array('k'=>1,'m'=>2,'g'=>3);
+                    $realLimit *= pow(1024, $factors[$match[2]]);
+                }
             } else {
                 Mage::helper( 'turpentine/debug' )->logWarn(
                     'Failed to determine Varnish cli_buffer limit, using default' );
@@ -470,6 +506,7 @@ protected function _translateCommandMethod( $verb ) {
             case '2.1':
                 $command = str_replace( 'ban', 'purge', $command );
                 break;
+	        case '4.0':
             case '3.0':
                 $command = str_replace( 'purge', 'ban', $command );
                 break;
@@ -479,21 +516,4 @@ protected function _translateCommandMethod( $verb ) {
         }
         return $command;
     }
-
-    /**
-     * Guess the Varnish version based on the availability of the 'banner' command
-     *
-     * @return string
-     */
-    protected function _determineVersion() {
-        $resp = $this->_write( 'help' )->_read();
-        if( strpos( $resp['text'], 'ban.url' ) !== false ) {
-            return '3.0';
-        } elseif( strpos( $resp['text'], 'purge.url' ) !== false &&
-                strpos( $resp['text'], 'banner' ) ) {
-            return '2.1';
-        } else {
-            Mage::throwException( 'Unable to determine instance version' );
-        }
-    }
 }