Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Third-party code whitelisting #305

Closed
kmgalanakis opened this issue Mar 8, 2019 · 18 comments
Closed

Third-party code whitelisting #305

kmgalanakis opened this issue Mar 8, 2019 · 18 comments

Comments

@kmgalanakis
Copy link
Contributor

Hi! In my unscoped files I'm using a third party class constant \Foo::BAR. When my files are getting scoped, this constant becomes \PREFIX\Foo:BAR. Is there any way I could prevent this?

@theofidry
Copy link
Member

You can check if the global classes are whitelisted (if they are however keep in mind they will appear as scoped but will have an alias registered, cf. the alias description details). Alternatively you may use the namespace whitelisting (although you need to be careful about the autoloading)

@kmgalanakis
Copy link
Contributor Author

I don't have control over the global classes, they are from another project. They are not included in mine.

When I try to whitelist them in my scoper.inc.php file, adding the classes there has absolutely no effect.

Am I missing something?

@theofidry
Copy link
Member

Ah I see. Hm there is currently no easy way to do this built-in. It would be a nice to have indeed. Meanwhile I think your best shot would be to make use of a patcher to manually fix it.

@theofidry theofidry changed the title Third party class constant from the global scope whitelisting Third-party code whitelisting Mar 8, 2019
@kmgalanakis
Copy link
Contributor Author

I see. Thanks for your (REALLY) prompt answer.

@theofidry
Copy link
Member

theofidry commented May 7, 2019

After some thought, for the next time I dig into this issue:

So the issue right now is in PHP-Scoper we have:

'whitelist' => [
  'Acme\Foo', // Can be any symbol
],

So what I do is I always prefix stuff, and when I come across Acme\Foo, I register it to the Whitelist object and then at the end, I know what type of symbol I need to alias.

The issue in that PR is: when one wants to whitelist a symbol that is not part of your codebase, i.e. a symbol dynamically loaded from somewhere. For example some PHPUnit related symbols for your PHPUnit extension (which doesn't require them).

I think what I can do is allow the whitelist to be more explicit:

'whitelist' => [
  'Acme\Foo', // Can be any symbol
  'Acme\Bar' => 'class',
  'Acme\Baz' => ['class', 'constant'],
],

This way I can register the alias for Acme\Bar and Acme\Baz even if not encountered during the scoping process, because I know what can of alias they are.

@theofidry theofidry added this to the 1.0.0 milestone Jun 26, 2019
@dyszczo
Copy link

dyszczo commented Jul 22, 2019

I'm not sure if I have understood the solution correctly.
Even if a whitelist was more explicit, it will still add a prefix to the given \Foo::BAR and then add alias "if \Foo::BAR not exists use \prefix\Foo::BAR".
The \Foo::BAR class/function/const will exist because it will be loaded from a third party, so the condition to add an alias won't ever be fulfilled.
The client code will try to use \prefix\Foo::BAR, but it would not be found as the correct declaration from a third party is still \Foo::BAR, therefore a fatal error will occur.

@theofidry
Copy link
Member

There is two problems:

  • Aliasing requires to know what is the type of the symbol, you don't alias the same way a function and a class
  • Punctually whitelisting stuff, i.e. prefixing only some symbols and not prefixing whitelisted symbols, completely breaks autoloading in pretty much any project

Also note that the whitelisting for constants works differently because there just no easy way to alias constants: https://github.com/humbug/php-scoper#constants-whitelisting

That said I'm not convinced the solution propose above is perfect either. For example what if you third-party code is loaded after the autoloading of your code? The code aliasing will not work in this case...

@mundschenk-at
Copy link

Since there is no autoloading for functions, "real" whitelisting third-party functions should not break anything in that regard. (Constants are different matter of course.)

@theofidry
Copy link
Member

yes functions are fine mostly because of the way PHP-Scoper does it: https://github.com/humbug/php-scoper#functions-whitelisting

Unlike the class aliasing, it can be lazily evaluated

@mundschenk-at
Copy link

mundschenk-at commented Jul 22, 2019

yes functions are fine mostly because of the way PHP-Scoper does it: https://github.com/humbug/php-scoper#functions-whitelisting

Unlike the class aliasing, it can be lazily evaluated

Unfortunately, that does not work for third-party functions that are not defined in the scoped codebase (e.g. WordPress functions). The functions calls are prefixed, but there would need to be a "reverse alias" (the other way round from what PHP-Scoper currently generates) for them to work.

@theofidry
Copy link
Member

Currently it doesn't, but if you have:

'whitelist' => [
  'Acme\foo' => 'function',
],

The existing whitelisting mechanism could work fine without further changes. It's however a problem still for classes and constants I think

@mundschenk-at
Copy link

I'm not sure how that work exactly. You'd not prefix those functions?

@theofidry
Copy link
Member

I tried to selectively prefix what was necessary to prefix before, but after over a year of trial it was not getting anywhere really.

That said it was mostly for "traditional" code bases, i.e. Composer based autoloading without dependencies on arbitrary third-party calls. Maybe the solution would be to force some symbols to remain unchanged when it comes to third-party call. I would probably be better to have a different configuration entry in that case though

@dyszczo
Copy link

dyszczo commented Jul 22, 2019

In my case I've messed with Humbug\PhpScoper\Reflector. Unfortunately I could not inject it into a scoper, so I've just replaced the file with my own version in postUpdate.

@mundschenk-at
Copy link

I tried to selectively prefix what was necessary to prefix before, but after over a year of trial it was not getting anywhere really.

That said it was mostly for "traditional" code bases, i.e. Composer based autoloading without dependencies on arbitrary third-party calls. Maybe the solution would be to force some symbols to remain unchanged when it comes to third-party call. I would probably be better to have a different configuration entry in that case though

A secondary whitelist would probably be best (keeping the the "What is this?" mapping for the symbols, e.g. function, constant), or would a single entry for all symbols (e.g. thirdparty) be enough?

@theofidry
Copy link
Member

since the symbol would be left untouched I don't think requiring the type would bring anything, so I think a thirdparty entry for all symbols would be enough

@calvinalkan
Copy link
Contributor

@theofidry this should be solvable with the cli tool I mentioned in #303

@theofidry
Copy link
Member

theofidry commented Feb 5, 2022

Leaving symbols untouched is now (0.16) possible via the exclude- config options. Better doc on how to use it or scan a code for some symbols (like the case of WP) can be discussed in more specific issues (WP already has its own #303)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants