Skip to content

Commit 3ff6933

Browse files
committed
AC-3483:: SVC false-positive: modifying system.xml file from another module
1 parent 66389c1 commit 3ff6933

File tree

1 file changed

+39
-82
lines changed

1 file changed

+39
-82
lines changed

src/Analyzer/SystemXml/Analyzer.php

Lines changed: 39 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public function analyze($registryBefore, $registryAfter)
8383

8484
//process removed files
8585
$this->reportRemovedFiles($removedModules, $registryBefore);
86-
$duplicateNode = [];
86+
8787
//process common files
8888
foreach ($commonModules as $moduleName) {
8989
$moduleNodesBefore = $nodesBefore[$moduleName];
@@ -97,13 +97,30 @@ public function analyze($registryBefore, $registryAfter)
9797

9898
if ($addedNodes) {
9999
$afterFile = $registryAfter->mapping[XmlRegistry::NODES_KEY][$moduleName];
100-
$duplicateNode = $this->reportAddedNodesWithDuplicateCheck($afterFile, $addedNodes);
100+
$baseDir = $this->getBaseDir($afterFile);
101+
foreach ($addedNodes as $nodeId => $node) {
102+
$newNodeData = $this->getNodeData($node);
103+
$nodePath = $newNodeData['path'];
104+
105+
// Extract section, group, and fieldId with error handling
106+
$extractedData = $this->extractSectionGroupField($nodePath);
107+
if ($extractedData === null) {
108+
// Skip the node if its path is invalid
109+
continue;
110+
}
101111

102-
print_r($duplicateNode);
103-
if ($duplicateNode) {
104-
$this->reportDuplicateNodes($afterFile, $addedNodes);
105-
} else {
106-
$this->reportAddedNodes($afterFile, $addedNodes);
112+
// Extract section, group, and fieldId
113+
list($sectionId, $groupId, $fieldId) = $extractedData;
114+
115+
// Call function to check if this field is duplicated in other system.xml files
116+
$isDuplicated = $this->isDuplicatedFieldInXml($baseDir, $sectionId, $groupId, $fieldId, $afterFile);
117+
foreach ($isDuplicated as $isDuplicatedItem) {
118+
if ($isDuplicatedItem['status'] === 'duplicate') {
119+
$this->reportDuplicateNodes($afterFile, [$nodeId => $node ]);
120+
} else {
121+
$this->reportAddedNodes($afterFile, [$nodeId => $node ]);
122+
}
123+
}
107124
}
108125
}
109126
}
@@ -161,68 +178,6 @@ private function searchSystemXmlFiles($magentoBaseDir, $excludeFile = null)
161178
return $systemXmlFiles;
162179
}
163180

164-
/**
165-
* Check and Report duplicate nodes
166-
*
167-
* @param $afterFile
168-
* @param $addedNodes
169-
* @param $moduleNodesBefore
170-
* @return bool
171-
* @throws \Exception
172-
*/
173-
private function reportAddedNodesWithDuplicateCheck($afterFile, $addedNodes)
174-
{
175-
$baseDir = $this->getBaseDir($afterFile);
176-
$hasDuplicate = false;
177-
178-
foreach ($addedNodes as $nodeId => $node) {
179-
$newNodeData = $this->getNodeData($node);
180-
$nodePath = $newNodeData['path'];
181-
182-
// Extract section, group, and fieldId with error handling
183-
$extractedData = $this->extractSectionGroupField($nodePath);
184-
185-
// var_dump($extractedData);
186-
if ($extractedData === null) {
187-
// Skip the node if its path is invalid
188-
// echo "Skipping node with invalid path: $nodePath\n";
189-
continue;
190-
}
191-
192-
// Extract section, group, and fieldId
193-
list($sectionId, $groupId, $fieldId) = $extractedData;
194-
195-
196-
197-
// Call function to check if this field is duplicated in other system.xml files
198-
$isDuplicated = $this->isDuplicatedFieldInXml($baseDir, $sectionId, $groupId, $fieldId, $afterFile);
199-
200-
if ($isDuplicated) {
201-
$hasDuplicate = true;
202-
echo "\n\nDuplicate found for the field: $fieldId\n\n";
203-
} else {
204-
205-
$hasDuplicate = false;
206-
echo "\n\nNo duplicates for the field: $fieldId\n";
207-
}
208-
209-
// $nodeIds = strrpos($newNodeData['path'],'/');
210-
211-
// Extract the substring after the last "/"
212-
// if ($nodeIds !== false) {
213-
// $fieldId = substr($newNodeData['path'], $nodeIds + 1);
214-
// }
215-
// echo "\nDuplicate $hasDuplicate for the\n";
216-
// echo "Checking for duplicates for Section: $sectionId, Group: $groupId, Field: $fieldId\n";
217-
218-
// return $hasDuplicate;
219-
}
220-
221-
return $hasDuplicate;
222-
223-
// return $this->isDuplicatedFieldInXml($baseDir, $fieldId, $afterFile);
224-
}
225-
226181
/**
227182
* Method to extract section, group and field from the Node
228183
*
@@ -405,39 +360,41 @@ private function reportRemovedNodes(string $file, array $nodes)
405360
* @return array
406361
* @throws \Exception
407362
*/
408-
private function isDuplicatedFieldInXml(?string $baseDir, string $sectionId, string $groupId, string $fieldId, string $afterFile): array
363+
private function isDuplicatedFieldInXml(?string $baseDir, string $sectionId, string $groupId, ?string $fieldId, string $afterFile): array
409364
{
365+
$hasDuplicate = false;
410366
if ($baseDir) {
411367
$systemXmlFiles = $this->searchSystemXmlFiles($baseDir, $afterFile);
412368

413369
$result = [];
414370

415-
416371
foreach ($systemXmlFiles as $systemXmlFile) {
417372
$xmlContent = file_get_contents($systemXmlFile);
418373
try {
419374
$xml = new \SimpleXMLElement($xmlContent);
420375
} catch (\Exception $e) {
421-
echo "\nError parsing XML: " . $e->getMessage() . "\n";
422376
continue; // Skip this file if there's a parsing error
423377
}
424378
// Find <field> nodes with the given field ID
425379
// XPath to search for <field> within a specific section and group
426380
$fields = $xml->xpath("//section[@id='$sectionId']/group[@id='$groupId']/field[@id='$fieldId']");
427-
428-
429381
if (!empty($fields)) {
430-
echo "\n\nFound match in: $systemXmlFile\n";
431-
$result[] = [
432-
'status' => 'patch',
433-
'field' => $fieldId
434-
];
435-
// break ;
382+
$hasDuplicate = true; // Set the duplicate flag to true if a match is found
383+
break; // Since we found a duplicate, we don't need to check further for this field
436384
}
437385
}
438-
386+
if ($hasDuplicate) {
387+
$result[] = [
388+
'status' => 'duplicate',
389+
'field' => $fieldId
390+
];
391+
} else {
392+
$result[] = [
393+
'status' => 'minor',
394+
'field' => $fieldId
395+
];
396+
}
439397
}
440-
441398
return $result;
442399
}
443400
}

0 commit comments

Comments
 (0)