After extensive debugging I've found a module breaking bug with regards to the wrongful usage of the ->has()
method in the parameter class. At the moment, the Access class is ALWAYS going to return AccessResult::forbidden('None-existing payment');
because of the use of the ->has()
method in obtaining the COMPLUS parameter since ->has()
will always return either TRUE or FALSE and not the value of COMPLUS with regards to this line if code:
$payment = $this->paymentStorage->load((int) $parameters->has('complus'));
result is: NULL
when it should be:
$payment = $this->paymentStorage->load((int) $parameters->get('complus'));
result is: COMPLUS value
The other wrongful usage of the ->has() method results in a logging message always logging an error log regardless of a successful payment or not. In the feedback method of the KbcpaypageController, the following statement always logs the error message because ->has() in this instance will always return TRUE since the parameter NCERROR always exists. The current code is:
if ($parameters->has('ncerror')) {
when it should be:
if ($parameters->has('ncerror') && $parameters->get('ncerror')) {
Checking the value as well as checking if the parameter exists ensures we log the correct message.
I will provide a patch with these changes
Comment | File | Size | Author |
---|---|---|---|
#2 | wrong-use-of-has-method-3257383-2.patch | 1.34 KB | ChristianSanders |
Comments
Comment #2
ChristianSanders CreditAttribution: ChristianSanders at ComputerMinds commentedComment #4
MatthijsSorry for the delayed response, apparently I wasn't following the issues of this project.
Your patch has been applied and committed.