Issues in Concat.php - All Issues - Inspection of "Improved param injection (#822)" - consolidation/robo - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( d3a073...5737c8 )

by Greg

created 2019-01-01 22:06 UTC

src/Task/File/Concat.php (4 issues)

Labels

Severity

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php
namespace Robo\Task\File;

use Iterator;
use Robo\Common\ResourceExistenceChecker;
use Robo\Result;
use Robo\Task\BaseTask;

/**
 * Merges files into one. Used for preparing assets.
 *
 * ``` php
 * <?php
 * $this->taskConcat([
 *      'web/assets/screen.css',
 *      'web/assets/print.css',
 *      'web/assets/theme.css'
 *  ])
 *  ->to('web/assets/style.css')
 *  ->run()
 * ?>
 * ```
 */
class Concat extends BaseTask
{
    use ResourceExistenceChecker;

    /**
     * @var array|Iterator
     */
    protected $files;

    /**
     * @var string
     */
    protected $dst;

    /**
     * Constructor.
     *
     * @param array|Iterator $files
     */
    public function __construct($files)
    {
        $this->files = $files;
    }

    /**
     * set the destination file
     *
     * @param string $dst
     *
     * @return $this
     */
    public function to($dst)
    {
        $this->dst = $dst;

        return $this;
    }

    /**
     * {@inheritdoc}
     */
    public function run()
    {
        if (is_null($this->dst) || "" === $this->dst) {
            return Result::error($this, 'You must specify a destination file with to() method.');
        }

        if (!$this->checkResources($this->files, 'file')) {
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
            return Result::error($this, 'Source files are missing!');
        }

        if (file_exists($this->dst) && !is_writable($this->dst)) {

            return Result::error($this, 'Destination already exists and cannot be overwritten.');
        }

        $dump = '';

        foreach ($this->files as $path) {
            foreach (glob($path) as $file) {
                $dump .= file_get_contents($file) . "\n";
            }
        }

        $this->printTaskInfo('Writing {destination}', ['destination' => $this->dst]);

        $dst = $this->dst . '.part';
        $write_result = file_put_contents($dst, $dump);

        if (false === $write_result) {
            @unlink($dst);
// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
            return Result::error($this, 'File write failed.');
        }
        // Cannot be cross-volume; should always succeed.
        @rename($dst, $this->dst);
// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}

        return Result::success($this);
    }
}


1		<?php
2		namespace Robo\Task\File;
3
4		use Iterator;
5		use Robo\Common\ResourceExistenceChecker;
6		use Robo\Result;
7		use Robo\Task\BaseTask;
8
9		/**
10		* Merges files into one. Used for preparing assets.
11		*
12		* ``` php
13		* <?php
14		* $this->taskConcat([
15		* 'web/assets/screen.css',
16		* 'web/assets/print.css',
17		* 'web/assets/theme.css'
18		* ])
19		* ->to('web/assets/style.css')
20		* ->run()
21		* ?>
22		* ```
23		*/
24		class Concat extends BaseTask
25		{
26		use ResourceExistenceChecker;
27
28		/**
29		* @var array\|Iterator
30		*/
31		protected $files;
32
33		/**
34		* @var string
35		*/
36		protected $dst;
37
38		/**
39		* Constructor.
40		*
41		* @param array\|Iterator $files
42		*/
43		public function __construct($files)
44		{
45		$this->files = $files;
46		}
47
48		/**
49		* set the destination file
50		*
51		* @param string $dst
52		*
53		* @return $this
54		*/
55		public function to($dst)
56		{
57		$this->dst = $dst;
58
59		return $this;
60		}
61
62		/**
63		* {@inheritdoc}
64		*/
65		public function run()
66		{
67		if (is_null($this->dst) \|\| "" === $this->dst) {
68		return Result::error($this, 'You must specify a destination file with to() method.');
69		}
70
71		if (!$this->checkResources($this->files, 'file')) {
		0 ignored issues – show Documentation introduced 2016-10-16 16:10 UTC by Report Bug Copy Issue Report Show Similar Issues like this `$this->files` is of type `array\|object<Iterator>`, but the function expects a `string\|array<integer,string>`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
72		return Result::error($this, 'Source files are missing!');
73		}
74
75	View Code Duplication	if (file_exists($this->dst) && !is_writable($this->dst)) {
		0 ignored issues – show Duplication introduced 2016-09-13 23:40 UTC by Report Bug Copy Issue Report Show Similar Issues like this This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
76		return Result::error($this, 'Destination already exists and cannot be overwritten.');
77		}
78
79		$dump = '';
80
81		foreach ($this->files as $path) {
82		foreach (glob($path) as $file) {
83		$dump .= file_get_contents($file) . "\n";
84		}
85		}
86
87		$this->printTaskInfo('Writing {destination}', ['destination' => $this->dst]);
88
89		$dst = $this->dst . '.part';
90		$write_result = file_put_contents($dst, $dump);
91
92		if (false === $write_result) {
93		@unlink($dst);
		0 ignored issues – show Security Best Practice introduced 2016-09-13 23:40 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended. If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of @mkdir($dir); // Better use if (@mkdir($dir) === false) { throw new \RuntimeException('The directory '.$dir.' could not be created.'); } Loading history...
94		return Result::error($this, 'File write failed.');
95		}
96		// Cannot be cross-volume; should always succeed.
97		@rename($dst, $this->dst);
		0 ignored issues – show Security Best Practice introduced 2016-09-13 23:40 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended. If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of @mkdir($dir); // Better use if (@mkdir($dir) === false) { throw new \RuntimeException('The directory '.$dir.' could not be created.'); } Loading history...
98
99		return Result::success($this);
100		}
101		}
102

consolidation / robo

Push — master ( d3a073...5737c8 )

src/Task/File/Concat.php (4 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like