From 3fc50383c81b51e7718c9f29f9cef23dfadfa7fb Mon Sep 17 00:00:00 2001 From: Jason Coward Date: Wed, 11 Jul 2018 16:29:25 -0600 Subject: [PATCH 01/28] Filter user input used in phpthumb cherry-picked from #13979 --- core/docs/changelog.txt | 4 ++ core/model/phpthumb/modphpthumb.class.php | 59 ++++++++++++++++------- 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/core/docs/changelog.txt b/core/docs/changelog.txt index 5f84507ba00..1bfe05b9f6a 100644 --- a/core/docs/changelog.txt +++ b/core/docs/changelog.txt @@ -2,6 +2,10 @@ This file shows the changes in recent releases of MODX. The most current release is usually the development release, and is only shown to give an idea of what's currently in the pipeline. +MODX Revolution 2.6.5-pl (TBD) +==================================== +- Filter user input used in phpthumb [#13979] + MODX Revolution 2.6.4-pl (June 7, 2018) ==================================== - Fix sorting by access column in Template Access tab of Template Variable edit view [#13893] diff --git a/core/model/phpthumb/modphpthumb.class.php b/core/model/phpthumb/modphpthumb.class.php index 28f49e4c650..ff81eabf107 100644 --- a/core/model/phpthumb/modphpthumb.class.php +++ b/core/model/phpthumb/modphpthumb.class.php @@ -1,9 +1,7 @@ modx =& $modx; - $this->config = array_merge(array( + $this->config = $config; - ),$config); parent::__construct(); } /** * Setup some site-wide phpthumb options from modx config */ - public function initialize() { + public function initialize() + { $cachePath = $this->modx->getOption('core_path',null,MODX_CORE_PATH).'cache/phpthumb/'; - if (!is_dir($cachePath)) $this->modx->cacheManager->writeTree($cachePath); - $this->setParameter('config_cache_directory',$cachePath); - $this->setParameter('config_temp_directory',$cachePath); + if (!is_dir($cachePath)) { + $this->modx->cacheManager->writeTree($cachePath); + } + $this->setParameter('config_cache_directory', $cachePath); + $this->setParameter('config_temp_directory', $cachePath); $this->setCacheDirectory(); $this->setParameter('config_allow_src_above_docroot',(boolean)$this->modx->getOption('phpthumb_allow_src_above_docroot',$this->config,false)); @@ -51,13 +58,15 @@ public function initialize() { $this->setParameter('config_nooffsitelink_erase_image',(boolean)$this->modx->getOption('phpthumb_nooffsitelink_erase_image',$this->config,true)); $this->setParameter('config_nooffsitelink_watermark_src',(string)$this->modx->getOption('phpthumb_nooffsitelink_watermark_src',$this->config,'')); $this->setParameter('config_nooffsitelink_text_message',(string)$this->modx->getOption('phpthumb_nooffsitelink_text_message',$this->config,'Off-server linking is not allowed')); + $this->setParameter('config_ttf_directory', (string)$this->modx->getOption('core_path', $this->config, MODX_CORE_PATH) . 'model/phpthumb/fonts/'); + $this->setParameter('config_imagemagick_path', (string)$this->modx->getOption('phpthumb_imagemagick_path', $this->config, null)); + $this->setParameter('cache_source_enabled',(boolean)$this->modx->getOption('phpthumb_cache_source_enabled',$this->config,false)); $this->setParameter('cache_source_directory',$cachePath.'source/'); $this->setParameter('allow_local_http_src',true); $this->setParameter('zc',$this->modx->getOption('zc',$_REQUEST,$this->modx->getOption('phpthumb_zoomcrop',$this->config,0))); $this->setParameter('far',$this->modx->getOption('far',$_REQUEST,$this->modx->getOption('phpthumb_far',$this->config,'C'))); $this->setParameter('cache_directory_depth',4); - $this->setParameter('config_ttf_directory',$this->modx->getOption('core_path',$this->config,MODX_CORE_PATH).'model/phpthumb/fonts/'); $documentRoot = $this->modx->getOption('phpthumb_document_root',$this->config, ''); if ($documentRoot == '') $documentRoot = $this->modx->getOption('base_path', null, ''); @@ -65,10 +74,24 @@ public function initialize() { $this->setParameter('config_document_root',$documentRoot); } + // Only public parameters of phpThumb should be allowed to pass from user input. + // List properties between START PARAMETERS and START PARAMETERS in src/core/model/phpthumb/phpthumb.class.php + $allowed = array( + 'src', 'new', 'w', 'h', 'wp', 'hp', 'wl', 'hl', 'ws', 'hs', + 'f', 'q', 'sx', 'sy', 'sw', 'sh', 'zc', 'bc', 'bg', 'fltr', + 'goto', 'err', 'xto', 'ra', 'ar', 'aoe', 'far', 'iar', 'maxb', 'down', + 'md5s', 'sfn', 'dpi', 'sia', 'phpThumbDebug' + ); + /* iterate through properties */ foreach ($this->config as $property => $value) { - $this->setParameter($property,$value); + if (!in_array($property, $allowed, true)) { + $this->modx->log(modX::LOG_LEVEL_WARN,"Detected attempt of using private parameter `$property` (for internal usage) of phpThumb that not allowed and insecure"); + continue; + } + $this->setParameter($property, $value); } + return true; } @@ -317,5 +340,5 @@ function ResolveFilenameToAbsolute($filename) { } return $AbsoluteFilename; } - } + From 606dc0f1635de4b699d1151616af75e5c08d4cdd Mon Sep 17 00:00:00 2001 From: Jason Coward Date: Wed, 11 Jul 2018 19:01:57 -0600 Subject: [PATCH 02/28] Prevent directory traversal and limit files deleted when clearing modFileRegister --- _build/transport.core.php | 2 +- core/docs/changelog.txt | 1 + core/model/modx/modfilehandler.class.php | 2 +- .../modx/registry/modfileregister.class.php | 43 +++++++++++++------ 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/_build/transport.core.php b/_build/transport.core.php index e06c1cc5ce5..0d07085980e 100644 --- a/_build/transport.core.php +++ b/_build/transport.core.php @@ -108,7 +108,7 @@ $cacheManager->deleteTree($packageDirectory . 'core',array( 'deleteTop' => true, 'skipDirs' => false, - 'extensions' => '*', + 'extensions' => array(), )); } if (!file_exists($packageDirectory . 'core') && !file_exists($packageDirectory . 'core.transport.zip')) { diff --git a/core/docs/changelog.txt b/core/docs/changelog.txt index 1bfe05b9f6a..5c11318a1d1 100644 --- a/core/docs/changelog.txt +++ b/core/docs/changelog.txt @@ -4,6 +4,7 @@ development release, and is only shown to give an idea of what's currently in th MODX Revolution 2.6.5-pl (TBD) ==================================== +- Prevent directory traversal and limit files deleted when clearing modFileRegister [#13980] - Filter user input used in phpthumb [#13979] MODX Revolution 2.6.4-pl (June 7, 2018) diff --git a/core/model/modx/modfilehandler.class.php b/core/model/modx/modfilehandler.class.php index 4fafa19fb7f..0254152493c 100644 --- a/core/model/modx/modfilehandler.class.php +++ b/core/model/modx/modfilehandler.class.php @@ -632,7 +632,7 @@ public function remove($options = array()) { $options = array_merge(array( 'deleteTop' => true, 'skipDirs' => false, - 'extensions' => '', + 'extensions' => array(), ), $options); $this->fileHandler->modx->getCacheManager(); diff --git a/core/model/modx/registry/modfileregister.class.php b/core/model/modx/registry/modfileregister.class.php index dae744b928f..8441b9b1692 100644 --- a/core/model/modx/registry/modfileregister.class.php +++ b/core/model/modx/registry/modfileregister.class.php @@ -27,16 +27,21 @@ class modFileRegister extends modRegister { /** * Construct a new modFileRegister instance. * - * {@inheritdoc} + * @param modX &$modx A reference to a modX instance. + * @param string $key A valid PHP variable which will be set on the modRegistry instance. + * @param array $options Optional array of registry options. */ - function __construct(& $modx, $key, $options = array()) { - parent :: __construct($modx, $key, $options); + function __construct(& $modx, $key, $options = array()) + { + parent::__construct($modx, $key, $options); + $modx->getCacheManager(); $this->directory = $modx->getCachePath() . 'registry/'; $this->directory .= isset($options['directory']) - ? $options['directory'] - : $key; - if ($this->directory[strlen($this->directory)-1] != '/') $this->directory .= '/'; + ? $options['directory'] + : $key; + + $this->directory = rtrim($this->directory, '/') . '/'; } /** @@ -57,12 +62,16 @@ public function connect(array $attributes = array()) { * * {@inheritdoc} */ - public function clear($topic) { - $topicDirectory = $this->directory; - $topicDirectory.= $topic[0] == '/' ? substr($topic, 1) : $topic ; - return $this->modx->cacheManager->deleteTree($topicDirectory, array( - 'extensions' => '.msg.php' - )); + public function clear($topic) + { + $topicDirectory = $this->directory . ltrim($this->sanitizePath($topic), '/'); + + return $this->modx->cacheManager->deleteTree( + realpath($topicDirectory), + array( + 'extensions' => array('.msg.php') + ) + ); } /** @@ -263,4 +272,14 @@ public function send($topic, $message, array $options = array()) { public function close() { return true; } + + /** + * Sanitize the specified path + * + * @param string $path The path to clean + * @return string The sanitized path + */ + protected function sanitizePath($path) { + return preg_replace(array("/\.*[\/|\\\]/i", "/[\/|\\\]+/i"), array('/', '/'), $path); + } } From 4ab357178e98708feeb92392557a4f3c87f0cebc Mon Sep 17 00:00:00 2001 From: Jason Coward Date: Wed, 11 Jul 2018 19:05:23 -0600 Subject: [PATCH 03/28] MODX Revolution 2.6.5-pl --- _build/build.sample.properties | 2 +- _build/build.xml | 2 +- core/docs/changelog.txt | 2 +- core/docs/version.inc.php | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/_build/build.sample.properties b/_build/build.sample.properties index 577973b5af0..cba51093822 100644 --- a/_build/build.sample.properties +++ b/_build/build.sample.properties @@ -11,7 +11,7 @@ git.command = git #project.name.fs = modx # Override to set the version and release strings -#modx.core.version = 2.6.4 +#modx.core.version = 2.6.5 #modx.core.release = pl # these properties require a local Git clone in order to produce distributions diff --git a/_build/build.xml b/_build/build.xml index 494d3400473..4f4d0ff9017 100644 --- a/_build/build.xml +++ b/_build/build.xml @@ -19,7 +19,7 @@ - + diff --git a/core/docs/changelog.txt b/core/docs/changelog.txt index 5c11318a1d1..8deec577d0a 100644 --- a/core/docs/changelog.txt +++ b/core/docs/changelog.txt @@ -2,7 +2,7 @@ This file shows the changes in recent releases of MODX. The most current release is usually the development release, and is only shown to give an idea of what's currently in the pipeline. -MODX Revolution 2.6.5-pl (TBD) +MODX Revolution 2.6.5-pl (July 11, 2018) ==================================== - Prevent directory traversal and limit files deleted when clearing modFileRegister [#13980] - Filter user input used in phpthumb [#13979] diff --git a/core/docs/version.inc.php b/core/docs/version.inc.php index cfa2f3b1cf0..a6ce468d8e0 100644 --- a/core/docs/version.inc.php +++ b/core/docs/version.inc.php @@ -2,7 +2,7 @@ $v= array (); $v['version']= '2'; // Current version. $v['major_version']= '6'; // Current major version. -$v['minor_version']= '4'; // Current minor version. +$v['minor_version']= '5'; // Current minor version. $v['patch_level']= 'pl'; // Current patch level. $v['code_name']= 'Revolution'; // Current codename. $v['distro']= '@git@'; From eef48c8d09529f7f1380bc2a5ed410065f37590a Mon Sep 17 00:00:00 2001 From: Jason Coward Date: Fri, 13 Jul 2018 14:17:23 -0600 Subject: [PATCH 04/28] Fix zips from build to ensure they do not include 777/666 attributes --- _build/build.xml | 5 +++++ _build/fixzip.php | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 _build/fixzip.php diff --git a/_build/build.xml b/_build/build.xml index 4f4d0ff9017..6745341b845 100644 --- a/_build/build.xml +++ b/_build/build.xml @@ -11,6 +11,7 @@ + @@ -97,6 +98,7 @@ + @@ -210,6 +212,7 @@ + @@ -311,6 +314,7 @@ + @@ -374,6 +378,7 @@ + diff --git a/_build/fixzip.php b/_build/fixzip.php new file mode 100644 index 00000000000..21797c38642 --- /dev/null +++ b/_build/fixzip.php @@ -0,0 +1,26 @@ +open($zipFileName); + +for ($i = 0; $i < $zip->count(); $i++) { + $zip->setExternalAttributesIndex($i, 0, 0); +} + +$zip->close(); + +return 0; From f291a8371b134d6eff9e88c5932023e50f1a7423 Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Sat, 28 Jul 2018 03:21:22 +0200 Subject: [PATCH 05/28] [Package Manager] Re-Download missing packages Was surprised that the package manager is not able to re-download packages in case the zip files / extracted folders are missing for an installed package (for whatever reason, in my particular case did I want to save space for backups and did not include the packages folder...). When trying to re-install a package with missing files it would proceed to the installation (of course without metadata displaying), then when hitting "Continue" show the modal and say could not download the package etc., but it never actually tried to re-download... This PR changes that so if the zip file is not present, it would actually try to re-download it. There is an edge case where the package is not available anymore on the provider (for example I had an old version of the "console" extra that was not available anymore) and the provider would return an error code (e.g. 404 in that case), but the old code would actually "package" the response HTML or JSON code into a .zip file and then of course the installation would fail with "could not unpack package"...This PR also solves that issue by handling error codes from the provider. Although not used very often, I did rewrite large parts of the _getByFsockopen() method as it needed some love. --- .../transport/modtransportpackage.class.php | 71 +++++++++++++------ 1 file changed, 49 insertions(+), 22 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 7cbba8f11b3..b3fc32d53c0 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -362,8 +362,18 @@ public function transferPackage($sourceFile, $targetDir) { if (is_dir($targetDir) && is_writable($targetDir)) { if (!is_array($this->xpdo->version)) { $this->xpdo->getVersionData(); } $productVersion = $this->xpdo->version['code_name'].'-'.$this->xpdo->version['full_version']; - - $source= $this->get('service_url') . $sourceFile.(strpos($sourceFile,'?') !== false ? '&' : '?').'revolution_version='.$productVersion; + + // make sure the package is downloaded, if not attempt re-download + if (!file_exists($targetDir . $sourceFile)) { + // not very pretty way of getting the download URL, otherwise we have to make a + // remote request to $this->Provider->info($this->signature) to get the URL + $location = $this->get('metadata')[array_keys(array_filter($this->get('metadata'), function($item) { if ($item['name'] === 'location') { return $item; } }))[0]]; + + // assign remote download URL + $source = $location['text']; + } else { + $source = $this->get('service_url') . $sourceFile.(strpos($sourceFile,'?') !== false ? '&' : '?').'revolution_version='.$productVersion; + } /* see if user has allow_url_fopen on and is not behind a proxy */ $proxyHost = $this->xpdo->getOption('proxy_host',null,''); @@ -417,6 +427,11 @@ public function transferPackage($sourceFile, $targetDir) { } } $content = curl_exec($ch); + $status = curl_getinfo($ch, CURLINFO_HTTP_CODE); + + if ($status >= 400) { + $content = ''; + } curl_close($ch); } @@ -432,7 +447,7 @@ public function transferPackage($sourceFile, $targetDir) { $transferred= $cacheManager->writeFile($target, $content); } } else { - $this->xpdo->log(xPDO::LOG_LEVEL_ERROR,'MODX could not download the file. You must enable allow_url_fopen, cURL or fsockopen to use remote transport packaging.'); + $this->xpdo->log(xPDO::LOG_LEVEL_ERROR, 'MODX could not download the file. ' . (isset($status) ? 'Server responded with status code ' . $status . '.' : 'You must enable allow_url_fopen, cURL or fsockopen to use remote transport packaging.')); } } else { $this->xpdo->log(xPDO::LOG_LEVEL_ERROR,$this->xpdo->lexicon('package_err_target_write',array( @@ -661,7 +676,9 @@ protected function _getByFsockopen($url) { $purl = parse_url($url); $host = $purl['host']; $path = !empty($purl['path']) ? $purl['path'] : '/'; - if (!empty($purl['query'])) { $path .= '?'.$purl['query']; } + if (!empty($purl['query'])) { + $path .= '?' . $purl['query']; + } $port = !empty($purl['port']) ? $purl['port'] : '80'; $timeout = 10; @@ -669,25 +686,35 @@ protected function _getByFsockopen($url) { $fp = @fsockopen($host,$port,$errno,$errstr,$timeout); if( !$fp ) { - $this->xpdo->log(xPDO::LOG_LEVEL_ERROR,'Could not retrieve from '.$url); + $this->xpdo->log(xPDO::LOG_LEVEL_ERROR, 'Could not retrieve from ' . $url); } else { - fwrite($fp, "GET $path ".$_SERVER['SERVER_PROTOCOL']."\r\n" . - "Host: $host\r\n" . - "User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3\r\n" . - "Accept: */*\r\n" . - "Accept-Language: en-us,en;q=0.5\r\n" . - "Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7\r\n" . - "Keep-Alive: 300\r\n" . - "Connection: keep-alive\r\n" . - "Referer: http://$host\r\n\r\n"); - - while ($line = fread($fp, 4096)) { - $response .= $line; - } - fclose($fp); - - $pos = strpos($response, "\r\n\r\n"); - $response = substr($response, $pos + 4); + $out = implode("\r\n", array( + "GET $path " . $_SERVER['SERVER_PROTOCOL'], + "Host: $host", + "User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3", + "Accept: */*", + "Accept-Language: en-us,en;q=0.5", + "Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7", + "Keep-Alive: 300", + "Connection: keep-alive", + "Referer: http://$host", + "\r\n" + )); + fwrite($fp, $out); + + $status = explode(' ', fgets($fp, 13))[1]; + + if (strpos($status, '4') !== false || strpos($status, '5') !== false) { + $response = ''; + } else { + while ($line = fread($fp, 4096)) { + $response .= $line; + } + $pos = strpos($response, "\r\n\r\n"); + $response = substr($response, $pos + 4); + } + + fclose($fp); } return $response; } From 51f4052ca69b0313b225c27b27d8da759f273634 Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Sun, 29 Jul 2018 02:29:32 +0200 Subject: [PATCH 06/28] Make monster oneliner multiline --- core/model/modx/transport/modtransportpackage.class.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index b3fc32d53c0..8bf650b8c4a 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -367,8 +367,11 @@ public function transferPackage($sourceFile, $targetDir) { if (!file_exists($targetDir . $sourceFile)) { // not very pretty way of getting the download URL, otherwise we have to make a // remote request to $this->Provider->info($this->signature) to get the URL - $location = $this->get('metadata')[array_keys(array_filter($this->get('metadata'), function($item) { if ($item['name'] === 'location') { return $item; } }))[0]]; - + $location = $this->get('metadata')[array_keys(array_filter($this->get('metadata'), function($item) { + if ($item['name'] === 'location') { + return $item; + } + }))[0]]; // assign remote download URL $source = $location['text']; } else { From bd29d3f550a74ae8ade92a968a7c89d3a384bf03 Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Sun, 29 Jul 2018 02:48:51 +0200 Subject: [PATCH 07/28] Split up oneliner into variables... Hopefully slightly more readable, also added comments. --- .../modx/transport/modtransportpackage.class.php | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 8bf650b8c4a..868deb6f55b 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -365,13 +365,18 @@ public function transferPackage($sourceFile, $targetDir) { // make sure the package is downloaded, if not attempt re-download if (!file_exists($targetDir . $sourceFile)) { - // not very pretty way of getting the download URL, otherwise we have to make a - // remote request to $this->Provider->info($this->signature) to get the URL - $location = $this->get('metadata')[array_keys(array_filter($this->get('metadata'), function($item) { + // not very pretty way of getting the download URL, but otherwise we have to make a + // remote request to $this->Provider->info($this->signature) to get the URL. + // loop trough metadata arrays until location is found + $locationarr = array_filter($this->get('metadata'), function($item) { if ($item['name'] === 'location') { return $item; } - }))[0]]; + }); + // determine the index at which the location metadata was found + $locationkey = array_keys($locationarr)[0]; + // get the location metadata + $location = $this->get('metadata')[$locationkey]; // assign remote download URL $source = $location['text']; } else { From 43a626c33185b0074ded10221171d341704c9f0b Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Sun, 29 Jul 2018 04:53:55 +0200 Subject: [PATCH 08/28] Harmonize comment style + empty dir fix Switch comment style to the one used in the rest of the file. Also make sure that if there is a package directory it is checked that it has files in it, I occasionally have the scenario when that directory is there, but its empty, then the package re-unpack fails and so does the installation of the package. --- .../transport/modtransportpackage.class.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 868deb6f55b..39be2f95960 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -212,9 +212,9 @@ public function getTransport($state = -1) { } if ($transferred) { if ($state < 0) { - /* if directory is missing but zip exists, and DB state value is incorrect, fix here */ + /* if directory is missing or empty but zip exists, and DB state value is incorrect, fix here */ $targetDir = basename($sourceFile, '.transport.zip'); - $state = is_dir($packageDir.$targetDir) ? $this->get('state') : xPDOTransport::STATE_PACKED; + $state = (is_dir($packageDir.$targetDir) && count(glob($packageDir.$targetDir.'/*')) !== 0) ? $this->get('state') : xPDOTransport::STATE_PACKED; } /* retrieve the package */ $this->package = xPDOTransport :: retrieve($this->xpdo, $packageDir . $sourceFile, $packageDir, $state); @@ -363,21 +363,21 @@ public function transferPackage($sourceFile, $targetDir) { if (!is_array($this->xpdo->version)) { $this->xpdo->getVersionData(); } $productVersion = $this->xpdo->version['code_name'].'-'.$this->xpdo->version['full_version']; - // make sure the package is downloaded, if not attempt re-download + /* make sure the package is downloaded, if not attempt re-download */ if (!file_exists($targetDir . $sourceFile)) { - // not very pretty way of getting the download URL, but otherwise we have to make a - // remote request to $this->Provider->info($this->signature) to get the URL. - // loop trough metadata arrays until location is found + /* not very pretty way of getting the download URL, but otherwise we have to make a + * remote request to $this->Provider->info($this->signature) to get the URL. + * loop trough metadata arrays until location is found */ $locationarr = array_filter($this->get('metadata'), function($item) { if ($item['name'] === 'location') { return $item; } }); - // determine the index at which the location metadata was found + /* determine the index at which the location metadata was found */ $locationkey = array_keys($locationarr)[0]; - // get the location metadata + /* get the location metadata */ $location = $this->get('metadata')[$locationkey]; - // assign remote download URL + /* assign remote download URL */ $source = $location['text']; } else { $source = $this->get('service_url') . $sourceFile.(strpos($sourceFile,'?') !== false ? '&' : '?').'revolution_version='.$productVersion; From 6a2f944970b4fde1ddc1f1433b46454ba7c34401 Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Sun, 29 Jul 2018 20:45:39 +0200 Subject: [PATCH 09/28] Separate code into getMetadata() method Following the advice from @OptimusCrime, I added a more generic getMetadata() method to the class, like this any other metadata can also be accessed more easily in case its ever needed. --- .../transport/modtransportpackage.class.php | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 39be2f95960..10d4e346a1d 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -236,6 +236,25 @@ public function getTransport($state = -1) { return $this->package; } + /** + * Get metadata for a package in a more usable format + * + * @return array an array of metadata accessible by metadata name field + */ + public function getMetadata() { + + $metadata = array_reduce($this->get('metadata'), function ($result, $item) { + $key = $item['name']; + unset($item['name']); + + $result[$key] = $item; + + return $result; + }, array()); + + return $metadata; + } + /** * Removes and uninstalls the package. * @@ -365,20 +384,13 @@ public function transferPackage($sourceFile, $targetDir) { /* make sure the package is downloaded, if not attempt re-download */ if (!file_exists($targetDir . $sourceFile)) { - /* not very pretty way of getting the download URL, but otherwise we have to make a - * remote request to $this->Provider->info($this->signature) to get the URL. - * loop trough metadata arrays until location is found */ - $locationarr = array_filter($this->get('metadata'), function($item) { - if ($item['name'] === 'location') { - return $item; - } - }); - /* determine the index at which the location metadata was found */ - $locationkey = array_keys($locationarr)[0]; - /* get the location metadata */ - $location = $this->get('metadata')[$locationkey]; - /* assign remote download URL */ - $source = $location['text']; + /* get the package metadata */ + $metadata = $this->getMetadata(); + + if (!empty($metadata) && $metadata['location']) { + /* assign remote download URL */ + $source = $metadata['location']['text']; + } } else { $source = $this->get('service_url') . $sourceFile.(strpos($sourceFile,'?') !== false ? '&' : '?').'revolution_version='.$productVersion; } From 69c081b6039b8926c3239214a42bad9b9f42c8de Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Sun, 29 Jul 2018 21:42:45 +0200 Subject: [PATCH 10/28] Fix issue with update mode When updating a package it would also trigger the re-download logic (as package file not present yet) and re-assign the `$sourceFile` variable passed to `transportPackage()` which would cause it to fail. Therefore it is now checked if the `$sourceFile` variable contains a URL already, if that's the case, the re-download logic is skipped. When triggering a re-install action from the manager the `$sourceFile` variable is set to the package string and not a URL like when an update is triggered. --- .../modx/transport/modtransportpackage.class.php | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 10d4e346a1d..2c15b7377f0 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -199,9 +199,9 @@ public function getTransport($state = -1) { $packageDir = $workspace->get('path') . 'packages/'; $sourceFile = $this->get('source'); if ($sourceFile) { - $transferred= file_exists($packageDir . $sourceFile); + $transferred = file_exists($packageDir . $sourceFile); if (!$transferred) { /* if no transport zip, attempt to get it */ - if (!$transferred= $this->transferPackage($sourceFile, $packageDir)) { + if (!$transferred = $this->transferPackage($sourceFile, $packageDir)) { $this->xpdo->log(xPDO::LOG_LEVEL_ERROR,$this->xpdo->lexicon('package_err_transfer',array( 'sourceFile' => $sourceFile, 'packageDir' => $packageDir, @@ -242,7 +242,6 @@ public function getTransport($state = -1) { * @return array an array of metadata accessible by metadata name field */ public function getMetadata() { - $metadata = array_reduce($this->get('metadata'), function ($result, $item) { $key = $item['name']; unset($item['name']); @@ -376,14 +375,17 @@ public function uninstall(array $options = array()) { * @return boolean True if successful. */ public function transferPackage($sourceFile, $targetDir) { - $transferred= false; - $content= ''; + $transferred = false; + $content = ''; if (is_dir($targetDir) && is_writable($targetDir)) { if (!is_array($this->xpdo->version)) { $this->xpdo->getVersionData(); } - $productVersion = $this->xpdo->version['code_name'].'-'.$this->xpdo->version['full_version']; + $productVersion = join('-', array( + $this->xpdo->version['code_name'], + $this->xpdo->version['full_version'] + )); /* make sure the package is downloaded, if not attempt re-download */ - if (!file_exists($targetDir . $sourceFile)) { + if (strpos($sourceFile, '//') === false && !file_exists($targetDir . $sourceFile)) { /* get the package metadata */ $metadata = $this->getMetadata(); From 290cee500b6dbde3ae2aa05e439db26635ed15ad Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Tue, 31 Jul 2018 01:00:14 +0200 Subject: [PATCH 11/28] Optimization & Dealing with various edge cases --- .../transport/modtransportpackage.class.php | 52 ++++++++++++++----- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 2c15b7377f0..02fe936c938 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -238,20 +238,36 @@ public function getTransport($state = -1) { /** * Get metadata for a package in a more usable format - * - * @return array an array of metadata accessible by metadata name field + * + * @access public + * @param array $metadata optional input array to be processed, defaults to package metadata + * @param string $keyfield the inner array element pointing to the value for key substitution + * @return array an array of metadata accessible by $keyfield */ - public function getMetadata() { - $metadata = array_reduce($this->get('metadata'), function ($result, $item) { - $key = $item['name']; - unset($item['name']); - - $result[$key] = $item; - - return $result; - }, array()); + public function getMetadata($metadata = array(), $keyfield = 'name') { + if (empty($metadata)) { + $metadata = $this->get('metadata'); + } - return $metadata; + if (is_array($metadata[0])) { + return array_reduce($metadata, function ($result, $item) use ($keyfield) { + $key = $item[$keyfield]; + unset($item[$keyfield]); + + /* recurisvely handle nested arrays, attributes and children */ + foreach ($item as $k => $v) { + if (is_array($v) && !empty($v)) { + $item[$k] = $this->getMetadata($v); + } + } + + $result[$key] = $item; + + return $result; + }, array()); + } else { + return $metadata; + } } /** @@ -389,9 +405,17 @@ public function transferPackage($sourceFile, $targetDir) { /* get the package metadata */ $metadata = $this->getMetadata(); - if (!empty($metadata) && $metadata['location']) { + if (!empty($metadata) && ($metadata['location'] || $metadata['file'])) { /* assign remote download URL */ - $source = $metadata['location']['text']; + if ($metadata['location']) { + if (is_array($metadata['location'])) { + $source = $metadata['location']['text']; + } else { + $source = $metadata['location']; + } + } else { + $source = $metadata['file']['children']['location']['text']; + } } } else { $source = $this->get('service_url') . $sourceFile.(strpos($sourceFile,'?') !== false ? '&' : '?').'revolution_version='.$productVersion; From 4195e57eb6aacd55b3998dc737e5f1fc15de35c0 Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Fri, 3 Aug 2018 17:20:56 +0200 Subject: [PATCH 12/28] Change getMetadata() to return directly also changed referrer URL scheme --- .../transport/modtransportpackage.class.php | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 02fe936c938..8cf545ee2c4 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -249,25 +249,25 @@ public function getMetadata($metadata = array(), $keyfield = 'name') { $metadata = $this->get('metadata'); } - if (is_array($metadata[0])) { - return array_reduce($metadata, function ($result, $item) use ($keyfield) { - $key = $item[$keyfield]; - unset($item[$keyfield]); - - /* recurisvely handle nested arrays, attributes and children */ - foreach ($item as $k => $v) { - if (is_array($v) && !empty($v)) { - $item[$k] = $this->getMetadata($v); - } + if (!is_array($metadata[0])) { + return $metadata; + } + + return array_reduce($metadata, function ($result, $item) use ($keyfield) { + $key = $item[$keyfield]; + unset($item[$keyfield]); + + /* recurisvely handle nested arrays, attributes and children */ + foreach ($item as $k => $v) { + if (is_array($v) && !empty($v)) { + $item[$k] = $this->getMetadata($v); } + } - $result[$key] = $item; + $result[$key] = $item; - return $result; - }, array()); - } else { - return $metadata; - } + return $result; + }, array()); } /** @@ -743,7 +743,7 @@ protected function _getByFsockopen($url) { "Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7", "Keep-Alive: 300", "Connection: keep-alive", - "Referer: http://$host", + "Referer: " . MODX_URL_SCHEME . $host, "\r\n" )); fwrite($fp, $out); From d0fd63169ef7581e93729e0fa88ffd9cd9898805 Mon Sep 17 00:00:00 2001 From: Bochkarev Ivan Date: Sun, 17 Feb 2019 11:50:54 +0600 Subject: [PATCH 13/28] Otherwise a xx4 or xx5 would be detected too. --- core/model/modx/transport/modtransportpackage.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 8cf545ee2c4..f1688fbe686 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -750,7 +750,7 @@ protected function _getByFsockopen($url) { $status = explode(' ', fgets($fp, 13))[1]; - if (strpos($status, '4') !== false || strpos($status, '5') !== false) { + if (strpos($status, '4') === 0 || strpos($status, '5') === 0) { $response = ''; } else { while ($line = fread($fp, 4096)) { From ade661c4d463c21c2294fb1b33d4f3920107cb2b Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Sat, 28 Jul 2018 03:21:22 +0200 Subject: [PATCH 14/28] [Package Manager] Re-Download missing packages Was surprised that the package manager is not able to re-download packages in case the zip files / extracted folders are missing for an installed package (for whatever reason, in my particular case did I want to save space for backups and did not include the packages folder...). When trying to re-install a package with missing files it would proceed to the installation (of course without metadata displaying), then when hitting "Continue" show the modal and say could not download the package etc., but it never actually tried to re-download... This PR changes that so if the zip file is not present, it would actually try to re-download it. There is an edge case where the package is not available anymore on the provider (for example I had an old version of the "console" extra that was not available anymore) and the provider would return an error code (e.g. 404 in that case), but the old code would actually "package" the response HTML or JSON code into a .zip file and then of course the installation would fail with "could not unpack package"...This PR also solves that issue by handling error codes from the provider. Although not used very often, I did rewrite large parts of the _getByFsockopen() method as it needed some love. --- .../transport/modtransportpackage.class.php | 71 +++++++++++++------ 1 file changed, 49 insertions(+), 22 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 69afd5a2adc..12e324a40f5 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -366,8 +366,18 @@ public function transferPackage($sourceFile, $targetDir) { if (is_dir($targetDir) && is_writable($targetDir)) { if (!is_array($this->xpdo->version)) { $this->xpdo->getVersionData(); } $productVersion = $this->xpdo->version['code_name'].'-'.$this->xpdo->version['full_version']; - - $source= $this->get('service_url') . $sourceFile.(strpos($sourceFile,'?') !== false ? '&' : '?').'revolution_version='.$productVersion; + + // make sure the package is downloaded, if not attempt re-download + if (!file_exists($targetDir . $sourceFile)) { + // not very pretty way of getting the download URL, otherwise we have to make a + // remote request to $this->Provider->info($this->signature) to get the URL + $location = $this->get('metadata')[array_keys(array_filter($this->get('metadata'), function($item) { if ($item['name'] === 'location') { return $item; } }))[0]]; + + // assign remote download URL + $source = $location['text']; + } else { + $source = $this->get('service_url') . $sourceFile.(strpos($sourceFile,'?') !== false ? '&' : '?').'revolution_version='.$productVersion; + } /* see if user has allow_url_fopen on and is not behind a proxy */ $proxyHost = $this->xpdo->getOption('proxy_host',null,''); @@ -421,6 +431,11 @@ public function transferPackage($sourceFile, $targetDir) { } } $content = curl_exec($ch); + $status = curl_getinfo($ch, CURLINFO_HTTP_CODE); + + if ($status >= 400) { + $content = ''; + } curl_close($ch); } @@ -436,7 +451,7 @@ public function transferPackage($sourceFile, $targetDir) { $transferred= $cacheManager->writeFile($target, $content); } } else { - $this->xpdo->log(xPDO::LOG_LEVEL_ERROR,'MODX could not download the file. You must enable allow_url_fopen, cURL or fsockopen to use remote transport packaging.'); + $this->xpdo->log(xPDO::LOG_LEVEL_ERROR, 'MODX could not download the file. ' . (isset($status) ? 'Server responded with status code ' . $status . '.' : 'You must enable allow_url_fopen, cURL or fsockopen to use remote transport packaging.')); } } else { $this->xpdo->log(xPDO::LOG_LEVEL_ERROR,$this->xpdo->lexicon('package_err_target_write',array( @@ -666,7 +681,9 @@ protected function _getByFsockopen($url) { $purl = parse_url($url); $host = $purl['host']; $path = !empty($purl['path']) ? $purl['path'] : '/'; - if (!empty($purl['query'])) { $path .= '?'.$purl['query']; } + if (!empty($purl['query'])) { + $path .= '?' . $purl['query']; + } $port = !empty($purl['port']) ? $purl['port'] : '80'; $timeout = 10; @@ -674,25 +691,35 @@ protected function _getByFsockopen($url) { $fp = @fsockopen($host,$port,$errno,$errstr,$timeout); if( !$fp ) { - $this->xpdo->log(xPDO::LOG_LEVEL_ERROR,'Could not retrieve from '.$url); + $this->xpdo->log(xPDO::LOG_LEVEL_ERROR, 'Could not retrieve from ' . $url); } else { - fwrite($fp, "GET $path ".$_SERVER['SERVER_PROTOCOL']."\r\n" . - "Host: $host\r\n" . - "User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3\r\n" . - "Accept: */*\r\n" . - "Accept-Language: en-us,en;q=0.5\r\n" . - "Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7\r\n" . - "Keep-Alive: 300\r\n" . - "Connection: keep-alive\r\n" . - "Referer: http://$host\r\n\r\n"); - - while ($line = fread($fp, 4096)) { - $response .= $line; - } - fclose($fp); - - $pos = strpos($response, "\r\n\r\n"); - $response = substr($response, $pos + 4); + $out = implode("\r\n", array( + "GET $path " . $_SERVER['SERVER_PROTOCOL'], + "Host: $host", + "User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3", + "Accept: */*", + "Accept-Language: en-us,en;q=0.5", + "Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7", + "Keep-Alive: 300", + "Connection: keep-alive", + "Referer: http://$host", + "\r\n" + )); + fwrite($fp, $out); + + $status = explode(' ', fgets($fp, 13))[1]; + + if (strpos($status, '4') !== false || strpos($status, '5') !== false) { + $response = ''; + } else { + while ($line = fread($fp, 4096)) { + $response .= $line; + } + $pos = strpos($response, "\r\n\r\n"); + $response = substr($response, $pos + 4); + } + + fclose($fp); } return $response; } From 160b7d63465aaff6f6208d23fe04c69b04b11c17 Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Sun, 29 Jul 2018 02:29:32 +0200 Subject: [PATCH 15/28] Make monster oneliner multiline --- core/model/modx/transport/modtransportpackage.class.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 12e324a40f5..4fa1ea5509c 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -371,8 +371,11 @@ public function transferPackage($sourceFile, $targetDir) { if (!file_exists($targetDir . $sourceFile)) { // not very pretty way of getting the download URL, otherwise we have to make a // remote request to $this->Provider->info($this->signature) to get the URL - $location = $this->get('metadata')[array_keys(array_filter($this->get('metadata'), function($item) { if ($item['name'] === 'location') { return $item; } }))[0]]; - + $location = $this->get('metadata')[array_keys(array_filter($this->get('metadata'), function($item) { + if ($item['name'] === 'location') { + return $item; + } + }))[0]]; // assign remote download URL $source = $location['text']; } else { From 5bbb2b1c1e7ea9a56c02136db7d6ad98755a914a Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Sun, 29 Jul 2018 02:48:51 +0200 Subject: [PATCH 16/28] Split up oneliner into variables... Hopefully slightly more readable, also added comments. --- .../modx/transport/modtransportpackage.class.php | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 4fa1ea5509c..73bb99bde1e 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -369,13 +369,18 @@ public function transferPackage($sourceFile, $targetDir) { // make sure the package is downloaded, if not attempt re-download if (!file_exists($targetDir . $sourceFile)) { - // not very pretty way of getting the download URL, otherwise we have to make a - // remote request to $this->Provider->info($this->signature) to get the URL - $location = $this->get('metadata')[array_keys(array_filter($this->get('metadata'), function($item) { + // not very pretty way of getting the download URL, but otherwise we have to make a + // remote request to $this->Provider->info($this->signature) to get the URL. + // loop trough metadata arrays until location is found + $locationarr = array_filter($this->get('metadata'), function($item) { if ($item['name'] === 'location') { return $item; } - }))[0]]; + }); + // determine the index at which the location metadata was found + $locationkey = array_keys($locationarr)[0]; + // get the location metadata + $location = $this->get('metadata')[$locationkey]; // assign remote download URL $source = $location['text']; } else { From 8257b2d6b2ca0de429d8293ea891c538294b9354 Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Sun, 29 Jul 2018 04:53:55 +0200 Subject: [PATCH 17/28] Harmonize comment style + empty dir fix Switch comment style to the one used in the rest of the file. Also make sure that if there is a package directory it is checked that it has files in it, I occasionally have the scenario when that directory is there, but its empty, then the package re-unpack fails and so does the installation of the package. --- .../transport/modtransportpackage.class.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 73bb99bde1e..cfb61a4a3f9 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -216,9 +216,9 @@ public function getTransport($state = -1) { } if ($transferred) { if ($state < 0) { - /* if directory is missing but zip exists, and DB state value is incorrect, fix here */ + /* if directory is missing or empty but zip exists, and DB state value is incorrect, fix here */ $targetDir = basename($sourceFile, '.transport.zip'); - $state = is_dir($packageDir.$targetDir) ? $this->get('state') : xPDOTransport::STATE_PACKED; + $state = (is_dir($packageDir.$targetDir) && count(glob($packageDir.$targetDir.'/*')) !== 0) ? $this->get('state') : xPDOTransport::STATE_PACKED; } /* retrieve the package */ $this->package = xPDOTransport :: retrieve($this->xpdo, $packageDir . $sourceFile, $packageDir, $state); @@ -367,21 +367,21 @@ public function transferPackage($sourceFile, $targetDir) { if (!is_array($this->xpdo->version)) { $this->xpdo->getVersionData(); } $productVersion = $this->xpdo->version['code_name'].'-'.$this->xpdo->version['full_version']; - // make sure the package is downloaded, if not attempt re-download + /* make sure the package is downloaded, if not attempt re-download */ if (!file_exists($targetDir . $sourceFile)) { - // not very pretty way of getting the download URL, but otherwise we have to make a - // remote request to $this->Provider->info($this->signature) to get the URL. - // loop trough metadata arrays until location is found + /* not very pretty way of getting the download URL, but otherwise we have to make a + * remote request to $this->Provider->info($this->signature) to get the URL. + * loop trough metadata arrays until location is found */ $locationarr = array_filter($this->get('metadata'), function($item) { if ($item['name'] === 'location') { return $item; } }); - // determine the index at which the location metadata was found + /* determine the index at which the location metadata was found */ $locationkey = array_keys($locationarr)[0]; - // get the location metadata + /* get the location metadata */ $location = $this->get('metadata')[$locationkey]; - // assign remote download URL + /* assign remote download URL */ $source = $location['text']; } else { $source = $this->get('service_url') . $sourceFile.(strpos($sourceFile,'?') !== false ? '&' : '?').'revolution_version='.$productVersion; From 35eeb5f6fa0ab2a2f6d187c1e60fa1939cd6daca Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Sun, 29 Jul 2018 20:45:39 +0200 Subject: [PATCH 18/28] Separate code into getMetadata() method Following the advice from @OptimusCrime, I added a more generic getMetadata() method to the class, like this any other metadata can also be accessed more easily in case its ever needed. --- .../transport/modtransportpackage.class.php | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index cfb61a4a3f9..4425209d5e6 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -240,6 +240,25 @@ public function getTransport($state = -1) { return $this->package; } + /** + * Get metadata for a package in a more usable format + * + * @return array an array of metadata accessible by metadata name field + */ + public function getMetadata() { + + $metadata = array_reduce($this->get('metadata'), function ($result, $item) { + $key = $item['name']; + unset($item['name']); + + $result[$key] = $item; + + return $result; + }, array()); + + return $metadata; + } + /** * Removes and uninstalls the package. * @@ -369,20 +388,13 @@ public function transferPackage($sourceFile, $targetDir) { /* make sure the package is downloaded, if not attempt re-download */ if (!file_exists($targetDir . $sourceFile)) { - /* not very pretty way of getting the download URL, but otherwise we have to make a - * remote request to $this->Provider->info($this->signature) to get the URL. - * loop trough metadata arrays until location is found */ - $locationarr = array_filter($this->get('metadata'), function($item) { - if ($item['name'] === 'location') { - return $item; - } - }); - /* determine the index at which the location metadata was found */ - $locationkey = array_keys($locationarr)[0]; - /* get the location metadata */ - $location = $this->get('metadata')[$locationkey]; - /* assign remote download URL */ - $source = $location['text']; + /* get the package metadata */ + $metadata = $this->getMetadata(); + + if (!empty($metadata) && $metadata['location']) { + /* assign remote download URL */ + $source = $metadata['location']['text']; + } } else { $source = $this->get('service_url') . $sourceFile.(strpos($sourceFile,'?') !== false ? '&' : '?').'revolution_version='.$productVersion; } From a2647cc2b4b40d4accc3458662c69bc745c66225 Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Sun, 29 Jul 2018 21:42:45 +0200 Subject: [PATCH 19/28] Fix issue with update mode When updating a package it would also trigger the re-download logic (as package file not present yet) and re-assign the `$sourceFile` variable passed to `transportPackage()` which would cause it to fail. Therefore it is now checked if the `$sourceFile` variable contains a URL already, if that's the case, the re-download logic is skipped. When triggering a re-install action from the manager the `$sourceFile` variable is set to the package string and not a URL like when an update is triggered. --- .../modx/transport/modtransportpackage.class.php | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 4425209d5e6..184ab834237 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -203,9 +203,9 @@ public function getTransport($state = -1) { $packageDir = $workspace->get('path') . 'packages/'; $sourceFile = $this->get('source'); if ($sourceFile) { - $transferred= file_exists($packageDir . $sourceFile); + $transferred = file_exists($packageDir . $sourceFile); if (!$transferred) { /* if no transport zip, attempt to get it */ - if (!$transferred= $this->transferPackage($sourceFile, $packageDir)) { + if (!$transferred = $this->transferPackage($sourceFile, $packageDir)) { $this->xpdo->log(xPDO::LOG_LEVEL_ERROR,$this->xpdo->lexicon('package_err_transfer',array( 'sourceFile' => $sourceFile, 'packageDir' => $packageDir, @@ -246,7 +246,6 @@ public function getTransport($state = -1) { * @return array an array of metadata accessible by metadata name field */ public function getMetadata() { - $metadata = array_reduce($this->get('metadata'), function ($result, $item) { $key = $item['name']; unset($item['name']); @@ -380,14 +379,17 @@ public function uninstall(array $options = array()) { * @return boolean True if successful. */ public function transferPackage($sourceFile, $targetDir) { - $transferred= false; - $content= ''; + $transferred = false; + $content = ''; if (is_dir($targetDir) && is_writable($targetDir)) { if (!is_array($this->xpdo->version)) { $this->xpdo->getVersionData(); } - $productVersion = $this->xpdo->version['code_name'].'-'.$this->xpdo->version['full_version']; + $productVersion = join('-', array( + $this->xpdo->version['code_name'], + $this->xpdo->version['full_version'] + )); /* make sure the package is downloaded, if not attempt re-download */ - if (!file_exists($targetDir . $sourceFile)) { + if (strpos($sourceFile, '//') === false && !file_exists($targetDir . $sourceFile)) { /* get the package metadata */ $metadata = $this->getMetadata(); From 317c82913d8b89380a02df3c5b42b46c08f38af4 Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Tue, 31 Jul 2018 01:00:14 +0200 Subject: [PATCH 20/28] Optimization & Dealing with various edge cases --- .../transport/modtransportpackage.class.php | 52 ++++++++++++++----- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 184ab834237..175af6ca34f 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -242,20 +242,36 @@ public function getTransport($state = -1) { /** * Get metadata for a package in a more usable format - * - * @return array an array of metadata accessible by metadata name field + * + * @access public + * @param array $metadata optional input array to be processed, defaults to package metadata + * @param string $keyfield the inner array element pointing to the value for key substitution + * @return array an array of metadata accessible by $keyfield */ - public function getMetadata() { - $metadata = array_reduce($this->get('metadata'), function ($result, $item) { - $key = $item['name']; - unset($item['name']); - - $result[$key] = $item; - - return $result; - }, array()); + public function getMetadata($metadata = array(), $keyfield = 'name') { + if (empty($metadata)) { + $metadata = $this->get('metadata'); + } - return $metadata; + if (is_array($metadata[0])) { + return array_reduce($metadata, function ($result, $item) use ($keyfield) { + $key = $item[$keyfield]; + unset($item[$keyfield]); + + /* recurisvely handle nested arrays, attributes and children */ + foreach ($item as $k => $v) { + if (is_array($v) && !empty($v)) { + $item[$k] = $this->getMetadata($v); + } + } + + $result[$key] = $item; + + return $result; + }, array()); + } else { + return $metadata; + } } /** @@ -393,9 +409,17 @@ public function transferPackage($sourceFile, $targetDir) { /* get the package metadata */ $metadata = $this->getMetadata(); - if (!empty($metadata) && $metadata['location']) { + if (!empty($metadata) && ($metadata['location'] || $metadata['file'])) { /* assign remote download URL */ - $source = $metadata['location']['text']; + if ($metadata['location']) { + if (is_array($metadata['location'])) { + $source = $metadata['location']['text']; + } else { + $source = $metadata['location']; + } + } else { + $source = $metadata['file']['children']['location']['text']; + } } } else { $source = $this->get('service_url') . $sourceFile.(strpos($sourceFile,'?') !== false ? '&' : '?').'revolution_version='.$productVersion; From 1a00781db8c2fb0850fd0b30469aa4b03926d8d4 Mon Sep 17 00:00:00 2001 From: Lukas Zahnd Date: Fri, 3 Aug 2018 17:20:56 +0200 Subject: [PATCH 21/28] Change getMetadata() to return directly also changed referrer URL scheme --- .../transport/modtransportpackage.class.php | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 175af6ca34f..1bf8bcd88d5 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -253,25 +253,25 @@ public function getMetadata($metadata = array(), $keyfield = 'name') { $metadata = $this->get('metadata'); } - if (is_array($metadata[0])) { - return array_reduce($metadata, function ($result, $item) use ($keyfield) { - $key = $item[$keyfield]; - unset($item[$keyfield]); - - /* recurisvely handle nested arrays, attributes and children */ - foreach ($item as $k => $v) { - if (is_array($v) && !empty($v)) { - $item[$k] = $this->getMetadata($v); - } + if (!is_array($metadata[0])) { + return $metadata; + } + + return array_reduce($metadata, function ($result, $item) use ($keyfield) { + $key = $item[$keyfield]; + unset($item[$keyfield]); + + /* recurisvely handle nested arrays, attributes and children */ + foreach ($item as $k => $v) { + if (is_array($v) && !empty($v)) { + $item[$k] = $this->getMetadata($v); } + } - $result[$key] = $item; + $result[$key] = $item; - return $result; - }, array()); - } else { - return $metadata; - } + return $result; + }, array()); } /** @@ -748,7 +748,7 @@ protected function _getByFsockopen($url) { "Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7", "Keep-Alive: 300", "Connection: keep-alive", - "Referer: http://$host", + "Referer: " . MODX_URL_SCHEME . $host, "\r\n" )); fwrite($fp, $out); From bb8318b922110ae4b4feb6302f3714bf35175f1e Mon Sep 17 00:00:00 2001 From: Bochkarev Ivan Date: Sun, 17 Feb 2019 11:50:54 +0600 Subject: [PATCH 22/28] Otherwise a xx4 or xx5 would be detected too. --- core/model/modx/transport/modtransportpackage.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 1bf8bcd88d5..23a6db45801 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -755,7 +755,7 @@ protected function _getByFsockopen($url) { $status = explode(' ', fgets($fp, 13))[1]; - if (strpos($status, '4') !== false || strpos($status, '5') !== false) { + if (strpos($status, '4') === 0 || strpos($status, '5') === 0) { $response = ''; } else { while ($line = fread($fp, 4096)) { From 017304ead9dd75b0ed621ff49e92030231268b38 Mon Sep 17 00:00:00 2001 From: luk Date: Wed, 22 May 2019 17:00:37 +0200 Subject: [PATCH 23/28] revert unwanted file changes... --- core/docs/changelog.txt | 3 +-- core/model/modx/modphpthumb.class.php | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/core/docs/changelog.txt b/core/docs/changelog.txt index 19928ae1995..8f722344f8f 100644 --- a/core/docs/changelog.txt +++ b/core/docs/changelog.txt @@ -4228,5 +4228,4 @@ MODx 0.9.7-alpha-1 (LastChangedRevision: 3664, LastChangedDate: 2008-04-28 12:43 - [New feature] Fine-grained configuration options for caching pages, database results, or disabling the cache altogether (see system settings starting with `cache.`). Turn the different caching options on/off or set a default time-to-live for those items being cached. - [New feature] Database result-set and xPDO object caching, with support for memcache, native-JSON object caching for high-performance AJAX requests. - [New feature] Configurable session management with default implementation configured for modSessionHandler, an xPDO-based implementation that stores sessions in a database, and allows a great deal of configurability, by site and/or context. -- [New feature] Contexts allows a site to be organized into sub-sites, subdomains, etc, and override any system settings by context. The default contexts are 'web' and 'mgr' to support the legacy ideas of front-end and back-end session contexts. -- Introducing the new MODx core built on top of xPDO; this will incrementally replace the entire existing codebase, but can co-exist until 1.0 release and provides about 90 to 95% legacy compatibility for existing tags and add-ons. \ No newline at end of file +- [New feature] Contexts allows a site to be organized into sub-sites, subdomains, etc, and override any system settings by context. The default contexts are 'web' and 'mgr' to support the legacy ideas of front-end and back-end session contexts. \ No newline at end of file diff --git a/core/model/modx/modphpthumb.class.php b/core/model/modx/modphpthumb.class.php index f932576406e..20ff01f5964 100644 --- a/core/model/modx/modphpthumb.class.php +++ b/core/model/modx/modphpthumb.class.php @@ -346,4 +346,4 @@ function ResolveFilenameToAbsolute($filename) { } return $AbsoluteFilename; } -} +} \ No newline at end of file From f4b6b9d6cd2cf556d6c75b0c1ce3239e9df755af Mon Sep 17 00:00:00 2001 From: luk Date: Wed, 22 May 2019 17:07:25 +0200 Subject: [PATCH 24/28] it's my editor trying to be smart... sorry about that, doing it directly in the github editor... I did a 1:1 copy from the original repo^^ --- core/docs/changelog.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/docs/changelog.txt b/core/docs/changelog.txt index 8f722344f8f..44fdc7ede99 100644 --- a/core/docs/changelog.txt +++ b/core/docs/changelog.txt @@ -4228,4 +4228,5 @@ MODx 0.9.7-alpha-1 (LastChangedRevision: 3664, LastChangedDate: 2008-04-28 12:43 - [New feature] Fine-grained configuration options for caching pages, database results, or disabling the cache altogether (see system settings starting with `cache.`). Turn the different caching options on/off or set a default time-to-live for those items being cached. - [New feature] Database result-set and xPDO object caching, with support for memcache, native-JSON object caching for high-performance AJAX requests. - [New feature] Configurable session management with default implementation configured for modSessionHandler, an xPDO-based implementation that stores sessions in a database, and allows a great deal of configurability, by site and/or context. -- [New feature] Contexts allows a site to be organized into sub-sites, subdomains, etc, and override any system settings by context. The default contexts are 'web' and 'mgr' to support the legacy ideas of front-end and back-end session contexts. \ No newline at end of file +- [New feature] Contexts allows a site to be organized into sub-sites, subdomains, etc, and override any system settings by context. The default contexts are 'web' and 'mgr' to support the legacy ideas of front-end and back-end session contexts. +- Introducing the new MODx core built on top of xPDO; this will incrementally replace the entire existing codebase, but can co-exist until 1.0 release and provides about 90 to 95% legacy compatibility for existing tags and add-ons. From 7dfa00922c6221d6a34e4c513182e6b1628d2b4b Mon Sep 17 00:00:00 2001 From: luk Date: Wed, 22 May 2019 17:11:18 +0200 Subject: [PATCH 25/28] Revert "revert unwanted file changes..." This reverts commit 017304ead9dd75b0ed621ff49e92030231268b38. --- core/docs/changelog.txt | 3 ++- core/model/modx/modphpthumb.class.php | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/core/docs/changelog.txt b/core/docs/changelog.txt index 8f722344f8f..19928ae1995 100644 --- a/core/docs/changelog.txt +++ b/core/docs/changelog.txt @@ -4228,4 +4228,5 @@ MODx 0.9.7-alpha-1 (LastChangedRevision: 3664, LastChangedDate: 2008-04-28 12:43 - [New feature] Fine-grained configuration options for caching pages, database results, or disabling the cache altogether (see system settings starting with `cache.`). Turn the different caching options on/off or set a default time-to-live for those items being cached. - [New feature] Database result-set and xPDO object caching, with support for memcache, native-JSON object caching for high-performance AJAX requests. - [New feature] Configurable session management with default implementation configured for modSessionHandler, an xPDO-based implementation that stores sessions in a database, and allows a great deal of configurability, by site and/or context. -- [New feature] Contexts allows a site to be organized into sub-sites, subdomains, etc, and override any system settings by context. The default contexts are 'web' and 'mgr' to support the legacy ideas of front-end and back-end session contexts. \ No newline at end of file +- [New feature] Contexts allows a site to be organized into sub-sites, subdomains, etc, and override any system settings by context. The default contexts are 'web' and 'mgr' to support the legacy ideas of front-end and back-end session contexts. +- Introducing the new MODx core built on top of xPDO; this will incrementally replace the entire existing codebase, but can co-exist until 1.0 release and provides about 90 to 95% legacy compatibility for existing tags and add-ons. \ No newline at end of file diff --git a/core/model/modx/modphpthumb.class.php b/core/model/modx/modphpthumb.class.php index 20ff01f5964..f932576406e 100644 --- a/core/model/modx/modphpthumb.class.php +++ b/core/model/modx/modphpthumb.class.php @@ -346,4 +346,4 @@ function ResolveFilenameToAbsolute($filename) { } return $AbsoluteFilename; } -} \ No newline at end of file +} From 509acd36b813fda01fc6166aadfb1a1469fada3b Mon Sep 17 00:00:00 2001 From: luk Date: Wed, 22 May 2019 17:27:16 +0200 Subject: [PATCH 26/28] re-add double whitespace at the end (yeah, still the editor)... --- core/model/modx/modphpthumb.class.php | 1 + 1 file changed, 1 insertion(+) diff --git a/core/model/modx/modphpthumb.class.php b/core/model/modx/modphpthumb.class.php index f932576406e..c7f1af2c118 100644 --- a/core/model/modx/modphpthumb.class.php +++ b/core/model/modx/modphpthumb.class.php @@ -347,3 +347,4 @@ function ResolveFilenameToAbsolute($filename) { return $AbsoluteFilename; } } + From 213601985869b0a1ec3136faf4cb494220f6f481 Mon Sep 17 00:00:00 2001 From: luk Date: Wed, 22 May 2019 17:29:18 +0200 Subject: [PATCH 27/28] whitespace again --- core/docs/changelog.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/docs/changelog.txt b/core/docs/changelog.txt index 19928ae1995..44fdc7ede99 100644 --- a/core/docs/changelog.txt +++ b/core/docs/changelog.txt @@ -4229,4 +4229,4 @@ MODx 0.9.7-alpha-1 (LastChangedRevision: 3664, LastChangedDate: 2008-04-28 12:43 - [New feature] Database result-set and xPDO object caching, with support for memcache, native-JSON object caching for high-performance AJAX requests. - [New feature] Configurable session management with default implementation configured for modSessionHandler, an xPDO-based implementation that stores sessions in a database, and allows a great deal of configurability, by site and/or context. - [New feature] Contexts allows a site to be organized into sub-sites, subdomains, etc, and override any system settings by context. The default contexts are 'web' and 'mgr' to support the legacy ideas of front-end and back-end session contexts. -- Introducing the new MODx core built on top of xPDO; this will incrementally replace the entire existing codebase, but can co-exist until 1.0 release and provides about 90 to 95% legacy compatibility for existing tags and add-ons. \ No newline at end of file +- Introducing the new MODx core built on top of xPDO; this will incrementally replace the entire existing codebase, but can co-exist until 1.0 release and provides about 90 to 95% legacy compatibility for existing tags and add-ons. From 4fb2db298ea010b3f8f3803cfc699c771bb88189 Mon Sep 17 00:00:00 2001 From: Thomas Jakobi Date: Sat, 19 Sep 2020 13:53:57 +0200 Subject: [PATCH 28/28] Update core/model/modx/transport/modtransportpackage.class.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Joshua Lückers --- core/model/modx/transport/modtransportpackage.class.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/model/modx/transport/modtransportpackage.class.php b/core/model/modx/transport/modtransportpackage.class.php index 23a6db45801..cd0be76f351 100644 --- a/core/model/modx/transport/modtransportpackage.class.php +++ b/core/model/modx/transport/modtransportpackage.class.php @@ -218,7 +218,8 @@ public function getTransport($state = -1) { if ($state < 0) { /* if directory is missing or empty but zip exists, and DB state value is incorrect, fix here */ $targetDir = basename($sourceFile, '.transport.zip'); - $state = (is_dir($packageDir.$targetDir) && count(glob($packageDir.$targetDir.'/*')) !== 0) ? $this->get('state') : xPDOTransport::STATE_PACKED; + $packageTargetDirIsNotEmpty = (count(glob($packageDir . $targetDir . '/*')) !== 0) + $state = (is_dir($packageDir . $targetDir) && $packageTargetDirIsNotEmpty) ? $this->get('state') : xPDOTransport::STATE_PACKED; } /* retrieve the package */ $this->package = xPDOTransport :: retrieve($this->xpdo, $packageDir . $sourceFile, $packageDir, $state);