Fix: Potential warning if getting bad content from API

Close #1 Signed-off-by: Yann 'Ze' Richard <[email protected]>
DSI-Universite-Rennes2 · Aug 28, 2022 · 1fb2bdd · 1fb2bdd
1 parent c9c45fc
commit 1fb2bdd
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 5 deletions.
diff --git a/src/PwnedPasswords.php b/src/PwnedPasswords.php
@@ -108,7 +108,7 @@ public function isPwned(string $password) : bool
      *
      * @return string[] Return array with http return code and body
      */
-    protected function callApi(string $hashPrefix) : array
+    public function callApi(string $hashPrefix) : array
     {
         $ch = curl_init($this->apiUrl . $hashPrefix);
         curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
@@ -131,7 +131,7 @@ protected function callApi(string $hashPrefix) : array
         return array($httpCode, $results);
     }
 
-    private function getFromApi(string $password) : int
+    public function getFromApi(string $password) : int
     {
         $hash       = strtoupper(sha1($password));
         $hashPrefix = substr($hash, 0, self::PP_RANGE_LENGTH);
@@ -143,9 +143,15 @@ private function getFromApi(string $password) : int
 
         switch ($httpCode) {
             case 200:
-                $hashes = array();
-                $lines  = explode("\r\n", $body);
+                $lines = explode("\r\n", $body);
                 foreach ($lines as $line) {
+                    if (mb_strpos($line, ':') === false) {
+                        // bad format for the line ?!
+                        error_log(
+                            sprintf(static::class . '::' . __METHOD__ . " : Bad line in Api reply for the '%s' k-anonimity prefix: %s", $hashPrefix, $line)
+                        );
+                        continue;
+                    }
                     [$resSuffix, $resCount] = explode(':', trim($line));
                     if (strcmp($resSuffix, $hashSuffix) === 0) {
                         return (int) $resCount;
@@ -164,7 +170,7 @@ private function getFromApi(string $password) : int
                 // There is no 404 or other responses on PwnedPasswords API :
                 // https://haveibeenpwned.com/API/v2#PwnedPasswords
                 throw new RuntimeException(
-                    sprintf('Unknown return code from API end-point %u', $httpCode)
+                    sprintf('Unknown return code from API end-point %u for %s prefix', $httpCode, $hashPrefix)
                 );
         }
     }

diff --git a/tests/units/PwnedPasswords.php b/tests/units/PwnedPasswords.php
@@ -115,6 +115,32 @@ public function testIPwned() : void
         ;
     }
 
+    public function testIsPwnedWithMockAPIReplyBadlines() : void
+    {
+        $i=0;
+
+        $i++;
+        $pass = 'UgpIlamNHVpCNufpSuaoYypfOjMmCwxkKgLHXoIdejqzU0KA9fTEST';
+        $this->mockGenerator->generate('\UniversiteRennes2\PwnedPasswords\PwnedPasswords', '\UR2PP', 'PwnedPasswords');
+        $mock = new \UR2PP\PwnedPasswords();
+        // Mock callApi() to reply with bad lines
+        $this->calling($mock)->callApi = array(0 => 200, 1 => "FFFFFaaaaaaaaaaaaaaaaaaaa:1\r\nFFFFFazeazeaze\n\r");
+
+        // mock error_log
+        $this->function->error_log = true;
+
+        $this->assert(__METHOD__ . ' : test #' . $i)
+            ->if($mock)
+            ->then
+                ->boolean($mock->isPwned($pass))->isFalse
+            ->mock($mock)
+                ->call('callApi')->once()
+        ;
+
+        // Check that error_log was called
+        $this->function('error_log')->wasCalled()->once();
+    }
+
     public function testIsPwnedWithMockCurlExec() : void
     {
         $i =0;