Skip to content

Commit

Permalink
Merge pull request #10092 from mmcev106/respect-stubs-with-autoload-d…
Browse files Browse the repository at this point in the history
…efined-require-path

Respect stubs in all cases
  • Loading branch information
orklah authored Aug 16, 2023
2 parents 5c4cd06 + 09cdb35 commit fb95bc0
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 14 deletions.
15 changes: 1 addition & 14 deletions src/Psalm/Internal/Codebase/Functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,8 @@ public function getStorage(
$function_id = substr($function_id, 1);
}

$from_stubs = false;
if (isset(self::$stubbed_functions[$function_id])) {
$from_stubs = self::$stubbed_functions[$function_id];
return self::$stubbed_functions[$function_id];
}

$file_storage = null;
Expand Down Expand Up @@ -113,10 +112,6 @@ public function getStorage(
return $this->reflection->getFunctionStorage($function_id);
}

if ($from_stubs) {
return $from_stubs;
}

throw new UnexpectedValueException(
'Expecting non-empty $root_file_path and $checked_file_path',
);
Expand All @@ -135,10 +130,6 @@ public function getStorage(
}
}

if ($from_stubs) {
return $from_stubs;
}

throw new UnexpectedValueException(
'Expecting ' . $function_id . ' to have storage in ' . $checked_file_path,
);
Expand All @@ -149,10 +140,6 @@ public function getStorage(
$declaring_file_storage = $this->file_storage_provider->get($declaring_file_path);

if (!isset($declaring_file_storage->functions[$function_id])) {
if ($from_stubs) {
return $from_stubs;
}

throw new UnexpectedValueException(
'Not expecting ' . $function_id . ' to not have storage in ' . $declaring_file_path,
);
Expand Down
44 changes: 44 additions & 0 deletions tests/StubTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1514,4 +1514,48 @@ function em(EntityManager $em) : void {

$this->analyzeFile($file_path, new Context());
}

/**
* This covers the following case encountered by mmcev106:
* - A function was defined without a docblock
* - The autoloader defined a global containing the path to that file
* - The code being scanned required the path specified by the autoloader defined global
* - A docblock was added via a stub that marked the function as a taint source
* - The stub docblock was incorrectly ignored, causing the the taint source to be ignored
*/
public function testAutoloadDefinedRequirePath(): void
{
$this->project_analyzer = $this->getProjectAnalyzerWithConfig(
TestConfig::loadFromXML(
dirname(__DIR__),
'<?xml version="1.0"?>
<psalm
errorLevel="1"
autoloader="tests/fixtures/stubs/define_custom_require_path.php"
>
<projectFiles>
<directory name="src" />
</projectFiles>
<stubs>
<file name="tests/fixtures/stubs/custom_taint_source.phpstub" />
</stubs>
</psalm>',
),
);

$this->project_analyzer->trackTaintedInputs();

$file_path = getcwd() . '/src/somefile.php';

$this->addFile(
$file_path,
'<?php
require_once CUSTOM_REQUIRE_PATH;
echo custom_taint_source();',
);

$this->expectExceptionMessage('TaintedHtml - /src/somefile.php');
$this->analyzeFile($file_path, new Context());
}
}
3 changes: 3 additions & 0 deletions tests/fixtures/stubs/custom_taint_source.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

function custom_taint_source(){}
6 changes: 6 additions & 0 deletions tests/fixtures/stubs/custom_taint_source.phpstub
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?php

/**
* @psalm-taint-source input
*/
function custom_taint_source(){}
3 changes: 3 additions & 0 deletions tests/fixtures/stubs/define_custom_require_path.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

define('CUSTOM_REQUIRE_PATH', __DIR__ . '/custom_taint_source.php');

0 comments on commit fb95bc0

Please sign in to comment.