Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Have weak_vendors ignore deprecations from outside#24864

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@greg0ire
Copy link
Contributor

@greg0iregreg0ire commentedNov 7, 2017•
edited
Loading

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

QA
Branch?3.3
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?not yet
Fixed tickets#24853
LicenseMIT

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 loadvendor/autoload.php from the phar or something like that? solved, had nothing to do with that.

@greg0ire
Copy link
ContributorAuthor

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

@sroze
Copy link
Contributor

Nop, just thatsome version of PHP are skipped for pull-requests. See the logs of thesuccessful ones.

@greg0ire
Copy link
ContributorAuthor

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

@greg0iregreg0ireforce-pushed theignore_deprecations_from_phar branch 2 times, most recently from3f43275 tod2371faCompareNovember 8, 2017 21:59
@greg0iregreg0ire changed the titleWIP: Have weak_vendors ignore deprecations from outsideHave weak_vendors ignore deprecations from outsideNov 8, 2017
@greg0ire
Copy link
ContributorAuthor

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

@greg0ire
Copy link
ContributorAuthor

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

@soullivaneuh
Copy link
Contributor

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

Should not be?

greg0ire reacted with thumbs up emoji

@greg0ire
Copy link
ContributorAuthor

That's correct, let me rebase.

soullivaneuh reacted with thumbs up emoji

@greg0iregreg0ireforce-pushed theignore_deprecations_from_phar branch fromd2371fa to8f8d5f4CompareNovember 10, 2017 15:44
@greg0iregreg0ire changed the base branch from3.4 to3.3November 10, 2017 15:44
@nicolas-grekasnicolas-grekas added this to the3.3 milestoneNov 10, 2017
@greg0ire
Copy link
ContributorAuthor

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?

}
$path =realpath($path) ?:$path;
$realPath =realpath($path);
if (false ===$realPath &&'-' !==$path) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Can't think of a less fragile solution though.

@greg0iregreg0ireforce-pushed theignore_deprecations_from_phar branch from8f8d5f4 tod378244CompareDecember 7, 2017 20:50
@greg0ire
Copy link
ContributorAuthor

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 fromphp://stdin.

@greg0ire
Copy link
ContributorAuthor

greg0ire commentedDec 9, 2017•
edited
Loading

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

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

And then confirmed with somevar_dump statements.

@greg0ire
Copy link
ContributorAuthor

greg0ire commentedDec 9, 2017•
edited
Loading

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/shphp -d memory_limit=-1 /usr/local/bin/composer updatecurl -sSL https://phar.phpunit.de/phpunit-4.8.9.phar>phpunitchmod +x phpunit./phpunit src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler

@greg0ire
Copy link
ContributorAuthor

fabbot seems stuck, let's close and reopen

@greg0ire
Copy link
ContributorAuthor

Please review again

@greg0ire
Copy link
ContributorAuthor

ping@nicolas-grekas

@greg0iregreg0ireforce-pushed theignore_deprecations_from_phar branch fromd378244 to6580c38CompareJanuary 5, 2018 18:38
@greg0ire
Copy link
ContributorAuthor

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.
@greg0iregreg0ireforce-pushed theignore_deprecations_from_phar branch fromf776108 to9ce0ae2CompareJanuary 21, 2018 18:42
@nicolas-grekas
Copy link
Member

Thank you@greg0ire.

greg0ire and OskarStark reacted with hooray emoji

@nicolas-grekasnicolas-grekas merged commit9ce0ae2 intosymfony:3.3Jan 21, 2018
nicolas-grekas added a commit that referenced this pull requestJan 21, 2018
This PR was merged into the 3.3 branch.Discussion----------Have weak_vendors ignore deprecations from outsidephar:// 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       | MITI 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
@greg0iregreg0ire deleted the ignore_deprecations_from_phar branchJanuary 21, 2018 18:50
This was referencedJan 29, 2018
greg0ire added a commit to sonata-project/dev-kit that referenced this pull requestMar 6, 2018
This reverts commite103e32.The issues with this mode have been fixed and are released.Seesymfony/symfony#24864
soullivaneuh pushed a commit to sonata-project/dev-kit that referenced this pull requestMay 5, 2018
This reverts commite103e32.The issues with this mode have been fixed and are released.Seesymfony/symfony#24864
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

6 participants

@greg0ire@sroze@soullivaneuh@nicolas-grekas@xabbuh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp