Skip to content

Commit 2c0a91c

Browse files
buskamuzabuskamuza
authored
Merge pull request #1879 from magento-honey-badgers/MAGETWO-83266-Sniffs
MAGETWO-83266: Prohibit global variables and output buffering with Code Sniffer
2 parents 9c2ce25 + 389bae2 commit 2c0a91c

File tree

8 files changed

+137
-36
lines changed

8 files changed

+137
-36
lines changed

‎app/code/Magento/CatalogImportExport/Model/Import/Product.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2001,12 +2001,10 @@ protected function _getUploader()
20012001
$dirConfig = DirectoryList::getDefaultConfig();
20022002
$dirAddon = $dirConfig[DirectoryList::MEDIA][DirectoryList::PATH];
20032003

2004-
$DS = DIRECTORY_SEPARATOR;
2005-
20062004
if (!empty($this->_parameters[Import::FIELD_NAME_IMG_FILE_DIR])) {
20072005
$tmpPath = $this->_parameters[Import::FIELD_NAME_IMG_FILE_DIR];
20082006
} else {
2009-
$tmpPath = $dirAddon . $DS . $this->_mediaDirectory->getRelativePath('import');
2007+
$tmpPath = $dirAddon . '/' . $this->_mediaDirectory->getRelativePath('import');
20102008
}
20112009

20122010
if (!$this->_fileUploader->setTmpDir($tmpPath)) {
@@ -2015,7 +2013,7 @@ protected function _getUploader()
20152013
);
20162014
}
20172015
$destinationDir = "catalog/product";
2018-
$destinationPath = $dirAddon . $DS . $this->_mediaDirectory->getRelativePath($destinationDir);
2016+
$destinationPath = $dirAddon . '/' . $this->_mediaDirectory->getRelativePath($destinationDir);
20192017

20202018
$this->_mediaDirectory->create($destinationPath);
20212019
if (!$this->_fileUploader->setDestDir($destinationPath)) {

‎app/code/Magento/DownloadableImportExport/Helper/Uploader.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,10 @@ public function getUploader($type, $parameters)
7777
$dirConfig = DirectoryList::getDefaultConfig();
7878
$dirAddon = $dirConfig[DirectoryList::MEDIA][DirectoryList::PATH];
7979

80-
$DS = DIRECTORY_SEPARATOR;
81-
8280
if (!empty($parameters[\Magento\ImportExport\Model\Import::FIELD_NAME_IMG_FILE_DIR])) {
8381
$tmpPath = $parameters[\Magento\ImportExport\Model\Import::FIELD_NAME_IMG_FILE_DIR];
8482
} else {
85-
$tmpPath = $dirAddon . $DS . $this->mediaDirectory->getRelativePath('import');
83+
$tmpPath = $dirAddon . '/' . $this->mediaDirectory->getRelativePath('import');
8684
}
8785

8886
if (!$this->fileUploader->setTmpDir($tmpPath)) {
@@ -91,7 +89,7 @@ public function getUploader($type, $parameters)
9189
);
9290
}
9391
$destinationDir = "downloadable/files/" . $type;
94-
$destinationPath = $dirAddon . $DS . $this->mediaDirectory->getRelativePath($destinationDir);
92+
$destinationPath = $dirAddon . '/' . $this->mediaDirectory->getRelativePath($destinationDir);
9593

