Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

putContents doesn't truncate the file like file_put_contents #63

Closed
choval opened this issue Feb 19, 2019 · 15 comments · Fixed by #80
Closed

putContents doesn't truncate the file like file_put_contents #63

choval opened this issue Feb 19, 2019 · 15 comments · Fixed by #80

Comments

@choval
Copy link

choval commented Feb 19, 2019

React\Filesystem\Node\File::putContents @ line 158 opens without truncating option t. Hence, it writes the length of the content and leaves previous existing bytes.

It kinda goes against the regular behaviour of file_put_contents, is this intended?

$file = 'tmp.txt';
file_put_contents( $file, '000');
$promise = $fs->file( $file )->putContents( '11' );
$res = Block\await($promise, $loop);
var_dump( file_get_contents( $file ));
// returns 110 instead of 11
@choval
Copy link
Author

choval commented Feb 19, 2019

The open('cwt') described in the README does not work as expected, but instead open('w') behaves the way documented (creates when does not exist & truncates).

@WyriHaximus
Copy link
Member

That's not the intention, have you tried out the changes in #62?

@choval
Copy link
Author

choval commented Feb 26, 2019

Yes, tried with c98de2f and still the same.

@WyriHaximus
Copy link
Member

Will look into it, have some PR planned for filesystem and will fix this with those as well

@ghost
Copy link

ghost commented Mar 7, 2019

c describes that if the file exists, it will not be truncated. w will automatically attempt to create the file if it does not exist, and otherwise truncate it.

'w'
Open for writing only; place the file pointer at the
beginning of the file and truncate the file to zero length.
If the file does not exist, attempt to create it. 
'c'
Open the file for writing only. If the file does not exist,
it is created. If it exists, it is neither truncated (as opposed to 'w'),
nor the call to this function fails (as is the case with 'x').

https://secure.php.net/manual/en/function.fopen.php
(The behaviour of PHP open flags are effectively not the same to the POSIX ones.)

If you're using Eio, this might look a bit differently.

But this unit test on a separate branch (splitted from the PR with added truncate unit test) shows the file is truncated before writing:
https://github.com/CharlotteDunoisLabs/filesystem/compare/file-put-get-rocket...CharlotteDunoisLabs:file-test-truncate
https://travis-ci.org/CharlotteDunoisLabs/filesystem/builds/503252599

(The only build passing at the time of writing this comment was PHP 5.6, the others were restarted due to typical Eio timeouts and are unrelated to the described issue.)

@developernaren
Copy link

@WyriHaximus @choval @CharlotteDunois any update on this?

@ghost
Copy link

ghost commented Apr 25, 2020

@developernaren I'm not sure where we left off, can you reproduce the issue? Maybe the File::putContents method should be updated to use AdapterInterface::putContents instead of a normal open-write cycle?

@developernaren
Copy link

@CharlotteDunois it has the exact behavior as the original code in the first comment.

$file = 'tmp.txt';
file_put_contents( $file, '000');
$promise = $fs->file( $file )->putContents( '11' );
$res = Block\await($promise, $loop);
var_dump( file_get_contents( $file ));
// returns 110 instead of 11

@ghost
Copy link

ghost commented Apr 25, 2020

@developernaren Do you have the same behaviour with

$promise = $fs->getAdapter()->putContents($file, '11');

instead?

@developernaren
Copy link

@CharlotteDunois there is no putContents in AdapterInterface. I tried open method with cwt flag to truncate and write but that is also not working. I am having to delete the file and write again as a work around.

@ghost
Copy link

ghost commented Apr 25, 2020

@developernaren There is in the master branch. 00ed432

@developernaren
Copy link

I am trying to use it in this framework https://driftphp.io/. I am not exactly sure how to fix use master as this is the dependency of a dependency.

@choval
Copy link
Author

choval commented Apr 25, 2020

@developernaren I'll try the master branch tonite and get back to you.

@choval
Copy link
Author

choval commented Apr 25, 2020

Still the same using the latest master 3133bd7, which includes 00ed432.

If you use open('w') instead of open('cw') on the src/Node/File.php@158 of the current master, this behaves as expected.

https://github.com/reactphp/filesystem/blob/master/src/Node/File.php

From:

        return $this->open('cw')->then(function (WritableStreamInterface $stream) use ($contents) {

To:

        return $this->open('w')->then(function (WritableStreamInterface $stream) use ($contents) {

I no longer use this library. Good luck.

Test:

#!/usr/bin/env php
<?php
require('vendor/autoload.php');

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

$file = 'tmp.txt';
file_put_contents( $file, 'xx');
$promise = $fs->file( $file )->putContents( 'o' );
$promise->done(
    function($r) use ($file) {
        $data = file_get_contents($file);
        if ($data === 'o') {
            echo "GOOD!\n";
        } else {
            echo "Expected 'o', but got 'xx' \n";
        }
    },
    function($e) {
        echo $e->getMessage()."\n";
    });

$loop->run();

@developernaren
Copy link

@choval thank you so much for trying this out. really appreciate the effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants