Skip to content

Commit

Permalink
Fixed issue #CR-1386: prevent errors when try to load uninstalled the…
Browse files Browse the repository at this point in the history
…mes through the config.xml (#3646)
  • Loading branch information
ptelu committed Dec 4, 2023
1 parent c5680fd commit 1faa998
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 71 deletions.
2 changes: 2 additions & 0 deletions application/controllers/ThemeOptionsController.php
Expand Up @@ -472,6 +472,8 @@ public function actionIndex()
}

$aData['oSurveyTheme'] = $oSurveyTheme;
$aData['aTemplatesWithoutDB'] = TemplateConfig::getTemplatesWithNoDb();

$aData['canImport'] = $canImport;
$aData['importErrorMessage'] = $importErrorMessage;
$aData['pageSize'] = App()->user->getState('pageSizeTemplateView', App()->params['defaultPageSize']); // Page size
Expand Down
6 changes: 5 additions & 1 deletion application/controllers/admin/Themes.php
Expand Up @@ -813,7 +813,11 @@ public function deleteBrokenTheme()

if (Permission::model()->hasGlobalPermission('templates', 'delete')) {
// First we check that the theme is really broken
$aBrokenThemes = Template::getBrokenThemes();
$aBrokenThemes = [];
$aTemplatesWithNoDB = TemplateConfig::getTemplatesWithNoDb();
if (!empty($aTemplatesWithNoDB['invalid'])) {
$aBrokenThemes = $aTemplatesWithNoDB['invalid'];
}
$templatename = sanitize_dirname($templatename);
if (array_key_exists($templatename, $aBrokenThemes)) {
if (rmdirr(Yii::app()->getConfig('userthemerootdir') . "/" . $templatename)) {
Expand Down
25 changes: 0 additions & 25 deletions application/models/Template.php
Expand Up @@ -784,31 +784,6 @@ public static function getDeprecatedTemplates()
return $aTemplateList;
}

/**
* Retrieves a list of broken themes
*/
public static function getBrokenThemes($sFolder = null)
{
$aBrokenTemplateList = array();
$sFolder = (empty($sFolder)) ? App()->getConfig("userthemerootdir") : $sFolder;

if ($sFolder && $handle = opendir($sFolder)) {
while (false !== ($sFileName = readdir($handle))) {
if (!is_file("$sFolder/$sFileName") && $sFileName != "." && $sFileName != ".." && $sFileName != ".svn" && $sFileName != 'generalfiles') {
try {
$oTheme = Template::getTemplateConfiguration($sFileName, null, null, true); // Get the manifest;
} catch (Exception $e) {
$aBrokenTemplateList[$sFileName] = $e;
}
}
}
closedir($handle);
}
ksort($aBrokenTemplateList);
return $aBrokenTemplateList;
}


/**
* Returns the static model of the specified AR class.
* Please note that you should have this exact method in all your CActiveRecord descendants!
Expand Down
40 changes: 23 additions & 17 deletions application/models/TemplateConfig.php
Expand Up @@ -84,10 +84,6 @@ class TemplateConfig extends CActiveRecord
/** @var array $aCssFrameworkReplacement Css Framework Replacement */
protected $aCssFrameworkReplacement;

public $allDbTemplateFolders = null;

public static $aTemplatesWithoutDB = null;

public $options_page = 'core';

/**
Expand Down Expand Up @@ -1083,9 +1079,10 @@ public static function convertOptionsToJson($options)
* Returns an array of all unique template folders that are registered in the database
* @return array|null
*/
public function getAllDbTemplateFolders()
public static function getAllDbTemplateFolders()
{
if (empty($this->allDbTemplateFolders)) {
static $aAllDbTemplateFolders = [];
if (empty($aAllDbTemplateFolders)) {
$oCriteria = new CDbCriteria();
$oCriteria->select = 'folder';
$oAllDbTemplateFolders = Template::model()->findAll($oCriteria);
Expand All @@ -1095,33 +1092,42 @@ public function getAllDbTemplateFolders()
$aAllDbTemplateFolders[] = $oAllDbTemplateFolder->folder;
}

$this->allDbTemplateFolders = array_unique($aAllDbTemplateFolders);
$aAllDbTemplateFolders = array_unique($aAllDbTemplateFolders);
}

return $this->allDbTemplateFolders;
return $aAllDbTemplateFolders;
}

/**
* Returns an array with uninstalled and/or incompatible survey themes
* @return TemplateConfiguration[]
*/
public function getTemplatesWithNoDb(): array
public static function getTemplatesWithNoDb(): array
{
if (empty(self::$aTemplatesWithoutDB)) {
static $aTemplatesWithoutDB = [];
if (empty($aTemplatesWithoutDB)) {
$aTemplatesWithoutDB['valid'] = [];
$aTemplatesWithoutDB['invalid'] = [];
$aTemplatesDirectories = Template::getAllTemplatesDirectories();
$aTemplatesInDb = $this->getAllDbTemplateFolders();
$aTemplatesWithoutDB = array();
$aTemplatesInDb = self::getAllDbTemplateFolders();

foreach ($aTemplatesDirectories as $sName => $sPath) {
if (!in_array($sName, $aTemplatesInDb)) {
// Get the manifest
$aTemplatesWithoutDB[$sName] = Template::getTemplateConfiguration($sName, null, null, true);
// Get the theme manifest by forcing xml load
try {
$aTemplatesWithoutDB['valid'][$sName] = Template::getTemplateConfiguration($sName, null, null, true);
if (empty($aTemplatesWithoutDB['valid'][$sName]->config)) {
unset($aTemplatesWithoutDB['valid'][$sName]);
$aTemplatesWithoutDB['invalid'][$sName]['error'] = gT('Invalid theme configuration file');
}
} catch (Exception $e) {
unset($aTemplatesWithoutDB['valid'][$sName]);
$aTemplatesWithoutDB['invalid'][$sName]['error'] = $e->getMessage();
}
}
}
self::$aTemplatesWithoutDB = $aTemplatesWithoutDB;
}

return self::$aTemplatesWithoutDB;
return $aTemplatesWithoutDB;
}

/**
Expand Down
42 changes: 25 additions & 17 deletions application/models/TemplateManifest.php
Expand Up @@ -1015,33 +1015,41 @@ public static function extendsConfig($sToExtends, $sNewName)

/**
* Read the config.xml file of the template and push its contents to $this->config
* @throws Exception
*/
private function readManifest()
private function readManifest(): void
{
$this->xmlFile = $this->path . 'config.xml';

if (file_exists(realpath($this->xmlFile))) {
if (\PHP_VERSION_ID < 80000) {
$bOldEntityLoaderState = libxml_disable_entity_loader(true); // @see: http://phpsecurity.readthedocs.io/en/latest/Injection-Attacks.html#xml-external-entity-injection
$bOldEntityLoaderState = libxml_disable_entity_loader(
true
); // @see: http://phpsecurity.readthedocs.io/en/latest/Injection-Attacks.html#xml-external-entity-injection
}
$sXMLConfigFile = file_get_contents(realpath($this->xmlFile)); // @see: Now that entity loader is disabled, we can't use simplexml_load_file; so we must read the file with file_get_contents and convert it as a string
$sXMLConfigFile = file_get_contents(
realpath($this->xmlFile)
); // @see: Now that entity loader is disabled, we can't use simplexml_load_file; so we must read the file with file_get_contents and convert it as a string
$oDOMConfig = new DOMDocument();
$oDOMConfig->loadXML($sXMLConfigFile);
$oXPath = new DOMXpath($oDOMConfig);
foreach ($oXPath->query('//comment()') as $oComment) {
$oComment->parentNode->removeChild($oComment);
}
$oXMLConfig = simplexml_import_dom($oDOMConfig);
$filenames = $oXMLConfig->config->xpath("//file");
if ($filenames) {
foreach ($filenames as $oFileName) {
$oFileName[0] = get_absolute_path($oFileName[0]);
// the loadXML is error suppressed, so we can check the return value
$bLoadXMLSuccess = @$oDOMConfig->loadXML($sXMLConfigFile);
if ($bLoadXMLSuccess) {
$oXPath = new DOMXpath($oDOMConfig);
foreach ($oXPath->query('//comment()') as $oComment) {
$oComment->parentNode->removeChild($oComment);
}
$oXMLConfig = simplexml_import_dom($oDOMConfig);
$filenames = $oXMLConfig->config->xpath("//file");
if ($filenames) {
foreach ($filenames as $oFileName) {
$oFileName[0] = get_absolute_path($oFileName[0]);
}
}
}

$this->config = $oXMLConfig; // Using PHP >= 5.4 then no need to decode encode + need attributes : then other function if needed :https://secure.php.net/manual/en/book.simplexml.php#108688 for example
if (\PHP_VERSION_ID < 80000) {
libxml_disable_entity_loader($bOldEntityLoaderState); // Put back entity loader to its original state, to avoid contagion to other applications on the server
$this->config = $oXMLConfig; // Using PHP >= 5.4 then no need to decode encode + need attributes : then other function if needed :https://secure.php.net/manual/en/book.simplexml.php#108688 for example
if (\PHP_VERSION_ID < 80000) {
libxml_disable_entity_loader($bOldEntityLoaderState); // Put back entity loader to its original state, to avoid contagion to other applications on the server
}
}
} else {
throw new Exception(" Error: Can't find a manifest for $this->sTemplateName in ' $this->path ' ");
Expand Down
18 changes: 7 additions & 11 deletions application/views/themeOptions/index.php
Expand Up @@ -8,6 +8,7 @@
* @var TemplateConfig $oSurveyTheme
* @var int $pageSize
* @var array $aAdminThemes
* @var array $aTemplatesWithoutDB
*/

// TODO: rename to template_list.php and move to template controller
Expand Down Expand Up @@ -43,8 +44,7 @@
]
); ?>
<!-- Available Themes -->
<?php $templatewithNoDb = $oSurveyTheme->getTemplatesWithNoDb()?>
<?php if (count($templatewithNoDb) > 0) : ?>
<?php if (!empty($aTemplatesWithoutDB['valid'])) : ?>
<h3><?php eT('Available survey themes:'); ?></h3>
<div id="templates_no_db" >
<table class="items table table-hover">
Expand All @@ -62,7 +62,7 @@
<tbody>
<?php /** @var TemplateManifest $oTemplate */ ?>
<?php $surveyThemeIterator = 0 ?>
<?php foreach ($templatewithNoDb as $key => $oTemplate) : ?>
<?php foreach ($aTemplatesWithoutDB['valid'] as $key => $oTemplate) : ?>
<tr class="odd">
<td class="col-lg-1"><?php echo $oTemplate->getPreview(); ?></td>
<td class="col-lg-2"><?php echo $oTemplate->sTemplateName; ?></td>
Expand Down Expand Up @@ -134,11 +134,8 @@ class="btn btn-danger btn-sm selector--ConfirmModal">
<?php endif; ?>
<!-- End Available Themes -->
<!-- Broken Themes -->
<?php $aBrokenThemes = Template::getBrokenThemes();
if (count($aBrokenThemes) > 0) : ?>
<?php if (!empty($aTemplatesWithoutDB['invalid'])) : ?>
<h3><?php eT('Broken survey themes'); ?></h3>


<div id="thembes_broken" >
<table class="items table table-hover">
<thead>
Expand All @@ -150,12 +147,11 @@ class="btn btn-danger btn-sm selector--ConfirmModal">
</thead>

<tbody>
<?php foreach ($aBrokenThemes as $sName => $oBrokenTheme) : ?>
<?php // echo $oTemplate; ?>
<?php foreach ($aTemplatesWithoutDB['invalid'] as $sName => $oBrokenTheme) : ?>
<tr class="odd">
<td class="col-lg-1 text-danger"><?php echo $sName; ?></td>
<td class="col-lg-1 text-danger"><?= $sName ?></td>
<td class="col-lg-8 ">
<blockquote><?php echo $oBrokenTheme->getMessage(); ?></blockquote>
<blockquote><?= $oBrokenTheme['error'] ?? '' ?></blockquote>
</td>
<td class="col-lg-2">
<div class="d-grid gap-2">
Expand Down

0 comments on commit 1faa998

Please sign in to comment.