Merge pull request #107 from paragonie/psalm-redundant

Psalm redundant
paragonie · Mar 27, 2018 · cbda59b · cbda59b
2 parents 653bbf0 + c7c16a0
commit cbda59b
Show file tree

Hide file tree

Showing 11 changed files with 22 additions and 53 deletions.
diff --git a/psalm.xml b/psalm.xml
@@ -7,7 +7,6 @@
         <directory name="./src" />
     </projectFiles>
     <issueHandlers>
-        <RedundantConditionGivenDocblockType errorLevel="info" /><!-- We can clean this up later -->
         <TooFewArguments errorLevel="suppress" /> <!-- \sodium_memzero() -->
         <PropertyNotSetInConstructor errorLevel="suppress" />
     </issueHandlers>

diff --git a/src/Asymmetric/Crypto.php b/src/Asymmetric/Crypto.php
@@ -109,11 +109,6 @@ public static function encryptWithAd(
             $ourPrivateKey,
             $theirPublicKey
         );
-        // @codeCoverageIgnoreStart
-        if (!($ss instanceof HiddenString)) {
-            throw new \TypeError();
-        }
-        // @codeCoverageIgnoreEnd
         $sharedSecretKey = new EncryptionKey($ss);
         $ciphertext = SymmetricCrypto::encryptWithAd(
             $plaintext,
@@ -188,11 +183,6 @@ public static function decryptWithAd(
             $ourPrivateKey,
             $theirPublicKey
         );
-        // @codeCoverageIgnoreStart
-        if (!($ss instanceof HiddenString)) {
-            throw new \TypeError();
-        }
-        // @codeCoverageIgnoreEnd
         $sharedSecretKey = new EncryptionKey($ss);
         $plaintext = SymmetricCrypto::decryptWithAd(
             $ciphertext,

diff --git a/src/Cookie.php b/src/Cookie.php
@@ -82,12 +82,12 @@ public function fetch(string $name)
             return null;
         }
         try {
-            /** @var string $stored */
+            /** @var string|array|int|float|bool $stored */
             $stored = $_COOKIE[$name];
             if (!\is_string($stored)) {
                 throw new InvalidType('Cookie value is not a string');
             }
-            $config = self::getConfig((string) $stored);
+            $config = self::getConfig($stored);
             $decrypted = Crypto::decrypt(
                 $stored,
                 $this->key,
@@ -120,10 +120,6 @@ protected static function getConfig(string $stored): SymmetricConfig
         if (\hash_equals(Binary::safeSubstr($stored, 0, 5), Halite::VERSION_PREFIX)) {
             /** @var string $decoded */
             $decoded = Base64UrlSafe::decode($stored);
-            if (!\is_string($decoded)) {
-                \sodium_memzero($stored);
-                throw new InvalidMessage('Incorrect encoding');
-            }
             return SymmetricConfig::getConfig(
                 $decoded,
                 'encrypt'

diff --git a/src/KeyFactory.php b/src/KeyFactory.php
@@ -732,7 +732,7 @@ public static function loadSignatureKeyPair(string $filePath): SignatureKeyPair
     /**
      * Export a cryptography key to a string (with a checksum)
      *
-     * @param Key|KeyPair $key
+     * @param object $key
      * @return HiddenString
      *
      * @throws CannotPerformOperation
@@ -745,8 +745,7 @@ public static function export($key): HiddenString
             return self::export(
                 $key->getSecretKey()
             );
-        }
-        if ($key instanceof Key) {
+        } elseif ($key instanceof Key) {
             return new HiddenString(
                 Hex::encode(
                     Halite::HALITE_VERSION_KEYS . $key->getRawKeyMaterial() .
@@ -758,9 +757,7 @@ public static function export($key): HiddenString
                 )
             );
         }
-        // @codeCoverageIgnoreStart
         throw new \TypeError('Expected a Key.');
-        // @codeCoverageIgnoreEnd
     }
 
     /**

diff --git a/src/Password.php b/src/Password.php
@@ -164,12 +164,6 @@ protected static function getConfig(string $stored): SymmetricConfig
         ) {
             /** @var string $decoded */
             $decoded = Base64UrlSafe::decode($stored);
-            if (!\is_string($decoded)) {
-                // @codeCoverageIgnoreStart
-                \sodium_memzero($stored);
-                throw new InvalidMessage('Invalid encoding');
-                // @codeCoverageIgnoreEnd
-            }
             return SymmetricConfig::getConfig(
                 $decoded,
                 'encrypt'

diff --git a/src/Stream/MutableFile.php b/src/Stream/MutableFile.php
@@ -56,6 +56,7 @@ class MutableFile implements StreamInterface
      * @param string|resource $file
      * @throws InvalidType
      * @throws FileAccessDenied
+     * @psalm-suppress RedundantConditionGivenDocblockType
      */
     public function __construct($file)
     {
@@ -182,7 +183,7 @@ public function readBytes(int $num, bool $skipTests = false): string
             }
             /** @var int $bufSize */
             $bufSize = \min($remaining, self::CHUNK);
-            /** @var string $read */
+            /** @var string|bool $read */
             $read = \fread($this->fp, $bufSize);
             if (!\is_string($read)) {
                 // @codeCoverageIgnoreStart

diff --git a/src/Stream/ReadOnlyFile.php b/src/Stream/ReadOnlyFile.php
@@ -72,6 +72,7 @@ class ReadOnlyFile implements StreamInterface
      * @throws FileError
      * @throws InvalidType
      * @throws \TypeError
+     * @psalm-suppress RedundantConditionGivenDocblockType
      */
     public function __construct($file, Key $key = null)
     {
@@ -81,6 +82,7 @@ public function __construct($file, Key $key = null)
                     'Could not open file for reading'
                 );
             }
+            /** @var resource|bool $fp */
             $fp = \fopen($file, 'rb');
             // @codeCoverageIgnoreStart
             if (!\is_resource($fp)) {
@@ -241,7 +243,7 @@ public function readBytes(int $num, bool $skipTests = false): string
                 break;
             }
             // @codeCoverageIgnoreEnd
-            /** @var string $read */
+            /** @var string|bool $read */
             $read = \fread($this->fp, $remaining);
             if (!\is_string($read)) {
                 // @codeCoverageIgnoreStart

diff --git a/src/Symmetric/Crypto.php b/src/Symmetric/Crypto.php
@@ -160,14 +160,6 @@ public static function decryptWithAd(
         /** @var string $auth */
         $auth = $pieces[5];
 
-        // @codeCoverageIgnoreStart
-        if (!($config instanceof Config)) {
-            throw new CannotPerformOperation(
-                'Config is not an instance of Config. This should not happen.'
-            );
-        }
-        // @codeCoverageIgnoreEnd
-
         /* Split our key into two keys: One for encryption, the other for
            authentication. By using separate keys, we can reasonably dismiss
            likely cross-protocol attacks.
@@ -210,13 +202,6 @@ public static function decryptWithAd(
             (string) $nonce,
             (string) $encKey
         );
-        if (!\is_string($plaintext)) {
-            // @codeCoverageIgnoreStart
-            throw new InvalidMessage(
-                'Invalid message'
-            );
-            // @codeCoverageIgnoreEnd
-        }
         \sodium_memzero($encrypted);
         \sodium_memzero($nonce);
         \sodium_memzero($encKey);

diff --git a/src/Util.php b/src/Util.php
@@ -160,14 +160,6 @@ public static function hkdfBlake2b(
         // ORM = first L octets of T
         /** @var string $orm */
         $orm = Binary::safeSubstr($t, 0, $length);
-
-        // @codeCoverageIgnoreStart
-        if (!\is_string($orm)) {
-            throw new CannotPerformOperation(
-                'An unknown error has occurred'
-            );
-        }
-        // @codeCoverageIgnoreEnd
         return $orm;
     }
 

diff --git a/test/unit/KeyTest.php b/test/unit/KeyTest.php
@@ -246,6 +246,12 @@ public function testImport()
             bin2hex($encKeypair->getPublicKey()->getRawKeyMaterial()),
             bin2hex($import->getRawKeyMaterial())
         );
+
+        try {
+            KeyFactory::export(new stdClass());
+            $this->fail('Expected a TypeError to be raised');
+        } catch (TypeError $ex) {
+        }
     }
 
     /**

diff --git a/test/unit/StreamTest.php b/test/unit/StreamTest.php
@@ -72,6 +72,13 @@ public function testUnreadableFile()
             $this->assertSame('Could not open file for writing', $ex->getMessage());
         }
         unlink($filename);
+
+        try {
+            new ReadOnlyFile('/etc/shadow');
+            $this->fail('File should not be readable');
+        } catch (CryptoException\FileAccessDenied $ex) {
+            $this->assertSame('Could not open file for reading', $ex->getMessage());
+        }
     }
 
     /**