[HttpKernel] Base DataCollector throws warning on unsupported scheme strings #20336
Conversation
|
Don't know if we should consider this an issue or not, but this new behavior, coupled with #19973 allow to easily access files like the
Of course it should not be a real issue, as it only work with the web profiler enabled, but deploying by mistake the |
|
In fact, #19973 alone is enough to get access to any file under the project root, no help needed from the DataCollector: just write the URL to the file in your browser bar: |
|
@nicolas-grekas : Do you know why the https://travis-ci.org/symfony/symfony/builds/171330048 travis build didn't fail, whereas it does locally through the warning, and what should be done about it in order for the test case to be relevant ? |
| @@ -130,7 +130,7 @@ private function decorateVar($var) | |||
| return new ClassStub($var); | |||
| } | |||
| } | |||
| if (false !== strpos($var, DIRECTORY_SEPARATOR) && file_exists($var)) { | |||
| if (false !== strpos($var, DIRECTORY_SEPARATOR) && @file_exists($var)) { | |||
There was a problem hiding this comment.
I suggest being more strict here:
if (false !== strpos($var, DIRECTORY_SEPARATOR) && false === strpos($var, '://') && file_exists($var)) {
There was a problem hiding this comment.
Indeed, I think it totally makes sense 👍
I've also updated the test case which could be completed later in order to cover the base collector cloneVar behavior, but let me know if you think it's not that much useful.
|
|
||
| class DataCollectorTest extends \PHPUnit_Framework_TestCase | ||
| { | ||
| use VarDumperTestTrait; |
There was a problem hiding this comment.
not sure I'd use this trait
instead, I propose to turn the Data object into a string using:
$dumper = new CliDumper();
$dumper->setColors(false);
$this->assertDumpMatchesFormat($expected, $dumper->dump($data, true));There was a problem hiding this comment.
I hesitated, thanks for the confirmation 👍
|
👍 |
|
Thank you @ogizanagi. |
…ted scheme strings (ogizanagi) This PR was merged into the 3.2-dev branch. Discussion ---------- [HttpKernel] Base DataCollector throws warning on unsupported scheme strings | Q | A | | --- | --- | | Branch? | master | | Bug fix? | yes | | New feature? | no | | BC breaks? | no | | Deprecations? | no | | Tests pass? | yes | | Fixed tickets | N/A | | License | MIT | | Doc PR | N/A | The issue concerns any collector based on the abstract `Symfony\Component\HttpKernel\DataCollector\DataCollector` class using `cloneVar` on a string containing a unsupported scheme. The easiest way to reproduce the issue is to add a simple query parameter like `?uri=foo://bar` on any url of your application: > ContextErrorException in DataCollector.php line 134: > Warning: file_exists(): Unable to find the wrapper "foo" - did you forget to enable it when you configured PHP? This PR simply fixes the issue by muting the warning on `file_exists`. But maybe there is a better strategy. Commits ------- 52faa00 Fix base DataCollector throws warning on unsupported scheme strings
The issue concerns any collector based on the abstract
Symfony\Component\HttpKernel\DataCollector\DataCollectorclass usingcloneVaron a string containing a unsupported scheme.The easiest way to reproduce the issue is to add a simple query parameter like
?uri=foo://baron any url of your application:This PR simply fixes the issue by muting the warning on
file_exists. But maybe there is a better strategy.