Bug #72561 [Ver->Nab]: 5.2.2 regressed with incorrect seeking in stream wrappers

Edit report at https://bugs.php.net/bug.php?id=72561&edit=1 ID: 72561 Updated by: bukka@php.net Reported by: nazar at mokrynskyi dot com Summary: 5.2.2 regressed with incorrect seeking in stream wrappers -Status: Verified +Status: Not a bug Type: Bug Package: Streams related Operating System: Linux PHP Version: 7.1.0alpha1 Assigned To: bukka Block user comment: N Private report: N New Comment: So I have done some debugging and my previous analysis wasn't correct because SEEK_CUR is already converted here: https://github.com/php/php-src/blob/4a5d13e20562a4af0bca8fe2ebfe93649715b110/main/streams/streams.c#L1360-L1365 Obviously the issue here was the fseek returning -1 and last ftell returning 5. After some digging into it I finally found out that it's all about returning result from fseek in streamWrapper::stream_seek. The problem is that fseek return 0 on success and -1 on failure but stream_seek expect boolean result - true on success and false on failure. As you can see in the supplied code, it returns 0 which is converted to false. So the code should look like this instead: https://3v4l.org/baAsV (the difference is just returning $result === 0 instead of $result). All is documented correctly so this is just user code bug - not a bug in PHP though. Previous Comments: ------------------------------------------------------------------------ [2023-06-11 14:06:14] bukka@php.net I just had a closer look into this. What's happening here is that for buffered stream the seeking is done without calling wrapper sync only in some cases. The problem is that SEEK_CUR can be handled reliably only without calling the wrapper. However if the requested position is negative (that's most likely because the buffer can be moved so everything before does not need to exist) or it's in the position behind the buffer size, then it needs to call the wrapper. Obviously the user wrapper cannot know the right position. Only internal wrappers can know that position so they can work fine. So we cannot modify the offset in seek cur based on the read position (like in the example) because only user wrappers might be using that and internals wrappers can use the right position. I think the only thing that could be potentially done is converting SEEK_CUR to SEEK_SET which means changing offset to (ftell() + offset). I will need to think about it more and do some further checking first though. What could be also useful is to have a way how to access the stream resource in the wrapper. Although the API for that might be a bit challenging so will leave that one for later (just making a note in my TODO list). ------------------------------------------------------------------------ [2020-09-08 11:34:08] cmb@php.net I agree that this behavior is confusing at best, but it is documented. Stream reads are buffered by default, and fseek() does not call ::stream_seek() if the target position is already contained in the read buffer[1]. However, that behavior renders our streamwrapper example[2] moot, since it relies on an internal position property like the supplied reproduce scripts in this ticket. [1] <https://www.php.net/manual/en/streamwrapper.stream-seek.php#refsect1-streamwrapper.stream-seek-notes> [2] <https://www.php.net/manual/en/stream.streamwrapper.example-1.php> ------------------------------------------------------------------------ [2017-12-20 16:18:58] cmb@php.net Related To: Bug #75708 ------------------------------------------------------------------------ [2016-07-08 08:16:21] requinix@php.net There's some weird magic going on, alright. https://3v4l.org/0oYBR#v550 1. stream_read gets called with the full 8192, implying PHP is automatically buffering the stream. This could be intentional (probably is) and may simply be an issue of documentation, but as a developer I would prefer PHP didn't do that for me. And I'll bet that 99% of stream wrapper code out there expects the method to be called with the specific length instead of the maximum length. 2. stream_tell is not being called. At all. The docs say it should be called when using fseek(), and one would expect it to be called with ftell() but apparently that's not the case, however it looks like PHP is managing the pointer position all by itself. 3. read 3 seek -1 results in ftell()=3 when it should be =2. Then later the read 2 results in ftell()=5 (should be =4). Seems that the relative seek isn't updating the internal pointer? > I believe this should be fixed and backported to stable versions, So we're clear, that would be 5.6.x, 7.0.y, and 7.1.z. Because 5.5 is dead and there's no way any changes would get into versions earlier than that. ------------------------------------------------------------------------ [2016-07-08 01:33:35] nazar at mokrynskyi dot com Description: ------------ When doing reading/seeking on stream wrapper, PHP does interesting magic under the hood (for instance, reading 3 bytes results in 8192 bytes actually tried to be read in ::stream_read()). This is all fine until it works as expected. Here is demo: https://3v4l.org/3RG7r PHP 5.1.4 - 5.2.1 have expected correct output. hhvm-3.9.1 - 3.12.0 seems to also have optimizations under the hood, but result is correct anyway. PHP 5.2.2+ (including latest 7.1.0-alpha2) regressed and does incorrect seeking which results in wrong position eventually. I believe this should be fixed and backported to stable versions, since I do not see any good workaround here, input in ::stream_seek() is already incorrect. ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=72561&edit=1