I see that advagg makes use of the exec() command. I was searching my Drupal codebase to learn what modules where making use of the exec() command and surprisingly you were one of only four use cases in my installation.

The reason that I was researching this is because I was tightening security on my server and first wanted to see if I could eliminate the use of exec() and system() in order to prevent abuse from an intruder. Further, if these functions need to be open I wanted to see if I could use open_basedir to prevent PHP from accessing executable files outside my document root.

So the point is, I wonder if it is possible to improve the security of this function by using PHP libraries instead or copying whatever executables are needed into the advagg module folder. Thoughts?

Also while studying the code I noticed that the calling function is designed to compress a string and begins by creating a file, executing a shell process to compress the file, and then reads the file. Wow, I was wondering how efficient that really is? Would it be possible to perform the compression without dealing with additional file i/o and process creation, but compressing the string using PHP functions? Perhaps I do not understand all that is happening.

/**
 * Compress Javascript using via command line.
 *
 * @param string $input_file
 *   The file containing the uncompressed css/js data.
 * @param string $ext
 *   The string css or js.
 * @param array $debug
 *   Optinal debug array.
 *
 * @return string
 *   The filename containting the compressed css/js data.
 */
function advagg_ext_compress_execute_cmd($input_file, $ext = '', array &$debug = array()) {
  // Get file extension.
  if (empty($ext)) {
    $ext = strtolower(pathinfo($input_file, PATHINFO_EXTENSION));
    if ($ext !== 'css' && $ext !== 'js') {
      // Get the $ext from the database.
      $row = db_select('advagg_files', 'af')
        ->fields('af')
        ->condition('filename', $input_file)
        ->execute()->fetchAssoc();
      if (!empty($row['filetype'])) {
        $ext = $row['filetype'];
      }
      if ($ext === 'less') {
        $ext = 'css';
      }
    }
  }

  // Generate temp file.
  $temp_file = drupal_tempnam('temporary://', 'advagg_file_');
  $new_temp_file = $temp_file . '.' . basename($input_file);
  @rename($temp_file, $new_temp_file);
  // Set the permissions on the temp file.
  drupal_chmod($new_temp_file);
  $output = advagg_get_relative_path($new_temp_file);

  // Create command to run.
  $run = variable_get("advagg_ext_compress_{$ext}_cmd", '');
  $cmd = str_replace(array(
    '{%CWD%}',
    '{%IN%}',
    '{%IN_URL_ENC%}',
    '{%OUT%}',
  ), array(
    DRUPAL_ROOT,
    $input_file,
    urlencode(file_create_url($input_file)),
    escapeshellarg(realpath($output)),
  ), $run);

  // Run command and return the output file.
  $shell_output = array();
  $return_var = 0;
  $shell = exec($cmd, $shell_output, $return_var);
  $debug = array($cmd, $shell_output, $return_var, $shell);
  return $output;
}

Comments

webservant316 created an issue. See original summary.

mikeytown2’s picture

This is for external compressors; as in no php equivalent. You have to install this sub module for this code to be used. I don't know of any way to call external code from outside of php other than use one of the shell functions.

If you don't use this sub module then it'll compress css/js using pure php.

webservant316’s picture

Got it. I do not have the external compressor sub-module installed, but instead have the jsmin php extension installed. So then this code is not called in my case?

Though I wonder what a performance evaluation of using an external compressor would reveal. Seems like it might be just as fast to use a PHP function to strip out all white space and be done with it rather than create a file and crank up a shell process.

mikeytown2’s picture

You are correct, exec is not called in your case.

By providing an external compressor option it allows for flexibility; windows boxes that can't use the jsmin php extension can now use AjaxMinifier.exe or https://closure-compiler.appspot.com/home or ... to compress js files. Usually the compression takes seconds where as file creation takes milliseconds; thus I'm not worried about micro optimizations in this code path, the external call is the slowest part, not using files would make the code vastly more complex.

webservant316’s picture

thanks. sounds like you have a good handle on it. I was probably thinking old school when one could do a whole lot of processing when compared with file i/o.

again thanks.

mikeytown2’s picture

Status: Active » Closed (works as designed)