evoWeb / recaptcha

TYPO3 Extension to make use of googles nocaptcha
GNU General Public License v2.0
5 stars 18 forks source link

[BUG] No connection to verify server result --> fatal PHP error (due to no array) #9

Closed AndreasA closed 6 years ago

AndreasA commented 6 years ago

The CaptchaService->queryVerificationServer method: https://github.com/evoWeb/recaptcha/blob/develop/Classes/Services/CaptchaService.php#L214 Will fail with a fatal PHP error getUrl fails because then it won't return an array but the return type is set to force an array. So there needs to be a check and if it isn't an array assume false verification.

garbast commented 6 years ago

You are welcome to hand in a pull request.

I' m a little curiouse why there is no getUrl result. And please remember if the result does not deliver anything useful the recaptcha does not server any purpose. Beside server config preventing connection to google there is nothing i can think off that causes such a behaviour.

AndreasA commented 6 years ago

In most cases this won't be an issue but there is always the possibility that the server cannot temporary connect to the server in which I case we shouldn't just get a PHP error that results in nothing being displayed but at least a negative result or maybe throw an Exception which could be caught and handled instead.

garbast commented 6 years ago

Well ok. I agree that you dont want to have fatal errors if you cant connect to the server. But that should be in theory already taken care of by the following lines:

$response = GeneralUtility::getUrl($this->configuration['verify_server'] . '?' . $request); return $response ? json_decode($response, true) : [];

If the getUrl does not get a result the $response is false or empty string. In both cases the json_decode does not get executed but an empty array is returned.

With the focus on the extension i cant see how to improve the situation. In case of an valid or invalid result of getUrl the method returns an array. So please submit a pull request with you solution for that.

AndreasA commented 6 years ago

Ah. True my fault - didn't see this code change in develop. That should do it. However, this change hasn't been released yet.

garbast commented 6 years ago

Tag 8.2.3 created and release to packagist and TER