9694
$this->mediaDirectory->create($destinationPath);
9795
if (!$this->fileUploader->setDestDir($destinationPath)) {

‎app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ImportPost.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,14 @@ class ImportPost extends \Magento\TaxImportExport\Controller\Adminhtml\Rate
1616
*/
1717
public function execute()
1818
{
19-
if ($this->getRequest()->isPost() && !empty($_FILES['import_rates_file']['tmp_name'])) {
19+
$importRatesFile = $this->getRequest()->getFiles('import_rates_file');
20+
if ($this->getRequest()->isPost() && isset($importRatesFile['tmp_name'])) {
2021
try {
2122
/** @var $importHandler \Magento\TaxImportExport\Model\Rate\CsvImportHandler */
2223
$importHandler = $this->_objectManager->create(
2324
\Magento\TaxImportExport\Model\Rate\CsvImportHandler::class
2425
);
25-
$importHandler->importFromCsvFile($this->getRequest()->getFiles('import_rates_file'));
26+
$importHandler->importFromCsvFile($importRatesFile);
2627

2728
$this->messageManager->addSuccess(__('The tax rate has been imported.'));
2829
} catch (\Magento\Framework\Exception\LocalizedException $e) {

‎dev/tests/static/framework/Magento/Sniffs/Functions/OutputBufferingSniff.php

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Sniffs\Functions;
8+
9+
/**
10+
* Sniff prohibiting usage of output buffering functions.
11+
*/
12+
class OutputBufferingSniff extends \PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff
13+
{
14+
public $forbiddenFunctions = ['ob_start' => null];
15+
16+
/**
17+
* @inheritdoc
18+
*/
19+
protected function addError($phpcsFile, $stackPtr, $function, $pattern = null)
20+
{
21+
$data = [$function];
22+
$error = 'The usage of %s() is forbidden';
23+
$type = 'Found';
24+
25+
if ($this->error === true) {
26+
$phpcsFile->addError($error, $stackPtr, $type, $data);
27+
} else {
28+
$phpcsFile->addWarning($error, $stackPtr, $type, $data);
29+
}
30+
}
31+
}

‎dev/tests/static/framework/Magento/Sniffs/Variables/GlobalVariablesSniff.php

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Sniffs\Variables;
8+
9+
use PHP_CodeSniffer\Sniffs\Sniff;
10+
use PHP_CodeSniffer\Files\File;
11+
12+
/**
13+
* Sniff prohibiting usage of global variables.
14+
*/
15+
class GlobalVariablesSniff implements Sniff
16+
{
17+
/**
18+
* @inheritdoc
19+
*/
20+
public function register()
21+
{
22+
return [T_VARIABLE];
23+
}
24+
25+
/**
26+
* @inheritdoc
27+
*/
28+
public function process(File $phpcsFile, $stackPtr)
29+
{
30+
$tokens = $phpcsFile->getTokens();
31+
if (preg_match('/^\$[_A-Z0-9]+$/', $tokens[$stackPtr]['content'])) {
32+
$phpcsFile->addError(
33+
'Usage of global variables is not allowed: ' . $tokens[$stackPtr]['content'],
34+
$stackPtr,
35+
'ERROR'
36+
);
37+
return;
38+
}
39+
}
40+
}

‎dev/tests/static/framework/Magento/ruleset.xml

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,37 @@
2222
<rule ref="Magento.LiteralNamespaces.LiteralNamespaces">
2323
<exclude-pattern>*/_files/*</exclude-pattern>
2424
</rule>
25+
<rule ref="Magento.Functions.OutputBuffering">
26+
<include-pattern>*/(app/code|vendor|setup/src|lib/internal/Magento)/*</include-pattern>
27+
<exclude-pattern>*/lib/internal/Magento/Framework/Image/Adapter/Gd2.php</exclude-pattern>
28+
<exclude-pattern>*/lib/internal/Magento/Framework/View/Result/Page.php</exclude-pattern>
29+
<exclude-pattern>*/lib/internal/Magento/Framework/View/TemplateEngine/Php.php</exclude-pattern>
30+
<exclude-pattern>*/Test/Unit/*</exclude-pattern>
31+
</rule>
32+
<rule ref="Magento.Variables.GlobalVariables">
33+
<include-pattern>*/(app/code|vendor|setup/src)/*</include-pattern>
34+
<exclude-pattern>*/setup/src/Magento/Setup/Controller/WebConfiguration.php</exclude-pattern>
35+
<exclude-pattern>*/setup/src/Magento/Setup/Mvc/Bootstrap/InitParamListener.php</exclude-pattern>
36+
<exclude-pattern>*/app/code/Magento/Eav/Model/Attribute/Data/File.php</exclude-pattern>
37+
<exclude-pattern>*/app/code/Magento/Config/Model/Config/Reader/Source/Deployed/SettingChecker.php</exclude-pattern>
38+
<exclude-pattern>*/app/code/Magento/Backend/App/Area/FrontNameResolver.php</exclude-pattern>
39+
<exclude-pattern>*/app/code/Magento/Indexer/Console/Command/AbstractIndexerCommand.php</exclude-pattern>
40+
<exclude-pattern>*/app/code/Magento/Catalog/Model/Product/Option/Type/File/ValidatorFile.php</exclude-pattern>
41+
<exclude-pattern>*/app/code/Magento/Cron/Console/Command/CronCommand.php</exclude-pattern>
42+
<exclude-pattern>*/app/code/Magento/ImportExport/Controller/Adminhtml/Import/Validate.php</exclude-pattern>
43+
<exclude-pattern>*/app/code/Magento/OfflineShipping/Model/ResourceModel/Carrier/Tablerate.php</exclude-pattern>
44+
<exclude-pattern>*/app/code/Magento/Store/Model/Store.php</exclude-pattern>
45+
<exclude-pattern>*/app/code/Magento/Config/Model/Config/Processor/EnvironmentPlaceholder.php</exclude-pattern>
46+
<exclude-pattern>*/app/code/Magento/Config/Model/Config/Backend/Email/Logo.php</exclude-pattern>
47+
<exclude-pattern>*/app/code/Magento/Config/Model/Config/Backend/File/RequestData.php</exclude-pattern>
48+
<exclude-pattern>*/app/code/Magento/Config/App/Config/Source/EnvironmentConfigSource.php</exclude-pattern>
49+
<exclude-pattern>*/app/code/Magento/Theme/Controller/Adminhtml/Design/Config/FileUploader/Save.php</exclude-pattern>
50+
<exclude-pattern>*/app/code/Magento/Customer/Model/Metadata/Form/File.php</exclude-pattern>
51+
<exclude-pattern>*/app/code/Magento/Customer/Model/FileUploader.php</exclude-pattern>
52+
<exclude-pattern>*/app/code/Magento/Customer/Controller/Adminhtml/File/Address/Upload.php</exclude-pattern>
53+
<exclude-pattern>*/app/code/Magento/Customer/Controller/Adminhtml/File/Customer/Upload.php</exclude-pattern>
54+
<exclude-pattern>*/Test/Unit/*</exclude-pattern>
55+
</rule>
2556

2657
<rule ref="Generic.PHP.CharacterBeforePHPOpeningTag"/>
2758
<rule ref="Generic.Functions.CallTimePassByReference"/>

‎setup/src/Magento/Setup/Module.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,10 @@ public function onBootstrap(EventInterface $e)
5252
$headers->addHeaderLine('Expires', '1970-01-01');
5353
$headers->addHeaderLine('X-Frame-Options: SAMEORIGIN');
5454
$headers->addHeaderLine('X-Content-Type-Options: nosniff');
55-
$xssHeaderValue = !empty($_SERVER['HTTP_USER_AGENT'])
56-
&& strpos($_SERVER['HTTP_USER_AGENT'], XssProtection::IE_8_USER_AGENT) === false
55+
/** @var \Zend\Http\Header\UserAgent $userAgentHeader */
56+
$userAgentHeader = $e->getRequest()->getHeader('User-Agent');
57+
$xssHeaderValue = $userAgentHeader && $userAgentHeader->getFieldValue()
58+
&& strpos($userAgentHeader->getFieldValue(), XssProtection::IE_8_USER_AGENT) === false
5759
? XssProtection::HEADER_ENABLED : XssProtection::HEADER_DISABLED;
5860
$headers->addHeaderLine('X-XSS-Protection: ' . $xssHeaderValue);
5961
}

‎setup/src/Magento/Setup/Module/Di/Code/Reader/FileScanner.php

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ protected function scan()
5454
/*
5555
* MACRO creation
5656
*/
57-
$MACRO_TOKEN_ADVANCE = function () use (&$tokens, &$tokenIndex, &$token, &$tokenContent, &$tokenLine) {
57+
$macroTokenAdvance = function () use (&$tokens, &$tokenIndex, &$token, &$tokenContent, &$tokenLine) {
5858
$tokenIndex = ($tokenIndex === null) ? 0 : $tokenIndex + 1;
5959
if (!isset($tokens[$tokenIndex])) {
6060
$token = false;
@@ -79,15 +79,15 @@ protected function scan()
7979

8080
return $tokenIndex;
8181
};
82-
$MACRO_TOKEN_LOGICAL_START_INDEX = function () use (&$tokenIndex, &$docCommentIndex) {
82+
$macroTokenLogicalStartIndex = function () use (&$tokenIndex, &$docCommentIndex) {
8383
return ($docCommentIndex === false) ? $tokenIndex : $docCommentIndex;
8484
};
85-
$MACRO_DOC_COMMENT_START = function () use (&$tokenIndex, &$docCommentIndex) {
85+
$macroDocCommentStart = function () use (&$tokenIndex, &$docCommentIndex) {
8686
$docCommentIndex = $tokenIndex;
8787

8888
return $docCommentIndex;
8989
};
90-
$MACRO_DOC_COMMENT_VALIDATE = function () use (&$docCommentIndex) {
90+
$macroDocCommentValidate = function () use (&$docCommentIndex) {
9191
static $validTrailingTokens = null;
9292
if ($validTrailingTokens === null) {
9393
$validTrailingTokens = [T_WHITESPACE, T_FINAL, T_ABSTRACT, T_INTERFACE, T_CLASS, T_FUNCTION];
@@ -98,7 +98,7 @@ protected function scan()
9898

9999
return $docCommentIndex;
100100
};
101-
$MACRO_INFO_ADVANCE = function () use (&$infoIndex, &$infos, &$tokenIndex, &$tokenLine) {
101+
$macroInfoAdvance = function () use (&$infoIndex, &$infos, &$tokenIndex, &$tokenLine) {
102102
$infos[$infoIndex]['tokenEnd'] = $tokenIndex;
103103
$infos[$infoIndex]['lineEnd'] = $tokenLine;
104104
$infoIndex++;
@@ -111,7 +111,7 @@ protected function scan()
111111
*/
112112

113113
// Initialize token
114-
$MACRO_TOKEN_ADVANCE();
114+
$macroTokenAdvance();
115115

116116
SCANNER_TOP:
117117

@@ -120,26 +120,26 @@ protected function scan()
120120
}
121121

122122
// Validate current doc comment index
123-
$MACRO_DOC_COMMENT_VALIDATE();
123+
$macroDocCommentValidate();
124124

125125
switch ($this->tokenType) {
126126
case T_DOC_COMMENT:
127-
$MACRO_DOC_COMMENT_START();
127+
$macroDocCommentStart();
128128
goto SCANNER_CONTINUE;
129129
//goto no break needed
130130

131131
case T_NAMESPACE:
132132
$infos[$infoIndex] = [
133133
'type' => 'namespace',
134-
'tokenStart' => $MACRO_TOKEN_LOGICAL_START_INDEX(),
134+
'tokenStart' => $macroTokenLogicalStartIndex(),
135135
'tokenEnd' => null,
136136
'lineStart' => $token[2],
137137
'lineEnd' => null,
138138
'namespace' => null,
139139
];
140140

141141
// start processing with next token
142-
if ($MACRO_TOKEN_ADVANCE() === false) {
142+
if ($macroTokenAdvance() === false) {
143143
goto SCANNER_END;
144144
}
145145

@@ -159,7 +159,7 @@ protected function scan()
159159

160160
SCANNER_NAMESPACE_CONTINUE:
161161

162-
if ($MACRO_TOKEN_ADVANCE() === false) {
162+
if ($macroTokenAdvance() === false) {
163163
goto SCANNER_END;
164164
}
165165
goto SCANNER_NAMESPACE_TOP;
@@ -168,14 +168,14 @@ protected function scan()
168168

169169
$namespace = $infos[$infoIndex]['namespace'];
170170

171-
$MACRO_INFO_ADVANCE();
171+
$macroInfoAdvance();
172172
goto SCANNER_CONTINUE;
173173
//goto no break needed
174174

175175
case T_USE:
176176
$infos[$infoIndex] = [
177177
'type' => 'use',
178-
'tokenStart' => $MACRO_TOKEN_LOGICAL_START_INDEX(),
178+
'tokenStart' => $macroTokenLogicalStartIndex(),
179179
'tokenEnd' => null,
180180
'lineStart' => $tokens[$tokenIndex][2],
181181
'lineEnd' => null,
@@ -187,7 +187,7 @@ protected function scan()
187187
$useAsContext = false;
188188

189189
// start processing with next token
190-
if ($MACRO_TOKEN_ADVANCE() === false) {
190+
if ($macroTokenAdvance() === false) {
191191
goto SCANNER_END;
192192
}
193193

@@ -221,14 +221,14 @@ protected function scan()
221221

222222
SCANNER_USE_CONTINUE:
223223

224-
if ($MACRO_TOKEN_ADVANCE() === false) {
224+
if ($macroTokenAdvance() === false) {
225225
goto SCANNER_END;
226226
}
227227
goto SCANNER_USE_TOP;
228228

229229
SCANNER_USE_END:
230230

231-
$MACRO_INFO_ADVANCE();
231+
$macroInfoAdvance();
232232
goto SCANNER_CONTINUE;
233233
//goto no break needed
234234

@@ -246,7 +246,7 @@ protected function scan()
246246

247247
$infos[$infoIndex] = [
248248
'type' => 'include',
249-
'tokenStart' => $MACRO_TOKEN_LOGICAL_START_INDEX(),
249+
'tokenStart' => $macroTokenLogicalStartIndex(),
250250
'tokenEnd' => null,
251251
'lineStart' => $tokens[$tokenIndex][2],
252252
'lineEnd' => null,
@@ -255,7 +255,7 @@ protected function scan()
255255
];
256256

257257
// start processing with next token
258-
if ($MACRO_TOKEN_ADVANCE() === false) {
258+
if ($macroTokenAdvance() === false) {
259259
goto SCANNER_END;
260260
}
261261

@@ -269,14 +269,14 @@ protected function scan()
269269

270270
SCANNER_INCLUDE_CONTINUE:
271271

272-
if ($MACRO_TOKEN_ADVANCE() === false) {
272+
if ($macroTokenAdvance() === false) {
273273
goto SCANNER_END;
274274
}
275275
goto SCANNER_INCLUDE_TOP;
276276

277277
SCANNER_INCLUDE_END:
278278

279-
$MACRO_INFO_ADVANCE();
279+
$macroInfoAdvance();
280280
goto SCANNER_CONTINUE;
281281
//goto no break needed
282282

@@ -288,7 +288,7 @@ protected function scan()
288288
case T_TRAIT:
289289
$infos[$infoIndex] = [
290290
'type' => ($this->tokenType === T_FUNCTION) ? 'function' : 'class',
291-
'tokenStart' => $MACRO_TOKEN_LOGICAL_START_INDEX(),
291+
'tokenStart' => $macroTokenLogicalStartIndex(),
292292
'tokenEnd' => null,
293293
'lineStart' => $tokens[$tokenIndex][2],
294294
'lineEnd' => null,
@@ -333,20 +333,20 @@ protected function scan()
333333

334334
SCANNER_CLASS_CONTINUE:
335335

336-
if ($MACRO_TOKEN_ADVANCE() === false) {
336+
if ($macroTokenAdvance() === false) {
337337
goto SCANNER_END;
338338
}
339339
goto SCANNER_CLASS_TOP;
340340

341341
SCANNER_CLASS_END:
342342

343-
$MACRO_INFO_ADVANCE();
343+
$macroInfoAdvance();
344344
goto SCANNER_CONTINUE;
345345
}
346346

347347
SCANNER_CONTINUE:
348348

349-
if ($MACRO_TOKEN_ADVANCE() === false) {
349+
if ($macroTokenAdvance() === false) {
350350
goto SCANNER_END;
351351
}
352352
goto SCANNER_TOP;

0 commit comments

Comments
 (0)