We launched new forums in March 2019—join us there. In a hurry for help with your website? Get Help Now!
    • 18373 ☆ A M B ☆
    • 3,141 Posts
    Quote from: gerritvanaaken at Jun 01, 2011, 05:27 AM

    @Mark H: Is this [[++setting_phpthumb_nohotlink_enabled]] ?

    That sounds like the settingn I meant, yeah smiley
      Mark Hamstra • Developer spending his days working on Premium Extras and a MODX Site Dashboard with the ability to remotely upgrade MODX and extras to make the MODX world a little better.

      Tweet me @mark_hamstra, check my infrequent blog at markhamstra.com, my slightly more frequent ramblings at MODX.today or see code at Github.
      • 30107
      • 44 Posts
      I suggest to rethink about line 30:
      if ($dimensions = getimagesize($config['base_path'].$filename, $info))

      ...because it throws errors in MODX error log, if there are external images. For instance:
      [2011-06-03 18:00:54] (ERROR @ /myweb/core/cache/includes/elements/modplugin/2.include.cache.php : 36) PHP warning: getimagesize(/myweb/http://www.adobe.com/images/shared/download_buttons/get_adobe_flash_player.png) [<a href='function.getimagesize'>function.getimagesize</a>]:

      Maybe the regex in line 21 could be enhancend?
      // find all img elements with a src attribute
      preg_match_all('|\<img.*?src=[",\'](.*?)[",\'].*?[^>]+\>|i', $str, $filenames);

      I think it would be better if every src with preceeding http or https is considered external and isn’t put into the array $filenames. I would like to help, but I’m not sure if I’m capable to find the appropriate regex for this...
        • 16337
        • 44 Posts
        Quote from: titanium at Jun 03, 2011, 11:08 AM

        I suggest to rethink about line 30:
        if ($dimensions = getimagesize($config['base_path'].$filename, $info))

        ...because it throws errors in MODX error log, if there are external images. For instance:
        [2011-06-03 18:00:54] (ERROR @ /myweb/core/cache/includes/elements/modplugin/2.include.cache.php : 36) PHP warning: getimagesize(/myweb/http://www.adobe.com/images/shared/download_buttons/get_adobe_flash_player.png) [<a href='function.getimagesize'>function.getimagesize</a>]:

        Maybe the regex in line 21 could be enhancend?
        // find all img elements with a src attribute
        preg_match_all('|\<img.*?src=[",\'](.*?)[",\'].*?[^>]+\>|i', $str, $filenames);

        I think it would be better if every src with preceeding http or https is considered external and isn’t put into the array $filenames. I would like to help, but I’m not sure if I’m capable to find the appropriate regex for this...

        i think you are looking at an old source. update the extra.

        besides just ignoring httpd:// urls as external (like it is in the latest version) won’t work for offloading to subdomains or CDN. a user configurable mask/option would be the best.
          • 10713
          • 41 Posts
          Titanium: You’re right. I need to check whether $filename begins with "http://", before I set the modx basepath in front ;-)

          I’ll think about a user option for handling external files for version 1.1.

          For 1.0 pl I will simply disallow external images.
            • 10713
            • 41 Posts
            I added some barriers for external images to be cached, so could you please test this new version of the plugin? Would like to hear your opinion before I realease this as an update:

            <?php
            /**
             * @name AutoFixImageSize
             * @version 1.0.0 rc2
             * @author Gerrit van Aaken <[email protected]> April–June 2011
             *
             * @license GPLv2
             *
             * Fixes img elements with wrong width/height attributes. 
             * Uses phpThumbOf for generating correctly sized physical image files.
             *
             * Must be executed at "OnWebPagePrerender"
             */
            
            // get parsed document as string
            $str = $modx->resource->_output;
            
            // get configuration from global object
            $config = $modx->getConfig();
            
            // find all img elements with a src attribute
            preg_match_all('|\<img.*?src=[",\'](.*?)[",\'].*?[^>]+\>|i', $str, $filenames);
            
            // loop through all found img elements
            foreach($filenames[1] as $i => $filename) {
              
              $img_old = $filenames[0][$i];
              $allowcaching = false; // pessimistic
              
              // is file already cached?
              if (strpos($filename,"connector.php?") == false) {
            
                // check if external caching is allowed
                if (substr($filename,0,7) == "http://" || substr($filename,0,8) == "https://") {
                  $pre = "";
                  if ($config['setting_phpthumb_nohotlink_enabled']) {
                    foreach (explode(",", $config['setting_phpthumb_nohotlink_valid_domains']) as $alldomain) {
                      if ( strpos(strtolower($filename), strtolower(trim($alldomain))) != false) {
                        $allowcaching = true;
                      }
                    } 
                  } else {
                    $allowcaching = true;
                  }
                } else {
                  $pre = $config['base_path'];
                  $allowcaching = true;
                }
              }
              
              // do we have physical access to the file?
              if ($allowcaching && $dimensions = getimagesize($pre.$filename, $info)) {
            
                // find width and height attribut and save value
                preg_match_all('|width=[",\']([0-9]+?)[",\']|i', $filenames[0][$i], $widths);
                $width = $widths[1][0];
                preg_match_all('|height=[",\']([0-9]+?)[",\']|i', $filenames[0][$i], $heights);
                $height = $heights[1][0];
            
                // if resizing needed...
                if (($width && $width != $dimensions[0]) || ($height && $height != $dimensions[1])) {
            
                  // prepare resizing metadata
                  $filetype = strtolower(substr($filename, strrpos($filename,".")+1));
                  $image = array();
                  $image['input'] = $filename;
                  $image['options'] = "f=".$filetype."&h=".$height."&w=".$width; 
            
                  // perform physical resizing and caching via phpthumbof
                  $cacheurl = $modx->runSnippet('phpthumbof',$image);
            
                  // set freshly cached image file location into old src attribute
                  $img_new = str_replace($filename, $cacheurl, $img_old);  
            
                  // replace old image element with new one on whole page content
                  $str = str_replace($img_old, $img_new, $str);  
                }
              }
            }
            
            // exchange the output string with the replaced one
            $modx->resource->_output = $str;
            
              • 30107
              • 44 Posts
              Quote from: gerritvanaaken at Jun 03, 2011, 12:56 PM

              I added some barriers for external images to be cached, so could you please test this new version of the plugin? Would like to hear your opinion before I realease this as an update:
              Gerrit, you’re the fastest developer on earth smiley
              Hmmm... tried it as fast as I could, too. It throws another error now:
              [2011-06-04 00:44:09] (ERROR @ /myweb/core/cache/includes/elements/modplugin/2.include.cache.php : 57) PHP warning: getimagesize() [<a href='function.getimagesize'>function.getimagesize</a>]: URL file-access is disabled in the server configuration
              [2011-06-04 00:44:09] (ERROR @ /myweb/core/cache/includes/elements/modplugin/2.include.cache.php : 57) PHP warning: getimagesize(http://www.adobe.com/images/shared/download_buttons/get_adobe_flash_player.png) [<a href='function.getimagesize'>function.getimagesize</a>]: failed to open stream: no suitable wrapper could be found

              Line 57 reads:
              if ($allowcaching && $dimensions = getimagesize($pre.$filename, $info)) {

              The following condition is false, therefore $allowcaching becomes true and the external picture is still processed:
              if ($config['setting_phpthumb_nohotlink_enabled']) {
                      // NOT TRUE
                    } else {
                      $allowcaching = true; // BECOMES TRUE
                    }

              The default of "phpthumb_nohotlink_enabled" is true by default, as it is with my installation. You’re using "setting_phpthumb_nohotlink_enabled" - is the "setting_" in front of it correct?
                • 10713
                • 41 Posts
                Of course, the "setting_" prefix is a mistake... Just delete that (on both $config[] values) and try again.
                The script should now not even try to getimagesize(), when the MODx settings don’t allow hotlinking:

                <?php
                /**
                 * @name AutoFixImageSize
                 * @version 1.0.0 rc2
                 * @author Gerrit van Aaken <[email protected]> April–June 2011
                 *
                 * @license GPLv2
                 *
                 * Fixes img elements with wrong width/height attributes. 
                 * Uses phpThumbOf for generating correctly sized physical image files.
                 *
                 * Must be executed at "OnWebPagePrerender"
                 */
                
                // $modx->setDebug(E_USER_ERROR);
                // $modx->setLogLevel(modX::LOG_LEVEL_DEBUG);
                
                // get parsed document as string
                $str = $modx->resource->_output;
                
                // get configuration from global object
                $config = $modx->getConfig();
                
                // find all img elements with a src attribute
                preg_match_all('|\<img.*?src=[",\'](.*?)[",\'].*?[^>]+\>|i', $str, $filenames);
                
                // loop through all found img elements
                foreach($filenames[1] as $i => $filename) {
                  
                  $img_old = $filenames[0][$i];
                  $allowcaching = false; // pessimistic
                
                // $modx->log(modX::LOG_LEVEL_DEBUG, 'Prevent hotlinking: '.$config['phpthumb_nohotlink_enabled']);
                  
                  // is file already cached?
                  if (strpos($filename,"phpthumb") == false) {
                
                    // check if external caching is allowed
                    if (substr($filename,0,7) == "http://" || substr($filename,0,8) == "https://") {
                      $pre = "";
                      if ($config['phpthumb_nohotlink_enabled']) {
                        foreach (explode(",", $config['phpthumb_nohotlink_valid_domains']) as $alldomain) {
                          if ( strpos(strtolower($filename), strtolower(trim($alldomain))) != false) {
                            $allowcaching = true;
                          }
                        } 
                      } else {
                        $allowcaching = true;
                      }
                    } else {
                      $pre = $config['base_path'];
                      $allowcaching = true;
                    }
                  }
                  
                  // do we have physical access to the file?
                  if ($allowcaching && $dimensions = getimagesize($pre.$filename, $info)) {
                
                    // find width and height attribut and save value
                    preg_match_all('|width=[",\']([0-9]+?)[",\']|i', $filenames[0][$i], $widths);
                    $width = $widths[1][0];
                    preg_match_all('|height=[",\']([0-9]+?)[",\']|i', $filenames[0][$i], $heights);
                    $height = $heights[1][0];
                
                    // if resizing needed...
                    if (($width && $width != $dimensions[0]) || ($height && $height != $dimensions[1])) {
                
                      // prepare resizing metadata
                      $filetype = strtolower(substr($filename, strrpos($filename,".")+1));
                      $image = array();
                      $image['input'] = $filename;
                      $image['options'] = "f=".$filetype."&h=".$height."&w=".$width; 
                
                      // perform physical resizing and caching via phpthumbof
                      $cacheurl = $modx->runSnippet('phpthumbof',$image);
                
                      // set freshly cached image file location into old src attribute
                      $img_new = str_replace($filename, $cacheurl, $img_old);  
                
                      // replace old image element with new one on whole page content
                      $str = str_replace($img_old, $img_new, $str);  
                    }
                  }
                }
                
                // exchange the output string with the replaced one
                $modx->resource->_output = $str;
                
                  • 30107
                  • 44 Posts
                  line 62:
                  if ($allowcaching && $dimensions = getimagesize($pre.$filename, $info)) {

                  getimagesize() reports an error if there are any spaces in the filename (sic!). To overcome this, you could add the following line before line 62:
                  $filename = str_replace('%20', ' ', $filename);

                  ...and the following afterwards:
                  $filename = str_replace(' ', '%20', $filename);
                    • 10713
                    • 41 Posts
                    The newest version rc3 tolerates spaces in filenames. Check it out!
                      • 30107
                      • 44 Posts
                      Quote from: gerritvanaaken at Jun 13, 2011, 05:12 AM

                      The newest version rc3 tolerates spaces in filenames. Check it out!

                      Got it! Thanks, Gerrit.
                      While we are at it (or just to feed my curiosity): I wonder why you do explicitly set the filetype in line 67?
                      $image['options'] = "f=".$filetype."&h=".$height."&w=".$width; 

                      Since it’s always the same as the original one, this parameter is of no use, is it?