Correction of wrong parameter types in exception handling

[hajoseng]

In three cases an exception is thrown by using two parameters while the exception object accepts only one. This causes an internal server error when one of these exceptions is thrown.

[demiankatz]

@hajoseng, all three of the exceptions referenced in this PR extend PHP's base Exception class without overriding anything, so they should have the same constructor. The base Exception accepts three parameters according to the manual: message, code, and previous exception. Thus, the original code looks to me like it is already correct. Is it possible the failure you are encountering has a different cause? What exactly is showing up in the Apache error log when you experience the error?

[hajoseng]

@demiankatz I've just realized the websites shows "Internal Server Error" while there's no error not within the apache log. The error within the vufind log is:

2021-04-13T13:04:46+02:00 DEBUG (7): PAIAplus\ILS\Driver\PAIA: HTTP status 403 received
2021-04-13T13:04:46+02:00 CRIT (2): Error : Wrong parameters for VuFind\Exception\Auth([string $message [, long $code [, Throwable $previous = NULL]]])
[...]
Backtrace:
/opt/sites/beluga-core-dev7hh/module/VuFind/src/VuFind/ILS/Driver/PAIA.php line 957 - class = Exception, function = __construct, args: none.

Having a closer look the error seems to be rather a wrong parameter (empty string instead of long integer als second parameter) than a wrong parameter count. So the default - having no error code - shouldn't be '' but rather a number (probably 0).

[hajoseng]

PS: Ignore PAIAplus; it's only a wrapper for our special PAIA setting.

[demiankatz]

Yes, @hajoseng, I believe you are right, and it's the default value that is causing the problem. If you revert to the original code, and then change $array['code'] ?? '' to (int)($array['code'] ?? 0), does that fix the issue? If so, we might want to do a more comprehensive search through the code for other exceptions that may be doing similar things. Are you using PHP 8? This seems like an error that might be related to stricter type enforcement in newer PHP versions.

In any case, thanks for finding this issue!

[hajoseng]

@demiankatz Yes this fixes the issue; I've just committet an update. I'm using php 7.4. I'm going to search for more of these issues and come back with my results.

[demiankatz]

Thanks, @hajoseng, I have renamed this PR to reflect the slight change in the solution to the problem.

Is it safe to assume that $array['code'] will always contain an integer, or should we add the (int) typecast to be sure?

In any case, please let me know when you are ready for me to merge this. For now, I will wait until I hear back from you, in case you decide to add more fixes to this PR.

Thanks again!

[hajoseng]

I've looked through the entire code; there are only two other objects passing a second parameter to the Exception Object. One using a hardcoded 0 the other one passing an original http status code. I think there's nothing more to do and the pr can be merged.

[demiankatz]

@hajoseng, I really appreciate the help reviewing the code!

I've made one minor edit to add parentheses around the expression being typecast. In PHP, typecasting has a higher precedence than null coalescing, so without the extra parentheses, you would probably get error messages in situations where 'code' was not set. I'm pretty confident it's safe to merge now, so I'm going to go ahead -- but please let me know if I've somehow introduced a problem.

[demiankatz]

I've also backported this to the release-7.1 branch, since it qualifies as a bug fix.