ZF-8721: Zend_Service_ReCaptcha_Response breaks reCaptcha API doc recommendation

Description

Quote from the reCaptcha API documentation: "The response from verify is a series of strings separated by \n. To read the string, split the line and read each field. New lines may be added in the future. Implementations should ignore these lines"

Zend_Service_ReCaptcha_Response::setFromHttpResponse() breaks this recommendation by strictly testing the line number of the response:

if (count($parts) !== 2) { $status = 'false'; $errorCode = ''; } else { list($status, $errorCode) = $parts; }

instead of testing it with an inequality, thus, test should be:

if (count($parts) < 2) { // modified line $status = 'false'; $errorCode = ''; } else { list($status, $errorCode) = $parts; }

Regards,

Corentin Larose

Comments

I finally thing that this issue should be a minor bug.

Additionally, exceptions are thrown if either the challenge or response fields are empty, in the following method:

protected function _post($challengeField, $responseField)

..but this means an exception occurs in you present a captcha to the user but they enter nothing. Surely the way ReCaptcha is intended to work would include an empty (response) field as simply another permutation of an incorrect response.

e.g. if a user doesn't enter a response at all, they'd expect to see the normal 'Incorrect. Try again.' message when the ReCaptcha is re-displayed. With these exceptions being thrown, the only way to recreate the intended behaviour (using Zend_Service_ReCaptcha in a stand-alone sense here) would be to catch these exceptions, and then in the catch block simulate the outcome by manually setting the appropriate error code.

I suggest these exceptions for 'missing' challenge/response fields are not needed, and contradict the expected behaviour of ReCaptcha..

[~jonnott]: I agree. I don't think the exceptions should be thrown when the challenge and/or response is missing. I can fix this but need to make sure the core guys think it's OK.

I will anyways fix the issue above.

[~jonnott] Could you create a separate issue on this tracker with what you mentioned, and assign it to me? This current issue will be closed soon.