reactphp / filesystem

Evented filesystem access.
MIT License
135 stars 40 forks source link

ChildProcessAdapter cannot read/write binary files #38

Closed jmoo closed 6 years ago

jmoo commented 6 years ago

This is caused by WyriHaximus/reactphp-child-process-messenger use of json_encode/json_decode which does not support binary strings.

I'd make a pull request but I'm not sure if this should be fixed here by (probably base64) encoding the payloads or if it should be fixed in WyriHaximus/reactphp-child-process-messenger by moving to a binary format for rpc

Example:

$loop = \React\EventLoop\Factory::create();
$filesystem = \React\Filesystem\Filesystem::create($loop);

$contents = base64_decode('UEsDBAoAAAAAAEw400'); // start of zip header
$filename = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'bin.data';

echo 'Using ', get_class($filesystem->getAdapter()), PHP_EOL;
$filesystem->file($filename)->putContents($contents)->then(function ($contents) use ($filename) {
    echo file_get_contents($filename), PHP_EOL;
}, function (Exception $e) {
    echo $e->getMessage(), PHP_EOL;
    echo $e->getTraceAsString(), PHP_EOL;
});

$loop->run();
WyriHaximus commented 6 years ago

Hey @jmoo good catch! I've been working with msgpack as well lately and I'll try updatting wyrihaximus/react-child-process-messenger to use msgpack locally and see if it works.

ghost commented 6 years ago

Wouldn't serialize/json_encode & base64_encode and base64_decode & unserialize/json_decode solve the issue? This way you wouldn't have to require an external package or pecl extension to encode and decode payloads.

Whether you use JSON or serialize depends on the use case. I'm not sure about the performance difference though.

WyriHaximus commented 6 years ago

Yeah that makes sense, haven't had time yet with the 1.0 releases earlier this week. But will have a go at it tonight 👍

WyriHaximus commented 6 years ago

FYI #39 is up fixing this issue