Skip to content
This repository was archived by the owner on Oct 1, 2023. It is now read-only.

Commit edda6a3

Browse files
fredemmottfacebook-github-bot
authored andcommitted
Fix readUntilAsync/genReadUntil across chunk boundaries (#167)
Summary: fixes #166 Pull Request resolved: #167 Test Plan: - added unit test - if I revert the code change, test fails - if I revert the code chaange and replace the `- 1` in the test with `- 2`, it passes Reviewed By: jjergus Differential Revision: D25438423 Pulled By: fredemmott fbshipit-source-id: 0c20c7b47c0bce00a1687a8b625165dc22707a43
1 parent 3d940fb commit edda6a3

File tree

2 files changed

+30
-4
lines changed

2 files changed

+30
-4
lines changed

src/io/BufferedReader.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
namespace HH\Lib\IO;
1414

15-
use namespace HH\Lib\{IO, OS, Str};
15+
use namespace HH\Lib\{IO, Math, OS, Str};
1616
use namespace HH\Lib\_Private\_OS;
1717

1818
/** Wrapper for `ReadHandle`s, with buffered line-based byte-based accessors.
@@ -109,15 +109,18 @@ public function read(?int $max_bytes = null): string {
109109
}
110110

111111
do {
112+
// + 1 as it would have been matched in the previous iteration if it
113+
// fully fit in the chunk
114+
$offset = Math\maxva(0, Str\length($buf) - $suffix_len + 1);
112115
$chunk = await $this->handle->readAsync();
113116
if ($chunk === '') {
114117
$this->buffer = $buf;
115118
return null;
116119
}
117120
$buf .= $chunk;
118-
} while (!Str\contains($chunk, $suffix));
121+
$idx = Str\search($buf, $suffix, $offset);
122+
} while ($idx === null);
119123

120-
$idx = Str\search($buf, $suffix);
121124
invariant($idx !== null, 'Should not have exited loop without suffix');
122125
$this->buffer = Str\slice($buf, $idx + $suffix_len);
123126
return Str\slice($buf, 0, $idx);

tests/io/BufferedReaderTest.php

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
*
99
*/
1010

11-
use namespace HH\Lib\{IO, OS, Vec};
11+
use namespace HH\Lib\{IO, OS, Str, Vec};
12+
use namespace HH\Lib\_Private\_IO;
1213

1314
use function Facebook\FBExpect\expect; // @oss-enable
1415
use type Facebook\HackTest\HackTest; // @oss-enable
@@ -107,6 +108,28 @@ final class BufferedReaderTest extends HackTest {
107108
expect(await $r->readUntilAsync("FOO"))->toEqual("cd");
108109
}
109110

111+
public async function testReadUntilBufferBoundary(): Awaitable<void> {
112+
// Intent is to test the case when the separator starts in one chunk, and
113+
// ends in another, i.e.:
114+
// - Str\length($padding) < chunk size
115+
// - Str\length($padding.$separator) > chunk size
116+
$padding = Str\repeat('a', _IO\DEFAULT_READ_BUFFER_SIZE - 1);
117+
$separator = 'bc';
118+
119+
list($r, $w) = IO\pipe();
120+
concurrent {
121+
await async {
122+
await $w->writeAllAsync($padding.$separator.'junk');
123+
$w->close();
124+
};
125+
await async {
126+
$br = new IO\BufferedReader($r);
127+
expect(await $br->readUntilAsync($separator))->toEqual($padding);
128+
$r->close();
129+
};
130+
}
131+
}
132+
110133
public async function testReadLineVsReadUntil(): Awaitable<void> {
111134
$r = new IO\BufferedReader(new IO\MemoryHandle("ab\ncd"));
112135
expect(await $r->readLineAsync())->toEqual('ab');

0 commit comments

Comments
 (0)