Restore binary offsets of PDOStatement parameters

Entity identifiers that are PHP resource streams need to work for all
references, not just the first one. PDOStatement->execute is reading
the binary resources but not restoring the original offsets. No other
code is restoring these streams to their original position so that
they can be reused.

Examples of failures include empty collections on read (the first lazy
load collection on an entity populates correctly, the second is empty)
and foreign-key violations while persisting changes (the first entity
join produces the correct SQL, the second has no data to read and the
FK is violated with the missing binary data).

Making this change as close as possible to the external code that
moves the stream pointer eliminates the need to do this in calling
code. Resource offsets are retrieved immediately before execute in
case they change between the bindValue and execute calls.

The request was originally for the PDO driver but IBMDB2, Mysql, and
PgSQL drivers are also covered. Other drivers will likely also need
work. No attempt has been made to fix the deprecated bindParam code
path. I do not believe this is called by the current Doctrine code,
and is regardless much harder to patch because the reference variables
can be replaced during execute, so the original resources may no
longer be available to restore after the call.

A functional test was added for bindValue and a resource with a
seekable position.

#5895

Author: mcurland

src/Driver/IBMDB2/Statement.php

@@ -10,12 +10,15 @@
 use Doctrine\DBAL\Driver\Statement as StatementInterface;
 use Doctrine\DBAL\ParameterType;
 use Doctrine\Deprecations\Deprecation;
+use Throwable;
 
 use function assert;
 use function db2_bind_param;
 use function db2_execute;
 use function error_get_last;
 use function fclose;
+use function fseek;
+use function ftell;
 use function func_num_args;
 use function is_int;
 use function is_resource;
@@ -28,6 +31,7 @@
 use const DB2_LONG;
 use const DB2_PARAM_FILE;
 use const DB2_PARAM_IN;
+use const SEEK_SET;
 
 final class Statement implements StatementInterface
 {
@@ -213,8 +217,28 @@ private function createTemporaryFile()
      */
     private function copyStreamToStream($source, $target): void
     {
+        $resetTo = false;
+        if (stream_get_meta_data($source)['seekable']) {
+            $resetTo = ftell($source);
+        }
+
         if (@stream_copy_to_stream($source, $target) === false) {
-            throw CannotCopyStreamToStream::new(error_get_last());
+            $copyToStreamError = error_get_last();
+            if ($resetTo !== false) {
+                try {
+                    fseek($source, $resetTo, SEEK_SET);
+                } catch (Throwable $e) {
+                    // Swallow, we want the original exception from stream_copy_to_stream
+                }
+            }
+
+            throw CannotCopyStreamToStream::new($copyToStreamError);
         }
+
+        if ($resetTo === false) {
+            return;
+        }
+
+        fseek($source, $resetTo, SEEK_SET);
     }
 }

src/Driver/Mysqli/Statement.php

@@ -19,11 +19,16 @@
 use function count;
 use function feof;
 use function fread;
+use function fseek;
+use function ftell;
 use function func_num_args;
 use function get_resource_type;
 use function is_int;
 use function is_resource;
 use function str_repeat;
+use function stream_get_meta_data;
+
+use const SEEK_SET;
 
 final class Statement implements StatementInterface
 {
@@ -213,15 +218,26 @@ private function bindTypedParameters(): void
     private function sendLongData(array $streams): void
     {
         foreach ($streams as $paramNr => $stream) {
-            while (! feof($stream)) {
-                $chunk = fread($stream, 8192);
+            $resetTo = false;
+            if (stream_get_meta_data($stream)['seekable']) {
+                $resetTo = ftell($stream);
+            }
 
-                if ($chunk === false) {
-                    throw FailedReadingStreamOffset::new($paramNr);
-                }
+            try {
+                while (! feof($stream)) {
+                    $chunk = fread($stream, 8192);
 
-                if (! $this->stmt->send_long_data($paramNr - 1, $chunk)) {
-                    throw StatementError::new($this->stmt);
+                    if ($chunk === false) {
+                        throw FailedReadingStreamOffset::new($paramNr);
+                    }
+
+                    if (! $this->stmt->send_long_data($paramNr - 1, $chunk)) {
+                        throw StatementError::new($this->stmt);
+                    }
+                }
+            } finally {
+                if ($resetTo !== false) {
+                    fseek($stream, $resetTo, SEEK_SET);
                 }
             }
         }

src/Driver/PDO/Statement.php

@@ -7,17 +7,27 @@
 use Doctrine\DBAL\Driver\Statement as StatementInterface;
 use Doctrine\DBAL\ParameterType;
 use Doctrine\Deprecations\Deprecation;
+use PDO;
 use PDOException;
 use PDOStatement;
 
 use function array_slice;
+use function fseek;
+use function ftell;
 use function func_get_args;
 use function func_num_args;
+use function is_resource;
+use function stream_get_meta_data;
+
+use const SEEK_SET;
 
 final class Statement implements StatementInterface
 {
     private PDOStatement $stmt;
 
+    /** @var mixed[]|null */
+    private ?array $paramResources = null;
+
     /** @internal The statement can be only instantiated by its driver connection. */
     public function __construct(PDOStatement $stmt)
     {
@@ -43,6 +53,9 @@ public function bindValue($param, $value, $type = ParameterType::STRING)
         }
 
         $pdoType = ParameterTypeMap::convertParamType($type);
+        if ($pdoType === PDO::PARAM_LOB) {
+            $this->trackParamResource($value);
+        }
 
         try {
             return $this->stmt->bindValue($param, $value, $pdoType);
@@ -126,12 +139,86 @@ public function execute($params = null): ResultInterface
             );
         }
 
+        $resourceOffsets = $this->getResourceOffsets();
         try {
             $this->stmt->execute($params);
         } catch (PDOException $exception) {
             throw Exception::new($exception);
+        } finally {
+            if ($resourceOffsets !== null) {
+                $this->restoreResourceOffsets($resourceOffsets);
+            }
         }
 
         return new Result($this->stmt);
     }
+
+    /**
+     * Track a binary parameter reference at binding time. These
+     * are cached for later analysis by the getResourceOffsets.
+     *
+     * @param mixed $resource
+     */
+    private function trackParamResource($resource): void
+    {
+        if (! is_resource($resource)) {
+            return;
+        }
+
+        $this->paramResources ??= [];
+        $this->paramResources[] = $resource;
+    }
+
+    /**
+     * Determine the offset that any resource parameters needs to be
+     * restored to after the statement is executed. Call immediately
+     * before execute (not during bindValue) to get the most accurate offset.
+     *
+     * @return int[]|null Return offsets to restore if needed. The array may be sparse.
+     */
+    private function getResourceOffsets(): ?array
+    {
+        if ($this->paramResources === null) {
+            return null;
+        }
+
+        $resourceOffsets = null;
+        foreach ($this->paramResources as $index => $resource) {
+            $position = false;
+            if (stream_get_meta_data($resource)['seekable']) {
+                $position = ftell($resource);
+            }
+
+            if ($position === false) {
+                continue;
+            }
+
+            $resourceOffsets       ??= [];
+            $resourceOffsets[$index] = $position;
+        }
+
+        if ($resourceOffsets === null) {
+            $this->paramResources = null;
+        }
+
+        return $resourceOffsets;
+    }
+
+    /**
+     * Restore resource offsets moved by PDOStatement->execute
+     *
+     * @param int[]|null $resourceOffsets The offsets returned by getResourceOffsets.
+     */
+    private function restoreResourceOffsets(?array $resourceOffsets): void
+    {
+        if ($resourceOffsets === null || $this->paramResources === null) {
+            return;
+        }
+
+        foreach ($resourceOffsets as $index => $offset) {
+            fseek($this->paramResources[$index], $offset, SEEK_SET);
+        }
+
+        $this->paramResources = null;
+    }
 }

src/Driver/PgSQL/Statement.php

@@ -10,6 +10,8 @@
 use TypeError;
 
 use function assert;
+use function fseek;
+use function ftell;
 use function func_num_args;
 use function get_class;
 use function gettype;
@@ -26,6 +28,9 @@
 use function pg_send_execute;
 use function sprintf;
 use function stream_get_contents;
+use function stream_get_meta_data;
+
+use const SEEK_SET;
 
 final class Statement implements StatementInterface
 {
@@ -151,10 +156,24 @@ public function execute($params = null): Result
             switch ($this->parameterTypes[$parameter]) {
                 case ParameterType::BINARY:
                 case ParameterType::LARGE_OBJECT:
+                    $isResource = is_resource($value);
+                    $resource   = $value;
+                    $resetTo    = false;
+                    if ($isResource) {
+                        if (stream_get_meta_data($resource)['seekable']) {
+                            $resetTo = ftell($resource);
+                        }
+                    }
+
                     $escapedParameters[] = $value === null ? null : pg_escape_bytea(
                         $this->connection,
-                        is_resource($value) ? stream_get_contents($value) : $value,
+                        $isResource ? stream_get_contents($value) : $value,
                     );
+
+                    if ($resetTo !== false) {
+                        fseek($resource, $resetTo, SEEK_SET);
+                    }
+
                     break;
                 default:
                     $escapedParameters[] = $value;

tests/Functional/BlobTest.php

@@ -10,6 +10,9 @@
 use Doctrine\DBAL\Types\Types;
 
 use function fopen;
+use function fseek;
+use function ftell;
+use function fwrite;
 use function str_repeat;
 use function stream_get_contents;
 
@@ -198,6 +201,28 @@ public function testBlobBindingDoesNotOverwritePrevious(): void
         self::assertEquals(['test1', 'test2'], $actual);
     }
 
+    public function testBindValueResetsStream(): void
+    {
+        if (TestUtil::isDriverOneOf('oci8')) {
+            self::markTestIncomplete('The oci8 driver does not support stream resources as parameters');
+        }
+
+        $stmt = $this->connection->prepare(
+            "INSERT INTO blob_table(id, clobcolumn, blobcolumn) VALUES (1, 'ignored', ?)",
+        );
+
+        $stream = fopen('php://temp', 'rb+');
+        fwrite($stream, 'a test');
+        fseek($stream, 2);
+        $stmt->bindValue(1, $stream, ParameterType::LARGE_OBJECT);
+
+        $stmt->execute();
+
+        self::assertEquals(2, ftell($stream), 'Resource parameter should be reset to position before execute.');
+
+        $this->assertBlobContains('test');
+    }
+
     private function assertBlobContains(string $text): void
     {
         [, $blobValue] = $this->fetchRow();