Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed issue #CR-1386: prevent errors when try to load uninstalled themes through the config.xml #3646

Merged
merged 3 commits into from
Dec 4, 2023
Merged

Fixed issue #CR-1386: prevent errors when try to load uninstalled themes through the config.xml #3646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d8ec6e6
WIP: combined loading of uninstalled themes to prevent errors when tr…
ptelu Nov 28, 2023
5622cbf
WIP: static vars moved to the functions to reduce clutter
ptelu Nov 29, 2023
185768f
WIP: adjust delete themes code
ptelu Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions application/controllers/ThemeOptionsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -784,31 +784,6 @@ public static function getDeprecatedTemplates()
return $aTemplateList;
}

/**
* Retrieves a list of broken themes
*/
public static function getBrokenThemes($sFolder = null)
twilligls marked this conversation as resolved.
Show resolved Hide resolved
{
$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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Loading