mollie / mollie-api-php

Mollie API client for PHP
http://www.mollie.com
BSD 2-Clause "Simplified" License
552 stars 191 forks source link

Namespace inconsistency when not using Composer. #244

Closed kikosoft closed 6 years ago

kikosoft commented 6 years ago

I downloaded the mollie-api-php.zip from the releases page, version v2.0.10.

The example payment creation works fine, no Composer needed. However, when I required vendor/autoload.php in my own project, I got a fatal error when I used it in the same way as in the examples. The error reads:

Call to undefined function _PhpScoper5b5b241cd221b\GuzzleHttp\Psr7\stream_for() in /mollie-api-php/vendor/guzzlehttp/psr7/src/Request.php: 41

After some digging around I found that there was already another GuzzleHttp loaded on the system in its normal name space. This means that the test in vendor/guzzlehttp/psr7/src/functions_include.php assumed the functions were loaded in the Mollie name space when in reality they weren't. This is the code in that PHP file:

namespace _PhpScoper5b5b241cd221b;

// Don't redefine the functions if included multiple times.
if (!\function_exists('GuzzleHttp\\Psr7\\str')) {
    require __DIR__ . '/functions.php';
}

This tests for the presence of the function str() in two name spaces:

_PhpScoper5b5b241cd221b\GuzzleHttp\\Psr7\\
GuzzleHttp\\Psr7\\

The function existed the latter name space, in my project, so function.php was never loaded. When I changed the code to the version below, it worked fine.

namespace _PhpScoper5b5b241cd221b;

// Don't redefine the functions if included multiple times.
if (!\function_exists('_PhpScoper5b5b241cd221b\GuzzleHttp\\Psr7\\str')) {
    require __DIR__ . '/functions.php';
}

Now I will admit that this is a rare problem. I'm also not sure whether this is a php-scoper problem or not. Nevertheless, it created a problem for me when I tried to use the Mollie PHP API, so I decided to report it here.

willemstuursma commented 6 years ago

Thank you for the extensive research. I think we have to create a "patcher" for PhpScoper, to automatically rewrite the function_exists call.

kikosoft commented 6 years ago

Hmm, I still keep getting conflicts between Mollie's API and an existing normal Guzzle client in its default namespace. Has this ever been tested?

I could suggest to check for the presence of the Guzzle client, in its default namespace, and then use that one, but that can be problematic due to version differences. So forget that.

The only thing left to tell you, is how you can reproduce what I ran into:

  1. Include the Mollie PHP API, without using Composer.
  2. Include another package, containing Guzzle in it's default namespace.
  3. Use the latter package in such a way that the Guzzle client is actively used.

The errors will not make you happy. I found that the default Guzzle client often tried to access parts of the Mollie Guzzle client, and they are not compatible. I tested this in PHP 5.6.37.

Clearly not everything of the Mollie Guzzle package has been placed correctly in its unique namespace with PhpScoper. It works as long as you don't actively try to use the other Guzzle, apart from the error I reported above. I assume the active use of a second Guzzle has not been tested?

My temporary solution to this problem is to never use the Mollie API when I am using the other package as well, so no conflict can occur. This is obviously not ideal.

nickurt commented 6 years ago

I think I hit this as well with WooCommerce today cc https://github.com/mollie/WooCommerce/issues/235

davdebcom commented 6 years ago

Getting multiple reports from users of the WooCommerce plugin. Using PhpScoper seems to solve some conflicts (plugins I tested all seemed to now work) but creates new conflicts with other plugins.

Smitsel commented 6 years ago

Hi guys, we've just made sure that this function exists check will actually check the Scoped function. So those should be resolved.

I'm having troubles to actually test if this resolves the WooCommerce issues. Can you guys check this out with the new zipfile? @nickurt @davdebcom

davdebcom commented 6 years ago

@Smitsel Tested this fix, still getting

Fatal error: Declaration of GuzzleHttp\Client::send(Psr\Http\Message\RequestInterface $request, array $options = Array) must be compatible with _MollieApiPhp\GuzzleHttp\ClientInterface::send(_MollieApiPhp\Psr\Http\Message\RequestInterface $request, array $options = Array) in /var/www/html/wp-content/plugins/woocommerce-pdf-ips-pro/dropbox/vendor/guzzlehttp/guzzle/src/Client.php on line 25

And is there is special reason to whitelist: GuzzleHttp\ClientInterface in scoper.inc.php?

Reverted to a Mollie API PHP client without PHP Scoper for the WooCommerce plugin 4.0.2.

Smitsel commented 6 years ago

Yes this should not be whitelisted as it creates an Alias for the interface and we get these strict errors.

However this only brings us one step further. The next error we get is that the first Guzzle client that gets auto loaded will have its corresponding functions.php loaded. The next one auto loaded will have the same file require hash in composer. Which will not be loaded and thus will not have its functions.php resulting in the errors like stream_for not being found.

So we're kind of running in circles here. So we're gonna discuss this internally and come back to this.

willemstuursma commented 6 years ago

Created https://github.com/humbug/php-scoper/issues/256 to see if we can resolve the scoping issue at least.

willemstuursma commented 6 years ago

And is there is special reason to whitelist: GuzzleHttp\ClientInterface in scoper.inc.php?

If it is not whitelisted, it would be impossible to inject your own client.

However, I guess that for the WooCommerce module purposes it's not useful to add your own client anyway.

willemstuursma commented 6 years ago

humbug/php-scoper#256 has been resolved, the problem was caused by the 7 in the class name.

Once a new version has been released, we'll release a new version here too.

4r7if3x commented 5 years ago

@willemstuursma I'm using v2.2.0 in my project (without composer) and yet have the same issue. I'm not sure how the topic is resolved, but I have no clue how to fix this.