Fix r+ permission writeable check #77
Conversation
src/Internal/QueuedWritesFile.php
Outdated
| @@ -38,7 +38,7 @@ public function __construct( | |||
| } | |||
|
|
|||
| $this->queue = new \SplQueue(); | |||
| $this->writable = $this->mode[0] !== 'r'; | |||
| $this->writable = $this->mode[0] !== 'r' || $this->mode == 'r+'; | |||
There was a problem hiding this comment.
This likely needs normalization like in other places and can then just check for $this->mode !== 'r'?
See $mode = \str_replace(['b', 't', 'e'], '', $mode); in other places.
There was a problem hiding this comment.
Line 78 in efeb737
The openFile is amphp function. Maybe I am wrong, but AS I can understand, IF it possible to create new file with amphp library with "r+" permission, so the library will do the rest -> create it, write it, ...
If I am wrong could you please explain to me why. Thanks!
There was a problem hiding this comment.
@yurganov I think you didn't understand correctly: The new check is better than the old one, but still potentially incorrect, e.g. for rb+ (at least I think that works). So it should normalize the mode with the mentioned function and then check just for !== 'r', as that's the only mode that is non-writable.
There was a problem hiding this comment.
Hey @kelunik, absolutely, I overlooked the 'rb' option. I believe it's simpler to validate only for 'r' and 'rb. Please check my commit.
There was a problem hiding this comment.
@yurganov I still think we should remove b, t, and e from the mode, as they're just flags and that can be present in any order IIRC.
The r+ mode for opening a file is similar to r mode but has some added features. It opens the file in both read and write mode.