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

CoreGatewayFactory try to call gateway's string config parameter that accidentally same as name of php built-in function #692

Closed
Great-Antique opened this issue Sep 29, 2017 · 8 comments · Fixed by #877
Labels

Comments

@Great-Antique
Copy link
Contributor

I have a config of Payum gateways like this (in Symfony):

payum:
    security:
...
    storages:
...
    gateways:
        paypal:
...
        safecharge:
            factory: safecharge
            merchant_id: "%safecharge_merchant_id%"
            merchant_site_id: "%safecharge_merchant_site_id%"
            merchant_secret_key: "%safecharge_merchant_secret_key%"
            sandbox: "%safecharge_sandbox%"
            hash_algorithm: md5

When I run application I have an issue

Warning: md5() expects parameter 1 to be string, object given

Stack:

...
// vendor/payum/core/Payum/Core/CoreGatewayFactory.php line 55
$this->buildClosures($config);
...
// vendor/payum/core/Payum/Core/CoreGatewayFactory.php line 204
$config[$name] = call_user_func($value, $config);

So CoreGatewayFactory try to run md5($config) where $config is an object.

// dump($config);
...
    "merchant_id" => "..."
    "merchant_site_id" => "..."
    "merchant_secret_key" => "..."
    "sandbox" => true
    "hash_algorithm" => "md5"
    "payum.extension.token_factory" => GenericTokenFactoryExtension {#505 ▶}
    "payum.security.token_storage" => DoctrineStorage {#490 ▶}
    // and others
...
@makasim makasim added the bug label Sep 29, 2017
@makasim
Copy link
Member

makasim commented Sep 29, 2017

It must've happened. Any thoughts on how to fix it?

@Great-Antique
Copy link
Contributor Author

As for me we can add checks for string type or function existing

// Not strings
        foreach ($config as $name => $value) {
            if (!is_string($value) && is_callable($value)) {
                $config[$name] = call_user_func($value, $config);
            }
        }

// OR Not existing functions (but strings for static methods)
        foreach ($config as $name => $value) {
            if (is_string($value) && function_exists($value)) {
                continue;
            }
            
            if (is_callable($value)) {
                $config[$name] = call_user_func($value, $config);
            }
        }

but maybe someone uses facades (like in Laravel) or some functions that return needed value without arguments (maybe functions that increments some static properties etc).

I didn't find good solution yet. So I decide to make workaround (add prefix to that value in my config).

@diimpp
Copy link

diimpp commented Oct 27, 2017

@Great-Antique Hi, can you please tell me which project you're using as safecharge gateway?

@Great-Antique
Copy link
Contributor Author

@diimpp Hello. I'm currently developing lib, gateway and bundle for SafeCharge. It's very possible that it will be opensourced. But I can't tell you deadlines

@gromez
Copy link

gromez commented Dec 12, 2018

@makasim any news about this bug? Had the same problem with "sha1" value

@Kocal
Copy link
Contributor

Kocal commented May 13, 2020

Got the same issue with sha1 too...
I'm not sure what kind of patch should be applied, and I don't have "clean" workaround too.

@Great-Antique
Copy link
Contributor Author

@Kocal simple workaround is to not use such names. Just add some prefix. In existing gateways there are no such cases so I assume that you talk about some custom code. I use something like algo_sha1 and everything works.

@Kocal
Copy link
Contributor

Kocal commented May 13, 2020

Yeah that's what I've done too... Not a big fan but better than nothing 🤷

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

Successfully merging a pull request may close this issue.

5 participants