Merge pull request google#244 from google/show-connection-error

Show an explicit error when the connection fails

Author: rowan-m

ARCHITECTURE.md

@@ -54,3 +54,11 @@ unit tested by passing in a mock. Take a look at
 [`RequestMethod\Curl`](./src/ReCaptcha/RequestMethod/Curl.php) with the matching
 [`RequestMethod/CurlPostTest`](./tests/ReCaptcha/RequestMethod/CurlPostTest.php)
 to see this pattern in action.
+
+### Error conventions
+
+The client returns the response as provided by the reCAPTCHA services augmented
+with additional error codes based on the client's checks. When adding a new
+[`RequestMethod`](./src/ReCaptcha/RequestMethod.php) ensure that it returns the
+`ReCaptcha::E_CONNECTION_FAILED` and `ReCaptcha::E_BAD_RESPONSE` where
+appropriate.

src/ReCaptcha/ReCaptcha.php

@@ -53,7 +53,7 @@ class ReCaptcha
      * Could not connect to service
      * @const string
      */
-    const E_BAD_CONNECTION = 'bad-connection';
+    const E_CONNECTION_FAILED = 'connection-failed';
 
     /**
      * Did not receive a 200 from the service

src/ReCaptcha/RequestMethod/CurlPost.php

@@ -87,6 +87,10 @@ public function submit(RequestParameters $params)
         $response = $this->curl->exec($handle);
         $this->curl->close($handle);
 
-        return $response;
+        if ($response !== false) {
+            return $response;
+        }
+
+        return '{"success": false, "error-codes": ["'.ReCaptcha::E_CONNECTION_FAILED.'"]}';
     }
 }

src/ReCaptcha/RequestMethod/Post.php

@@ -69,6 +69,12 @@ public function submit(RequestParameters $params)
             ),
         );
         $context = stream_context_create($options);
-        return file_get_contents($this->siteVerifyUrl, false, $context);
+        $response = file_get_contents($this->siteVerifyUrl, false, $context);
+
+        if ($response !== false) {
+            return $response;
+        }
+
+        return '{"success": false, "error-codes": ["'.ReCaptcha::E_CONNECTION_FAILED.'"]}';
     }
 }

src/ReCaptcha/RequestMethod/SocketPost.php

@@ -68,7 +68,7 @@ public function submit(RequestParameters $params)
         $urlParsed = parse_url($this->siteVerifyUrl);
 
         if (false === $this->socket->fsockopen('ssl://' . $urlParsed['host'], 443, $errno, $errstr, 30)) {
-            return '{"success": false, "error-codes": ["'.ReCaptcha::E_BAD_CONNECTION.'"]}';
+            return '{"success": false, "error-codes": ["'.ReCaptcha::E_CONNECTION_FAILED.'"]}';
         }
 
         $content = $params->toQueryString();

tests/ReCaptcha/RequestMethod/CurlPostTest.php

@@ -26,6 +26,7 @@
 
 namespace ReCaptcha\RequestMethod;
 
+use \ReCaptcha\ReCaptcha;
 use \ReCaptcha\RequestParameters;
 use PHPUnit\Framework\TestCase;
 
@@ -88,4 +89,27 @@ public function testOverrideSiteVerifyUrl()
         $response = $pc->submit(new RequestParameters("secret", "response"));
         $this->assertEquals('RESPONSEBODY', $response);
     }
+
+    public function testConnectionFailureReturnsError()
+    {
+        $curl = $this->getMockBuilder(\ReCaptcha\RequestMethod\Curl::class)
+            ->disableOriginalConstructor()
+            ->setMethods(array('init', 'setoptArray', 'exec', 'close'))
+            ->getMock();
+        $curl->expects($this->once())
+                ->method('init')
+                ->willReturn(new \stdClass);
+        $curl->expects($this->once())
+                ->method('setoptArray')
+                ->willReturn(true);
+        $curl->expects($this->once())
+                ->method('exec')
+                ->willReturn(false);
+        $curl->expects($this->once())
+                ->method('close');
+
+        $pc = new CurlPost($curl);
+        $response = $pc->submit(new RequestParameters("secret", "response"));
+        $this->assertEquals('{"success": false, "error-codes": ["'.ReCaptcha::E_CONNECTION_FAILED.'"]}', $response);
+    }
 }

tests/ReCaptcha/RequestMethod/PostTest.php

@@ -26,6 +26,7 @@
 
 namespace ReCaptcha\RequestMethod;
 
+use \ReCaptcha\ReCaptcha;
 use ReCaptcha\RequestParameters;
 use PHPUnit\Framework\TestCase;
 
@@ -37,7 +38,7 @@ class PostTest extends TestCase
 
     public function setUp()
     {
-        $this->parameters = new RequestParameters("secret", "response", "remoteip", "version");
+        $this->parameters = new RequestParameters('secret', 'response', 'remoteip', 'version');
     }
 
     public function tearDown()
@@ -48,27 +49,39 @@ public function tearDown()
     public function testHTTPContextOptions()
     {
         $req = new Post();
-        self::$assert = array($this, "httpContextOptionsCallback");
+        self::$assert = array($this, 'httpContextOptionsCallback');
         $req->submit($this->parameters);
-        $this->assertEquals(1, $this->runcount, "The assertion was ran");
+        $this->assertEquals(1, $this->runcount, 'The assertion was ran');
     }
 
     public function testSSLContextOptions()
     {
         $req = new Post();
-        self::$assert = array($this, "sslContextOptionsCallback");
+        self::$assert = array($this, 'sslContextOptionsCallback');
         $req->submit($this->parameters);
-        $this->assertEquals(1, $this->runcount, "The assertion was ran");
+        $this->assertEquals(1, $this->runcount, 'The assertion was ran');
     }
 
     public function testOverrideVerifyUrl()
     {
         $req = new Post('https://over.ride/some/path');
         self::$assert = array($this, 'overrideUrlOptions');
         $req->submit($this->parameters);
-        $this->assertEquals(1, $this->runcount, "The assertion was ran");
+        $this->assertEquals(1, $this->runcount, 'The assertion was ran');
     }
 
+    public function testConnectionFailureReturnsError()
+    {
+        $req = new Post('https://bad.connection/');
+        self::$assert = array($this, 'connectionFailureResponse');
+        $response = $req->submit($this->parameters);
+        $this->assertEquals('{"success": false, "error-codes": ["'.ReCaptcha::E_CONNECTION_FAILED.'"]}', $response);
+    }
+
+    public function connectionFailureResponse()
+    {
+        return false;
+    }
     public function overrideUrlOptions(array $args)
     {
         $this->runcount++;
@@ -84,14 +97,14 @@ public function httpContextOptionsCallback(array $args)
         $this->assertArrayHasKey('http', $options);
 
         $this->assertArrayHasKey('method', $options['http']);
-        $this->assertEquals("POST", $options['http']['method']);
+        $this->assertEquals('POST', $options['http']['method']);
 
         $this->assertArrayHasKey('content', $options['http']);
         $this->assertEquals($this->parameters->toQueryString(), $options['http']['content']);
 
         $this->assertArrayHasKey('header', $options['http']);
         $headers = array(
-            "Content-type: application/x-www-form-urlencoded",
+            'Content-type: application/x-www-form-urlencoded',
         );
         foreach ($headers as $header) {
             $this->assertContains($header, $options['http']['header']);
@@ -112,9 +125,9 @@ public function sslContextOptionsCallback(array $args)
     protected function assertCommonOptions(array $args)
     {
         $this->assertCount(3, $args);
-        $this->assertStringStartsWith("https://www.google.com/", $args[0]);
+        $this->assertStringStartsWith('https://www.google.com/', $args[0]);
         $this->assertFalse($args[1]);
-        $this->assertTrue(is_resource($args[2]), "The context options should be a resource");
+        $this->assertTrue(is_resource($args[2]), 'The context options should be a resource');
     }
 }
 

tests/ReCaptcha/RequestMethod/SocketPostTest.php

@@ -112,7 +112,7 @@ public function testSubmitBadResponse()
         $this->assertEquals('{"success": false, "error-codes": ["'.ReCaptcha::E_BAD_RESPONSE.'"]}', $response);
     }
 
-    public function testSubmitBadRequest()
+    public function testConnectionFailureReturnsError()
     {
         $socket = $this->getMockBuilder(\ReCaptcha\RequestMethod\Socket::class)
             ->disableOriginalConstructor()
@@ -123,6 +123,6 @@ public function testSubmitBadRequest()
                 ->willReturn(false);
         $ps = new SocketPost($socket);
         $response = $ps->submit(new RequestParameters("secret", "response", "remoteip", "version"));
-        $this->assertEquals('{"success": false, "error-codes": ["'.ReCaptcha::E_BAD_CONNECTION.'"]}', $response);
+        $this->assertEquals('{"success": false, "error-codes": ["'.ReCaptcha::E_CONNECTION_FAILED.'"]}', $response);
     }
 }