include-interceptor icon indicating copy to clipboard operation
include-interceptor copied to clipboard

proc_open() with "file" descriptor does not work with active interceptor

Open WalterWoshid opened this issue 4 years ago • 3 comments

I added a file to a composer package into the "autoload/files" config from composer.json. As soon as I setup the interceptor and run "composer update", I'm getting a "fseek(): Stream does not support seeking" exception.

I'm running a laravel application on PHP 8.1

This is my code:

$interceptor = new \Nikic\IncludeInterceptor\Interceptor(function (string $path) {
    if (str_contains($path, '/public/index.php')) {
        echo($path);
        exit();
    }

    return null;
});
$interceptor->setUp();

WalterWoshid avatar Nov 28 '21 02:11 WalterWoshid

Start of the trace:

Exception trace:
 () at /home/nikic/repos/test-3/vendor/nikic/include-interceptor/src/Stream.php:159
 Composer\Util\ErrorHandler::handle() at n/a:n/a
 fseek() at /home/nikic/repos/test-3/vendor/nikic/include-interceptor/src/Stream.php:159
 Nikic\IncludeInterceptor\Stream->stream_seek() at n/a:n/a
 proc_open() at phar:///home/nikic/repos/composer.phar/vendor/symfony/process/Process.php:285
 Symfony\Component\Process\Process->start() at phar:///home/nikic/repos/composer.phar/vendor/symfony/process/Process.php:196
 Symfony\Component\Process\Process->run() at phar:///home/nikic/repos/composer.phar/src/Composer/Util/ProcessExecutor.php:146
 Composer\Util\ProcessExecutor->doExecute() at phar:///home/nikic/repos/composer.phar/src/Composer/Util/ProcessExecutor.php:89
 Composer\Util\ProcessExecutor->executeTty() at phar:///home/nikic/repos/composer.phar/src/Composer/EventDispatcher/EventDispatcher.php:348
 Composer\EventDispatcher\EventDispatcher->executeTty() at phar:///home/nikic/repos/composer.phar/src/Composer/EventDispatcher/EventDispatcher.php:312

Though there is also some

dup2: Bad file descriptor
dup2: Bad file descriptor
dup2: Bad file descriptor

before it.

It looks like this happens with a /dev/tty stream, which is not seekable. I'm not sure why proc_open() is trying to seek, I think the real problem here is that the stream is not castable.

nikic avatar Nov 28 '21 15:11 nikic

Minimal test case:

<?php
require __DIR__ . '/vendor/autoload.php';

$interceptor = new \Nikic\IncludeInterceptor\Interceptor(function (string $path) {
    return null;
});
$interceptor->setUp();

proc_open('echo Test', [
    ['file', '/dev/tty', 'r'],
    ['file', '/dev/tty', 'w'],
    ['file', '/dev/tty', 'w'],
], $pipes);

Produces:

Warning: fseek(): Stream does not support seeking in /home/nikic/repos/include-interceptor/src/Stream.php on line 159

Warning: fseek(): Stream does not support seeking in /home/nikic/repos/include-interceptor/src/Stream.php on line 159

Warning: fseek(): Stream does not support seeking in /home/nikic/repos/include-interceptor/src/Stream.php on line 159

Warning: proc_open(): Unable to copy file descriptor 4 (for pipe) into file descriptor 0: Bad file descriptor in /home/nikic/repos/include-interceptor/test.php on line 13

nikic avatar Nov 28 '21 15:11 nikic

There seems to be multiple levels of brokenness here. The seeking issue in particular is caused by https://github.com/php/php-src/blob/f0dd79a7e415d8b1d4a3868a27975d6578c87961/main/streams/cast.c#L199-L205, which will not try to seek a non-seekable stream, but as the stream is nominally seekable we call fseek() which throws a warning. This is an edge case that affects /dev/tty streams in particular.

The bigger problem as far as proc_open() is concerned, is that the stream cast is performed with PHP_STREAM_CAST_RELEASE in https://github.com/php/php-src/blob/f0dd79a7e415d8b1d4a3868a27975d6578c87961/ext/standard/proc_open.c#L790, which means that the stream will be freed in https://github.com/php/php-src/blob/f0dd79a7e415d8b1d4a3868a27975d6578c87961/main/streams/cast.c#L286 with an instruction to retain the underlying handle. Unfortunately, the stream wrapper API doesn't really provide a way to handle this -- we just end up closing the outer stream, which will close the inner stream. What we'd effectively want is that the cast instead takes ownership of the inner stream, but that's just not possible with the current API.

Not really sure what to do here...

nikic avatar Nov 28 '21 15:11 nikic