Skip to content

Commit edc3416

Browse files
authored
FileReadStream improvement (#184)
Address [this comment](WordPress/wordpress-importer#202) from @brandonpayton by moving the assignment after the check: > If this seek fails, bytes_already_forgotten will be left at the invalid target offset. Is that desired?
1 parent f1b965e commit edc3416

File tree

2 files changed

+63
-3
lines changed

2 files changed

+63
-3
lines changed

components/ByteStream/ReadStream/class-filereadstream.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,14 @@ protected function internal_pull( $n ): string {
5656
}
5757

5858
protected function seek_outside_of_buffer( int $target_offset ): void {
59+
$retval = fseek( $this->file_pointer, $target_offset );
60+
if ( -1 === $retval ) {
61+
throw new ByteStreamException( 'Failed to seek to offset' );
62+
}
63+
5964
$this->buffer = '';
6065
$this->offset_in_current_buffer = 0;
6166
$this->bytes_already_forgotten = $target_offset;
62-
if ( false === fseek( $this->file_pointer, $target_offset ) ) {
63-
throw new ByteStreamException( 'Failed to seek to offset' );
64-
}
6567
}
6668

6769
public function close_reading(): void {
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
use PHPUnit\Framework\TestCase;
4+
use WordPress\ByteStream\ByteStreamException;
5+
use WordPress\ByteStream\ReadStream\FileReadStream;
6+
7+
class FileReadStreamTest extends TestCase {
8+
9+
public function testStreamRemainsUsableAfterFailedFseek() {
10+
// Create a custom FileReadStream that simulates a non-seekable resource
11+
$content = str_repeat('ABCDEFGHIJ', 10); // 100 characters
12+
13+
// Create a memory stream with the content
14+
$resource = fopen('php://memory', 'r+');
15+
fwrite($resource, $content);
16+
rewind($resource);
17+
18+
// Create a custom FileReadStream where seek_outside_of_buffer always fails
19+
$stream = new class($resource, strlen($content)) extends FileReadStream {
20+
public function seek( int $target_offset ): void {
21+
// Skip all the sanity checks and pass the offset directly to seek_outside_of_buffer
22+
$this->seek_outside_of_buffer($target_offset);
23+
}
24+
};
25+
26+
try {
27+
// Go to a place in the buffer where reading 20 bytes will yield a different
28+
// result than reading 20 bytes from the beginning of the file
29+
$stream->pull(8);
30+
$stream->consume(8);
31+
$positionBefore = $stream->tell();
32+
33+
// Try to seek to the beginning - this will fail in seek_outside_of_buffer
34+
// because 0 is now outside the buffer range (bytes_already_forgotten > 0)
35+
try {
36+
$stream->seek(-1);
37+
$this->fail('Expected ByteStreamException was not thrown');
38+
} catch (ByteStreamException $e) {
39+
$this->assertEquals('Failed to seek to offset', $e->getMessage());
40+
}
41+
42+
// Verify stream position hasn't changed and stream is still usable
43+
$this->assertEquals($positionBefore, $stream->tell());
44+
45+
// Should be able to continue reading from current position
46+
$stream->pull(20);
47+
$nextData = $stream->consume(20);
48+
$expectedData = substr($content, $positionBefore, 20);
49+
$this->assertEquals($expectedData, $nextData);
50+
51+
} finally {
52+
// Clean up resource
53+
if (is_resource($resource)) {
54+
fclose($resource);
55+
}
56+
}
57+
}
58+
}

0 commit comments

Comments
 (0)