Saferpay Payment Provider broken

[virtualize]

https://github.com/isotope/core/blob/20b2425af49d166ec338b5e8c13b66af80700ea1/system/modules/isotope/library/Isotope/Model/Payment/Saferpay.php#L122

This condition will always fail as its returns true if the status code is not 200 OR the response message does not begin with "ERROR".

So if we have a valid response (code = 200 and message has no error string), we break here and redirect to the failed page. WTF?

This bug is more than a year old, so I assume nobody ever tested this or is using this PSP. Unfortunately we do. Will prepare a PR...

[aschempp]

That is not true. The second condition tests if ERROR: is not the first word in the response. So if the response code is 200 and the response starts with ERROR the request would be cancelled.

[virtualize]

Excuse me, but you're wrong here. You're last sentence describes what we want to achieve, but thats not what the expression does. Please see this simple test case:

<?php

function test($intCode, $strResponse) {
    if ((int)$intCode !== 200 || 0 !== strpos($strResponse, 'ERROR:')) {
        return "Request CANCELED \n";
    }
    return "Request EXECUTED \n";
}

$testCases = [
    [400, 'ERROR: message'],
    [200, 'ERROR: message'],
    [400, 'SUCCESS: message'],
    [200, 'SUCCESS: message'],
];

foreach ($testCases as list($intCode, $strResponse)) {
    echo sprintf("Code: '%s', Message: '%s' => Result: %s",
        $intCode, $strResponse, test($intCode, $strResponse)
    );
}

[micbis]

@aschempp Hallo Andreas - wie schauts hier aus? Siehst du eine Chance dies zeitnah zu integrieren/fixen? Wir würden gerne wenn irgendwie möglich auf die offizielle Version setzen und nicht forken müssen. Können wir dich allenfalls unterstützen?

[aschempp]

Closed in favor of #1857

[aschempp]

Fixed in 4088a3bda1b53cf04437501fa99bc988dd2e861c