Have weak_vendors ignore deprecations from outside #24864

greg0ire · 2017-11-07T20:22:25Z

phar:// and eval() can execute code that may or may not come from the vendors.

Q	A
Branch?	3.3
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	not yet
Fixed tickets	#24853
License	MIT

I haven't managed to get the phar test to pass yet, but before I do, is it ok for me to commit a test phar in Symfony? I'm thinking trust issues? Although the phar is almost plain text.

~~Next, I'm stuck because it looks like symfony does not know how to load classes anymore... should I somehow load vendor/autoload.php from the phar or something like that?~~ solved, had nothing to do with that.

greg0ire · 2017-11-07T21:00:59Z

Wait wut? It passes on some versions of php??? I thought it wouldn't pass at all...

sroze · 2017-11-07T21:41:43Z

Nop, just that some version of PHP are skipped for pull-requests. See the logs of the successful ones.

greg0ire · 2017-11-07T21:48:45Z

Thanks @sroze , things make sense again thanks to you!

greg0ire · 2017-11-08T22:11:07Z

Ok so now the build only fails on 7.2... with deps=low

greg0ire · 2017-11-08T23:00:17Z

Could it be become of the phpunit-bridge vendor interfering? I can see it being installed : https://travis-ci.org/symfony/symfony/jobs/299336081#L942

soullivaneuh · 2017-11-10T15:34:22Z

As a bug fix, it should be merged on 3.3: https://symfony.com/roadmap?version=3.3#checker

Should not be?

greg0ire · 2017-11-10T15:43:04Z

That's correct, let me rebase.

greg0ire · 2017-11-16T17:03:29Z

Tests are failing probably because

the script used to run tests is installing the latest version of the PHPUnit Bridge, which does not yet have this feature.

See #24867 (comment)

I'm afraid there is nothing I can do about that. Is there hope to get this merged nonetheless?

nicolas-grekas · 2017-12-04T11:14:49Z

src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php

@@ -75,9 +75,12 @@ public static function register($mode = 0)
                    }
                }
            }
-            $path = realpath($path) ?: $path;
+            $realPath = realpath($path);
+            if (false === $realPath && '-' !== $path) {


I think - has changed on 7.2, the string is different

Dang, good catch! I knew this was a bit fragile but don't know what else to use, will have to give it more thought.

Thanks to docker I now know this could be "Standard input code"

Can't think of a less fragile solution though.

greg0ire · 2017-12-07T20:54:01Z

Not sure it's that string you were thinking about @nicolas-grekas : here is the change : php/php-src@3ed8b7a , and it does not change from -, but from php://stdin.

greg0ire · 2017-12-09T14:35:40Z

Ok so now I am sure, tried the code with:

docker run -it -v $PWD:/tmp/sf php:7.2-cli-alpine3.6 /bin/sh
cd /tmp/sf
curl -sSL https://phar.phpunit.de/phpunit.phar > phpunit
chmod +x phpunit
phpunit src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/

And then confirmed with some var_dump statements.

greg0ire · 2017-12-09T15:48:05Z

I just confirmed the tests pass on 5.5 by using

docker run -it -v $PWD:/tmp/sf melkorm/php-5.5-cli-with-extensions /bin/sh
php -d memory_limit=-1 /usr/local/bin/composer update
curl -sSL https://phar.phpunit.de/phpunit-4.8.9.phar>phpunit
chmod +x phpunit
./phpunit src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler

greg0ire · 2017-12-16T16:06:48Z

fabbot seems stuck, let's close and reopen

greg0ire · 2017-12-20T18:04:41Z

Please review again

greg0ire · 2018-01-02T16:52:23Z

ping @nicolas-grekas

greg0ire · 2018-01-05T18:41:17Z

I added a script to regenerate the phar, so that it's easier to understand or maintain.

phar:// and eval() can execute code that may or may not come from the vendors.

nicolas-grekas · 2018-01-21T18:49:52Z

Thank you @greg0ire.

This PR was merged into the 3.3 branch. Discussion ---------- Have weak_vendors ignore deprecations from outside phar:// and eval() can execute code that may or may not come from the vendors. | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | not yet | Fixed tickets | #24853 | License | MIT I haven't managed to get the phar test to pass yet, but before I do, is it ok for me to commit a test phar in Symfony? I'm thinking trust issues? Although the phar is almost plain text. ~~Next, I'm stuck because it looks like symfony does not know how to load classes anymore... should I somehow load `vendor/autoload.php` from the phar or something like that?~~ solved, had nothing to do with that. Commits ------- 9ce0ae2 Have weak_vendors ignore deprecations from outside

This reverts commit e103e32. The issues with this mode have been fixed and are released. See symfony/symfony#24864

carsonbot added Status: Needs Review Bug labels Nov 7, 2017

greg0ire force-pushed the ignore_deprecations_from_phar branch 2 times, most recently from 3f43275 to d2371fa Compare November 8, 2017 21:59

greg0ire changed the title ~~WIP: Have weak_vendors ignore deprecations from outside~~ Have weak_vendors ignore deprecations from outside Nov 8, 2017

This was referenced Nov 9, 2017

[PhpUnitBridge]allow outdated vendors mode #24867

Closed

DevKit updates for 3.x branch sonata-project/SonataPageBundle#895

Merged

core23 mentioned this pull request Nov 10, 2017

Revert "Use the weak vendors mode" sonata-project/dev-kit#335

Merged

greg0ire force-pushed the ignore_deprecations_from_phar branch from d2371fa to 8f8d5f4 Compare November 10, 2017 15:44

greg0ire changed the base branch from 3.4 to 3.3 November 10, 2017 15:44

nicolas-grekas added this to the 3.3 milestone Nov 10, 2017

xabbuh added the PhpUnitBridge label Nov 16, 2017

nicolas-grekas reviewed Dec 4, 2017

View reviewed changes

greg0ire force-pushed the ignore_deprecations_from_phar branch from 8f8d5f4 to d378244 Compare December 7, 2017 20:50

greg0ire closed this Dec 16, 2017

greg0ire reopened this Dec 16, 2017

greg0ire force-pushed the ignore_deprecations_from_phar branch from d378244 to 6580c38 Compare January 5, 2018 18:38

greg0ire mentioned this pull request Jan 15, 2018

Recommend being lenient only with other vendors symfony/symfony-docs#9062

Merged

greg0ire force-pushed the ignore_deprecations_from_phar branch from 6580c38 to f776108 Compare January 21, 2018 18:41

Have weak_vendors ignore deprecations from outside

9ce0ae2

phar:// and eval() can execute code that may or may not come from the vendors.

greg0ire force-pushed the ignore_deprecations_from_phar branch from f776108 to 9ce0ae2 Compare January 21, 2018 18:42

nicolas-grekas approved these changes Jan 21, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Jan 21, 2018

nicolas-grekas merged commit 9ce0ae2 into symfony:3.3 Jan 21, 2018

greg0ire deleted the ignore_deprecations_from_phar branch January 21, 2018 18:50

This was referenced Jan 29, 2018

Release v3.3.16 #25955

Merged

Release v3.4.4 #25956

Merged

Release v4.0.4 #25957

Merged

greg0ire added a commit to sonata-project/dev-kit that referenced this pull request Mar 6, 2018

Try weak_vendors mode again

573c81f

This reverts commit e103e32. The issues with this mode have been fixed and are released. See symfony/symfony#24864

soullivaneuh pushed a commit to sonata-project/dev-kit that referenced this pull request May 5, 2018

Try weak_vendors mode again

9991997

This reverts commit e103e32. The issues with this mode have been fixed and are released. See symfony/symfony#24864

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Have weak_vendors ignore deprecations from outside #24864

Have weak_vendors ignore deprecations from outside #24864

greg0ire commented Nov 7, 2017 •

edited

Loading

greg0ire commented Nov 7, 2017

sroze commented Nov 7, 2017

greg0ire commented Nov 7, 2017

greg0ire commented Nov 8, 2017

greg0ire commented Nov 8, 2017

soullivaneuh commented Nov 10, 2017

greg0ire commented Nov 10, 2017

greg0ire commented Nov 16, 2017

nicolas-grekas Dec 4, 2017

greg0ire Dec 4, 2017

greg0ire Dec 7, 2017

greg0ire Dec 7, 2017

greg0ire commented Dec 7, 2017

greg0ire commented Dec 9, 2017 •

edited

Loading

greg0ire commented Dec 9, 2017 •

edited

Loading

greg0ire commented Dec 16, 2017

greg0ire commented Dec 20, 2017

greg0ire commented Jan 2, 2018

greg0ire commented Jan 5, 2018

nicolas-grekas commented Jan 21, 2018

Have weak_vendors ignore deprecations from outside #24864

Have weak_vendors ignore deprecations from outside #24864

Conversation

greg0ire commented Nov 7, 2017 • edited Loading

greg0ire commented Nov 7, 2017

sroze commented Nov 7, 2017

greg0ire commented Nov 7, 2017

greg0ire commented Nov 8, 2017

greg0ire commented Nov 8, 2017

soullivaneuh commented Nov 10, 2017

greg0ire commented Nov 10, 2017

greg0ire commented Nov 16, 2017

nicolas-grekas Dec 4, 2017

Choose a reason for hiding this comment

greg0ire Dec 4, 2017

Choose a reason for hiding this comment

greg0ire Dec 7, 2017

Choose a reason for hiding this comment

greg0ire Dec 7, 2017

Choose a reason for hiding this comment

greg0ire commented Dec 7, 2017

greg0ire commented Dec 9, 2017 • edited Loading

greg0ire commented Dec 9, 2017 • edited Loading

greg0ire commented Dec 16, 2017

greg0ire commented Dec 20, 2017

greg0ire commented Jan 2, 2018

greg0ire commented Jan 5, 2018

nicolas-grekas commented Jan 21, 2018

greg0ire commented Nov 7, 2017 •

edited

Loading

greg0ire commented Dec 9, 2017 •

edited

Loading

greg0ire commented Dec 9, 2017 •

edited

Loading