Contrib.social

Fix by checking if shell_exec is not in the disable_functions list and use the Drupal file system service to manage copy or removal of files

Open on Drupal.org →
Created on 11 October 2021, over 2 years ago
Updated 26 July 2023, 11 months ago

Problem/Motivation

Some shared hosting or servers do not allow running shell_exec on the production server.
That brings fatal errors while running Opigno on them.

Error: Call to undefined function shell_exec()

disable_functions "" PHP_INI_SYSTEM only link

PHP_INI_SYSTEM Entry can be set in php.ini or httpd.conf link

I was wondering if the PHP.ini for a shared system could have the exec and shell_exec were added to the disable_functions

The disable_functions can be only managed at the system level ( which worked under VM or Dedicated servers)
Most shared hosting can not change disable_functions in the user or folder level for php.ini
Even tho changing that in the settings.php or settings.local.php will not work.
Because all PHP_INI_SYSTEM PHP configs settings can only be changed from the php.ini for the system

Proposed resolution

Check if shell_exec is callable and in the enabled functions list


/**
 * Check if shell_exec is callable and in the enabled functions list.
 */
function opigno_module_is_shell_exec_enabled() {
  return is_callable('shell_exec') && false === stripos(ini_get('disable_functions'), 'shell_exec');
}

And call if (opigno_module_is_shell_exec_enabled()) before any call for shell_exec

Proposing to change with the new logic from Unmanaged file functions replaced with a service →

#2244513: Move the unmanaged file APIs to the file_system service (file.inc) →
#3023015: Replace usages of deprecated constants in file.inc →
#3038988: Remove support for empty destination in FileSystem::copy(), FileSystem::move() and FileSystem::saveData() → 

<?php

try {
    \Drupal::service('file_system')->copy($source, $destination, $replace);
    \Drupal::service('file_system')->move($source, $destination, $replace);
    \Drupal::service('file_system')->delete($path);
    \Drupal::service('file_system')->deleteRecursive($path, $callback);
    \Drupal::service('file_system')->saveData($data, $destination, $replace);
    \Drupal::service('file_system')->prepareDirectory($directory, $options);
    \Drupal::service('file_system')->getDestinationFilename($destination, $replace);
    \Drupal::service('file_system')->createFilename($basename, $directory);
} 
catch (\Drupal\Core\File\Exception\FileException $e) {
    // Log or set message or doing something else.
}
?>

Remaining tasks

  • Fix patch
  • Test
  • Review

User interface changes

N/A

API changes

N/A

Data model changes

N/A

🐛 Bug report
Status

Downport

Version

3.0

Component

Code

Created by

🇯🇴Jordan Rajab Natshah Jordan

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment 12 months ago →
    🇮🇳India hash6

    @Rajab Natshah I had the same issue while trying to view activity types when we click on New Activity page

    Page route - /module/manager/get-activity-types

    Error: Call to undefined function Drupal\opigno_module\Controller\shell_exec() in Drupal\opigno_module\Controller\OpignoModuleManagerController::getPptConvertAllow() (line 446 of /opilms/web/modules/contrib/opigno_module/src/Controller/OpignoModuleManagerController.php)

    Your code in patch resolves the issue, we will have to update code to all places where it has shell_access function.

  • Status changed to Needs work 12 months ago
  • Comment 12 months ago →
    🇮🇳India hash6
  • Issue was unassigned.
  • Comment 12 months ago →
    🇯🇴Jordan Rajab Natshah Jordan

    Re-role for the patch to work after opigno_module-3.1.1 →

  • Assigned to Rajab Natshah
  • Status changed to Active 12 months ago
  • Comment 12 months ago →
    🇯🇴Jordan Rajab Natshah Jordan
  • @rajab-natshah opened merge request.
  • Issue was unassigned.
  • Status changed to Needs review 12 months ago
  • Comment 12 months ago →
    🇯🇴Jordan Rajab Natshah Jordan
  • Comment 11 months ago →
    🇨🇭Switzerland axelm

    Hi,
    Thanks for raising this point.
    We cannot apply the patch suggested for several reasons:

    • It doesn’t fit code standards (static calls of services instead of dependency injection);
    • \Drupal::service('file_system')->copy($source_folder, $dest_folder); - this piece of code will not work because it’s possible to copy only files using Drupal file system, not directories.

    Submitted merge request could not be approved for the same reasons + it’s created to branch 8.x-1.x that is not the latest version.
    We will implement it in a different way and include this in the next Opigno release.

  • Status changed to Downport 11 months ago
  • Comment 11 months ago →
    🇨🇭Switzerland axelm
contrib.social Blog FAQ Discussions
Production build 0.69.0 2024