From 0180cb936f5742cac0b7900c50cee1e9545a17f5 Mon Sep 17 00:00:00 2001 From: Vladimir Reznichenko Date: Sun, 28 Oct 2018 19:38:52 +0100 Subject: [PATCH 1/4] SCA: simplify some ifs in favour of null coalescing operator --- .../DependencyInjection/SecurityExtension.php | 6 +----- src/Symfony/Component/Console/Helper/Table.php | 6 +----- src/Symfony/Component/Filesystem/Filesystem.php | 11 ++++------- src/Symfony/Component/Intl/Globals/IntlGlobals.php | 6 +----- .../Messenger/Asynchronous/Routing/SenderLocator.php | 6 +----- .../Component/Routing/Generator/UrlGenerator.php | 5 +---- .../Component/Serializer/Encoder/XmlEncoder.php | 6 +----- .../Serializer/Mapping/ClassDiscriminatorMapping.php | 6 +----- .../Component/Translation/Dumper/JsonFileDumper.php | 6 +----- 9 files changed, 12 insertions(+), 46 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 83ef38b69c..4e8eef0fb5 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -283,11 +283,7 @@ class SecurityExtension extends Extension $contextKey = null; // Context serializer listener if (false === $firewall['stateless']) { - $contextKey = $id; - if (isset($firewall['context'])) { - $contextKey = $firewall['context']; - } - + $contextKey = $firewall['context'] ?? $id; $listeners[] = new Reference($this->createContextListener($container, $contextKey)); $sessionStrategyId = 'security.authentication.session_strategy'; } else { diff --git a/src/Symfony/Component/Console/Helper/Table.php b/src/Symfony/Component/Console/Helper/Table.php index 9577548481..ca432e3b26 100644 --- a/src/Symfony/Component/Console/Helper/Table.php +++ b/src/Symfony/Component/Console/Helper/Table.php @@ -180,11 +180,7 @@ class Table */ public function getColumnStyle($columnIndex) { - if (isset($this->columnStyles[$columnIndex])) { - return $this->columnStyles[$columnIndex]; - } - - return $this->getStyle(); + return $this->columnStyles[$columnIndex] ?? $this->getStyle(); } /** diff --git a/src/Symfony/Component/Filesystem/Filesystem.php b/src/Symfony/Component/Filesystem/Filesystem.php index 9e44f2f3d9..186d0661e1 100644 --- a/src/Symfony/Component/Filesystem/Filesystem.php +++ b/src/Symfony/Component/Filesystem/Filesystem.php @@ -54,12 +54,12 @@ class Filesystem if ($doCopy) { // https://bugs.php.net/bug.php?id=64634 - if (false === $source = @fopen($originFile, 'r')) { + if (false === $source = @fopen($originFile, 'rb')) { throw new IOException(sprintf('Failed to copy "%s" to "%s" because source file could not be opened for reading.', $originFile, $targetFile), 0, null, $originFile); } // Stream context created to allow files overwrite when using FTP stream wrapper - disabled by default - if (false === $target = @fopen($targetFile, 'w', null, stream_context_create(array('ftp' => array('overwrite' => true))))) { + if (false === $target = @fopen($targetFile, 'wb', null, stream_context_create(array('ftp' => array('overwrite' => true))))) { throw new IOException(sprintf('Failed to copy "%s" to "%s" because target file could not be opened for writing.', $originFile, $targetFile), 0, null, $originFile); } @@ -557,10 +557,7 @@ class Filesystem } } - $copyOnWindows = false; - if (isset($options['copy_on_windows'])) { - $copyOnWindows = $options['copy_on_windows']; - } + $copyOnWindows = $options['copy_on_windows'] ?? false; if (null === $iterator) { $flags = $copyOnWindows ? \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::FOLLOW_SYMLINKS : \FilesystemIterator::SKIP_DOTS; @@ -650,7 +647,7 @@ class Filesystem // Use fopen instead of file_exists as some streams do not support stat // Use mode 'x+' to atomically check existence and create to avoid a TOCTOU vulnerability - $handle = @fopen($tmpFile, 'x+'); + $handle = @fopen($tmpFile, 'x+b'); // If unsuccessful restart the loop if (false === $handle) { diff --git a/src/Symfony/Component/Intl/Globals/IntlGlobals.php b/src/Symfony/Component/Intl/Globals/IntlGlobals.php index 4e520695ef..86bba305be 100644 --- a/src/Symfony/Component/Intl/Globals/IntlGlobals.php +++ b/src/Symfony/Component/Intl/Globals/IntlGlobals.php @@ -100,11 +100,7 @@ abstract class IntlGlobals */ public static function getErrorName($code) { - if (isset(self::$errorCodes[$code])) { - return self::$errorCodes[$code]; - } - - return '[BOGUS UErrorCode]'; + return self::$errorCodes[$code] ?? '[BOGUS UErrorCode]'; } /** diff --git a/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php b/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php index e85453691b..c16903887b 100644 --- a/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php +++ b/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php @@ -57,10 +57,6 @@ class SenderLocator implements SenderLocatorInterface if ($interfaceMapping = array_intersect_key($mapping, class_implements($message))) { return current($interfaceMapping); } - if (isset($mapping['*'])) { - return $mapping['*']; - } - - return null; + return $mapping['*'] ?? null; } } diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index 38dd13847c..7e13c466cd 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -254,10 +254,7 @@ class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInt }); // extract fragment - $fragment = ''; - if (isset($defaults['_fragment'])) { - $fragment = $defaults['_fragment']; - } + $fragment = $defaults['_fragment'] ?? ''; if (isset($extra['_fragment'])) { $fragment = $extra['_fragment']; diff --git a/src/Symfony/Component/Serializer/Encoder/XmlEncoder.php b/src/Symfony/Component/Serializer/Encoder/XmlEncoder.php index 0a6cc0edbd..a0894cec41 100644 --- a/src/Symfony/Component/Serializer/Encoder/XmlEncoder.php +++ b/src/Symfony/Component/Serializer/Encoder/XmlEncoder.php @@ -326,11 +326,7 @@ class XmlEncoder implements EncoderInterface, DecoderInterface, NormalizationAwa $val = $this->parseXml($subnode, $context); if ('item' === $subnode->nodeName && isset($val['@key'])) { - if (isset($val['#'])) { - $value[$val['@key']] = $val['#']; - } else { - $value[$val['@key']] = $val; - } + $value[$val['@key']] = $val['#'] ?? $val; } else { $value[$subnode->nodeName][] = $val; } diff --git a/src/Symfony/Component/Serializer/Mapping/ClassDiscriminatorMapping.php b/src/Symfony/Component/Serializer/Mapping/ClassDiscriminatorMapping.php index 5d33a001fd..65282e6aa0 100644 --- a/src/Symfony/Component/Serializer/Mapping/ClassDiscriminatorMapping.php +++ b/src/Symfony/Component/Serializer/Mapping/ClassDiscriminatorMapping.php @@ -32,11 +32,7 @@ class ClassDiscriminatorMapping public function getClassForType(string $type): ?string { - if (isset($this->typesMapping[$type])) { - return $this->typesMapping[$type]; - } - - return null; + return $this->typesMapping[$type] ?? null; } /** diff --git a/src/Symfony/Component/Translation/Dumper/JsonFileDumper.php b/src/Symfony/Component/Translation/Dumper/JsonFileDumper.php index 32bdaf5181..a4db4d6ab8 100644 --- a/src/Symfony/Component/Translation/Dumper/JsonFileDumper.php +++ b/src/Symfony/Component/Translation/Dumper/JsonFileDumper.php @@ -25,11 +25,7 @@ class JsonFileDumper extends FileDumper */ public function formatCatalogue(MessageCatalogue $messages, $domain, array $options = array()) { - if (isset($options['json_encoding'])) { - $flags = $options['json_encoding']; - } else { - $flags = JSON_PRETTY_PRINT; - } + $flags = $options['json_encoding'] ?? JSON_PRETTY_PRINT; return json_encode($messages->all($domain), $flags); } From 300b31fa75d641338cd9da6d8fa66fdb11e71f80 Mon Sep 17 00:00:00 2001 From: Vladimir Reznichenko Date: Sun, 28 Oct 2018 19:42:32 +0100 Subject: [PATCH 2/4] SCA: applied requested code style changes --- .../Component/Messenger/Asynchronous/Routing/SenderLocator.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php b/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php index c16903887b..27f6a85201 100644 --- a/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php +++ b/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php @@ -57,6 +57,7 @@ class SenderLocator implements SenderLocatorInterface if ($interfaceMapping = array_intersect_key($mapping, class_implements($message))) { return current($interfaceMapping); } + return $mapping['*'] ?? null; } } From c926b1abd3f5559eed2570d66b1edb42648d8d6e Mon Sep 17 00:00:00 2001 From: "vladimir.reznichenko" Date: Tue, 30 Oct 2018 14:12:48 +0100 Subject: [PATCH 3/4] SCA: reverted code style changes --- .../Component/Messenger/Asynchronous/Routing/SenderLocator.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php b/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php index 27f6a85201..c16903887b 100644 --- a/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php +++ b/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php @@ -57,7 +57,6 @@ class SenderLocator implements SenderLocatorInterface if ($interfaceMapping = array_intersect_key($mapping, class_implements($message))) { return current($interfaceMapping); } - return $mapping['*'] ?? null; } } From 636a87238927d563ed8ccc8cd634e31d09364589 Mon Sep 17 00:00:00 2001 From: "vladimir.reznichenko" Date: Tue, 30 Oct 2018 14:18:25 +0100 Subject: [PATCH 4/4] SCA: reverted code style changes --- src/Symfony/Component/Filesystem/Filesystem.php | 6 +++--- .../Messenger/Asynchronous/Routing/SenderLocator.php | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Filesystem/Filesystem.php b/src/Symfony/Component/Filesystem/Filesystem.php index 186d0661e1..ffd73bf510 100644 --- a/src/Symfony/Component/Filesystem/Filesystem.php +++ b/src/Symfony/Component/Filesystem/Filesystem.php @@ -54,12 +54,12 @@ class Filesystem if ($doCopy) { // https://bugs.php.net/bug.php?id=64634 - if (false === $source = @fopen($originFile, 'rb')) { + if (false === $source = @fopen($originFile, 'r')) { throw new IOException(sprintf('Failed to copy "%s" to "%s" because source file could not be opened for reading.', $originFile, $targetFile), 0, null, $originFile); } // Stream context created to allow files overwrite when using FTP stream wrapper - disabled by default - if (false === $target = @fopen($targetFile, 'wb', null, stream_context_create(array('ftp' => array('overwrite' => true))))) { + if (false === $target = @fopen($targetFile, 'w', null, stream_context_create(array('ftp' => array('overwrite' => true))))) { throw new IOException(sprintf('Failed to copy "%s" to "%s" because target file could not be opened for writing.', $originFile, $targetFile), 0, null, $originFile); } @@ -647,7 +647,7 @@ class Filesystem // Use fopen instead of file_exists as some streams do not support stat // Use mode 'x+' to atomically check existence and create to avoid a TOCTOU vulnerability - $handle = @fopen($tmpFile, 'x+b'); + $handle = @fopen($tmpFile, 'x+'); // If unsuccessful restart the loop if (false === $handle) { diff --git a/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php b/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php index c16903887b..27f6a85201 100644 --- a/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php +++ b/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php @@ -57,6 +57,7 @@ class SenderLocator implements SenderLocatorInterface if ($interfaceMapping = array_intersect_key($mapping, class_implements($message))) { return current($interfaceMapping); } + return $mapping['*'] ?? null; } }