From d143b8bf6e4ee2eeb21c4b0152f4ad1123245d6d Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Tue, 5 Apr 2022 17:19:12 +0200 Subject: [PATCH] Debug dol_uncompress and add phpunit tests --- htdocs/core/class/utils.class.php | 16 +++++++--- htdocs/core/lib/files.lib.php | 35 +++++++++++++++------- test/phpunit/FilesLibTest.php | 50 +++++++++++++++++++++++++++---- 3 files changed, 81 insertions(+), 20 deletions(-) diff --git a/htdocs/core/class/utils.class.php b/htdocs/core/class/utils.class.php index 4a4db439822..f3d46e09f30 100644 --- a/htdocs/core/class/utils.class.php +++ b/htdocs/core/class/utils.class.php @@ -651,12 +651,13 @@ class Utils * Warning: The command line is sanitize so can't contains any redirection char '>'. Use param $redirectionfile if you need it. * @param string $outputfile A path for an output file (used only when method is 2). For example: $conf->admin->dir_temp.'/out.tmp'; * @param int $execmethod 0=Use default method (that is 1 by default), 1=Use the PHP 'exec', 2=Use the 'popen' method - * @param string $redirectionfile If defined, a redirection of output to this files is added. + * @param string $redirectionfile If defined, a redirection of output to this file is added. * @param int $noescapecommand 1=Do not escape command. Warning: Using this parameter need you alreay sanitized the command. if not, it will lead to security vulnerability. * This parameter is provided for backward compatibility with external modules. Always use 0 in core. + * @param string $redirectionfileerr If defined, a redirection of error is added to this file instead of to channel 1. * @return array array('result'=>...,'output'=>...,'error'=>...). result = 0 means OK. */ - public function executeCLI($command, $outputfile, $execmethod = 0, $redirectionfile = null, $noescapecommand = 0) + public function executeCLI($command, $outputfile, $execmethod = 0, $redirectionfile = null, $noescapecommand = 0, $redirectionfileerr = null) { global $conf, $langs; @@ -667,10 +668,17 @@ class Utils if (empty($noescapecommand)) { $command = escapeshellcmd($command); } + if ($redirectionfile) { $command .= " > ".dol_sanitizePathName($redirectionfile); } - $command .= " 2>&1"; + + if ($redirectionfileerr && ($redirectionfileerr != $redirectionfile)) { + // If we ask a redirect of stderr on a given file not already used for stdout + $command .= " 2> ".dol_sanitizePathName($redirectionfileerr); + } else { + $command .= " 2>&1"; + } if (!empty($conf->global->MAIN_EXEC_USE_POPEN)) { $execmethod = $conf->global->MAIN_EXEC_USE_POPEN; @@ -679,7 +687,7 @@ class Utils $execmethod = 1; } //$execmethod=1; - dol_syslog("Utils::executeCLI execmethod=".$execmethod." system:".$command, LOG_DEBUG); + dol_syslog("Utils::executeCLI execmethod=".$execmethod." command=".$command, LOG_DEBUG); $output_arr = array(); if ($execmethod == 1) { diff --git a/htdocs/core/lib/files.lib.php b/htdocs/core/lib/files.lib.php index 3bbef8b1b96..2cf0beb013f 100644 --- a/htdocs/core/lib/files.lib.php +++ b/htdocs/core/lib/files.lib.php @@ -2094,6 +2094,9 @@ function dol_uncompress($inputfile, $outputdir) include_once ODTPHP_PATHTOPCLZIP.'/pclzip.lib.php'; $archive = new PclZip($inputfile); + // We create output dir manually, so it uses the correct permission (When created by the archive->extract, dir is rwx for everybody). + dol_mkdir(dol_sanitizePathName($outputdir)); + // Extract into outputdir, but only files that match the regex '/^((?!\.\.).)*$/' that means "does not include .." $result = $archive->extract(PCLZIP_OPT_PATH, $outputdir, PCLZIP_OPT_BY_PREG, '/^((?!\.\.).)*$/'); @@ -2150,10 +2153,19 @@ function dol_uncompress($inputfile, $outputdir) include_once DOL_DOCUMENT_ROOT."/core/class/utils.class.php"; $utils = new Utils($db); + dol_mkdir(dol_sanitizePathName($outputdir)); + $outputfilename = escapeshellcmd(dol_sanitizePathName($outputdir).'/'.dol_sanitizeFileName($fileinfo["filename"])); + dol_delete_file($outputfilename.'.tmp'); + dol_delete_file($outputfilename.'.err'); + $extension = strtolower(pathinfo($fileinfo["filename"], PATHINFO_EXTENSION)); if ($extension == "tar") { $cmd = 'tar -C '.escapeshellcmd(dol_sanitizePathName($outputdir)).' -xvf '.escapeshellcmd(dol_sanitizePathName($fileinfo["dirname"]).'/'.dol_sanitizeFileName($fileinfo["basename"])); - $resarray = $utils->executeCLI($cmd, $outputdir); + + $resarray = $utils->executeCLI($cmd, $outputfilename.'.tmp', 0, $outputfilename.'.err', 0); + if ($resarray["result"] != 0) { + $resarray["error"] .= file_get_contents($outputfilename.'.err'); + } } else { $program = ""; if ($fileinfo["extension"] == "gz") { @@ -2163,22 +2175,23 @@ function dol_uncompress($inputfile, $outputdir) } elseif ($fileinfo["extension"] == "zst") { $program = 'zstd'; } else { - return array('error'=>'ErrFileExtension'); + return array('error'=>'ErrorBadFileExtension'); } $cmd = $program.' -dc '.escapeshellcmd(dol_sanitizePathName($fileinfo["dirname"]).'/'.dol_sanitizeFileName($fileinfo["basename"])); - $outputfilename = escapeshellcmd(dol_sanitizePathName($outputdir).'/'.dol_sanitizeFileName($fileinfo["filename"])); - $resarray = $utils->executeCLI($cmd, $outputfilename, 0, $outputfilename); - if ($resarray["output"] == 2) { - $resarray["error"] = "ErrFilePermOrFileNotFound"; - } - if ($resarray["output"] == 1) { - $resarray["error"] = "Error"; + $cmd .= ' > '.$outputfilename; + + $resarray = $utils->executeCLI($cmd, $outputfilename.'.tmp', 0, null, 1, $outputfilename.'.err'); + if ($resarray["result"] != 0) { + $errfilecontent = @file_get_contents($outputfilename.'.err'); + if ($errfilecontent) { + $resarray["error"] .= " - ".$errfilecontent; + } } } - return $resarray["output"] != 0 ? $resarray["error"] : array(); + return $resarray["result"] != 0 ? array('error' => $resarray["error"]) : array(); } - return array('error'=>'ErrFileExtension'); + return array('error'=>'ErrorBadFileExtension'); } diff --git a/test/phpunit/FilesLibTest.php b/test/phpunit/FilesLibTest.php index b98237d1b29..48a00c8214d 100644 --- a/test/phpunit/FilesLibTest.php +++ b/test/phpunit/FilesLibTest.php @@ -401,10 +401,14 @@ class FilesLibTest extends PHPUnit\Framework\TestCase $langs=$this->savlangs; $db=$this->savdb; + // Format zip + print "\n"; + print 'testDolCompressUnCompress zip'."\n"; + $format='zip'; $filein=dirname(__FILE__).'/Example_import_company_1.csv'; $fileout=$conf->admin->dir_temp.'/test.'.$format; - $dirout=$conf->admin->dir_temp.'/test'; + $dirout=$conf->admin->dir_temp.'/testdir'.$format; dol_delete_file($fileout); $count=0; @@ -417,18 +421,50 @@ class FilesLibTest extends PHPUnit\Framework\TestCase $conf->logbuffer=array(); $result=dol_compress_file($filein, $fileout, $format, $errorstring); - print __METHOD__." result=".$result."\n"; + print __METHOD__." compress result=".$result."\n"; print join(', ', $conf->logbuffer); $this->assertGreaterThanOrEqual(1, $result, "Pb with dol_compress_file on ".$filein." into ".$fileout." : ".$errorstring); $result=dol_uncompress($fileout, $dirout); - print __METHOD__." result=".join(',', $result)."\n"; + print __METHOD__." uncompress result=".join(',', $result)."\n"; $this->assertEquals(0, count($result), "Pb with dol_uncompress_file of file ".$fileout); + // Format gz + print "\n"; + print 'testDolCompressUnCompress gz'."\n"; + + $format='gz'; + $filein=dirname(__FILE__).'/Example_import_company_1.csv'; + $fileout=$conf->admin->dir_temp.'/test.'.$format; + $dirout=$conf->admin->dir_temp.'/testdir'.$format; + + dol_delete_file($fileout); + $count=0; + dol_delete_dir_recursive($dirout, $count, 1); + + $errorstring = ''; + + dol_mkdir($conf->admin->dir_temp); + $conf->global->MAIN_ENABLE_LOG_TO_HTML=1; $conf->syslog->enabled=1; $_REQUEST['logtohtml']=1; + $conf->logbuffer=array(); + + $result=dol_compress_file($filein, $fileout, $format, $errorstring); + print __METHOD__." compress result=".$result."\n"; + print join(', ', $conf->logbuffer); + $this->assertGreaterThanOrEqual(1, $result, "Pb with dol_compress_file on ".$filein." into ".$fileout." : ".$errorstring); + + $result=dol_uncompress($fileout, $dirout); + print __METHOD__." uncompress result=".join(',', $result)."\n"; + print join(', ', $conf->logbuffer); + $this->assertEquals(0, count($result), "Pb with dol_uncompress_file of file ".$fileout); + + + // Test compression of a directory + // $dirout is $conf->admin->dir_temp.'/testdirgz' $excludefiles = '/(\.back|\.old|\.log|documents[\/\\\]admin[\/\\\]documents[\/\\\])/i'; if (preg_match($excludefiles, 'a/temp/b')) { echo '----- Regex OK -----'."\n"; } - $result=dol_compress_dir($dirout, $conf->admin->dir_temp.'/testdir.zip', 'zip', $excludefiles); - print __METHOD__." result=".$result."\n"; + $result=dol_compress_dir($dirout, $conf->admin->dir_temp.'/testcompressdirzip.zip', 'zip', $excludefiles); + print __METHOD__." dol_compress_dir result=".$result."\n"; print join(', ', $conf->logbuffer); $this->assertGreaterThanOrEqual(1, $result, "Pb with dol_compress_dir of ".$dirout." into ".$conf->admin->dir_temp.'/testdir.zip'); } @@ -466,6 +502,10 @@ class FilesLibTest extends PHPUnit\Framework\TestCase $db=$this->savdb; + if (empty($user->rights->facture)) { + $user->rights->facture = new stdClass(); + } + //$dummyuser=new User($db); //$result=restrictedArea($dummyuser,'societe');