From edda6a3e485c72965e3cad2f9d9d3fb036ca6ab0 Mon Sep 17 00:00:00 2001
From: Fred Emmott <fe@fb.com>
Date: Wed, 9 Dec 2020 16:47:57 -0800
Subject: [PATCH] Fix `readUntilAsync`/`genReadUntil` across chunk boundaries
 (#167)

Summary:
fixes hhvm/hsl-experimental#166

Pull Request resolved: https://github.com/hhvm/hsl-experimental/pull/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
---
 src/io/BufferedReader.php       |  9 ++++++---
 tests/io/BufferedReaderTest.php | 25 ++++++++++++++++++++++++-
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/src/io/BufferedReader.php b/src/io/BufferedReader.php
index f8a4e49..0176e38 100644
--- a/src/io/BufferedReader.php
+++ b/src/io/BufferedReader.php
@@ -12,7 +12,7 @@
 
 namespace HH\Lib\IO;
 
-use namespace HH\Lib\{IO, OS, Str};
+use namespace HH\Lib\{IO, Math, OS, Str};
 use namespace HH\Lib\_Private\_OS;
 
 /** Wrapper for `ReadHandle`s, with buffered line-based byte-based accessors.
@@ -109,15 +109,18 @@ public function read(?int $max_bytes = null): string {
     }
 
     do {
+      // + 1 as it would have been matched in the previous iteration if it
+      // fully fit in the chunk
+      $offset = Math\maxva(0, Str\length($buf) - $suffix_len + 1);
       $chunk = await $this->handle->readAsync();
       if ($chunk === '') {
         $this->buffer = $buf;
         return null;
       }
       $buf .= $chunk;
-    } while (!Str\contains($chunk, $suffix));
+      $idx = Str\search($buf, $suffix, $offset);
+    } while ($idx === null);
 
-    $idx = Str\search($buf, $suffix);
     invariant($idx !== null, 'Should not have exited loop without suffix');
     $this->buffer = Str\slice($buf, $idx + $suffix_len);
     return Str\slice($buf, 0, $idx);
diff --git a/tests/io/BufferedReaderTest.php b/tests/io/BufferedReaderTest.php
index d08acc8..d1e0607 100644
--- a/tests/io/BufferedReaderTest.php
+++ b/tests/io/BufferedReaderTest.php
@@ -8,7 +8,8 @@
  *
  */
 
-use namespace HH\Lib\{IO, OS, Vec};
+use namespace HH\Lib\{IO, OS, Str, Vec};
+use namespace HH\Lib\_Private\_IO;
 
 use function Facebook\FBExpect\expect; // @oss-enable
 use type Facebook\HackTest\HackTest; // @oss-enable
@@ -107,6 +108,28 @@ final class BufferedReaderTest extends HackTest {
     expect(await $r->readUntilAsync("FOO"))->toEqual("cd");
   }
 
+  public async function testReadUntilBufferBoundary(): Awaitable<void> {
+    // Intent is to test the case when the separator starts in one chunk, and
+    // ends in another, i.e.:
+    // - Str\length($padding) < chunk size
+    // - Str\length($padding.$separator) > chunk size
+    $padding = Str\repeat('a', _IO\DEFAULT_READ_BUFFER_SIZE - 1);
+    $separator = 'bc';
+
+    list($r, $w) = IO\pipe();
+    concurrent {
+      await async {
+        await $w->writeAllAsync($padding.$separator.'junk');
+        $w->close();
+      };
+      await async {
+        $br = new IO\BufferedReader($r);
+        expect(await $br->readUntilAsync($separator))->toEqual($padding);
+        $r->close();
+      };
+    }
+  }
+
   public async function testReadLineVsReadUntil(): Awaitable<void> {
     $r = new IO\BufferedReader(new IO\MemoryHandle("ab\ncd"));
     expect(await $r->readLineAsync())->toEqual('ab');