You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by dw...@apache.org on 2021/02/02 08:09:26 UTC

[lucene-solr] branch master updated: LUCENE-9686: Fix read past EOF handling in DirectIODirectory (#2258)

This is an automated email from the ASF dual-hosted git repository.

dweiss pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/master by this push:
     new 3835cb4  LUCENE-9686: Fix read past EOF handling in DirectIODirectory (#2258)
3835cb4 is described below

commit 3835cb4e95ce6ba93ab5e3d5caa35001c90db30a
Author: zacharymorn <za...@yahoo.com>
AuthorDate: Tue Feb 2 00:07:30 2021 -0800

    LUCENE-9686: Fix read past EOF handling in DirectIODirectory (#2258)
---
 .../lucene/misc/store/DirectIODirectory.java       | 17 +++---
 .../lucene/misc/store/TestDirectIODirectory.java   | 60 ++++++++++++++++++++++
 2 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/lucene/misc/src/java/org/apache/lucene/misc/store/DirectIODirectory.java b/lucene/misc/src/java/org/apache/lucene/misc/store/DirectIODirectory.java
index 0f95a9a..8676668 100644
--- a/lucene/misc/src/java/org/apache/lucene/misc/store/DirectIODirectory.java
+++ b/lucene/misc/src/java/org/apache/lucene/misc/store/DirectIODirectory.java
@@ -359,12 +359,8 @@ public class DirectIODirectory extends FilterDirectory {
         filePos = alignedPos - buffer.capacity();
 
         final int delta = (int) (pos - alignedPos);
-        refill();
-        try {
-          buffer.position(delta);
-        } catch (IllegalArgumentException iae) {
-          throw new EOFException("read past EOF: " + this);
-        }
+        refill(delta);
+        buffer.position(delta);
       }
       assert pos == getFilePointer();
     }
@@ -381,17 +377,18 @@ public class DirectIODirectory extends FilterDirectory {
     @Override
     public byte readByte() throws IOException {
       if (!buffer.hasRemaining()) {
-        refill();
+        refill(1);
       }
+
       return buffer.get();
     }
 
-    private void refill() throws IOException {
+    private void refill(int bytesToRead) throws IOException {
       filePos += buffer.capacity();
 
       // BaseDirectoryTestCase#testSeekPastEOF test for consecutive read past EOF,
       // hence throwing EOFException early to maintain buffer state (position in particular)
-      if (filePos > channel.size()) {
+      if (filePos > channel.size() || (channel.size() - filePos < bytesToRead)) {
         throw new EOFException("read past EOF: " + this);
       }
 
@@ -417,7 +414,7 @@ public class DirectIODirectory extends FilterDirectory {
           buffer.get(dst, offset, left);
           toRead -= left;
           offset += left;
-          refill();
+          refill(toRead);
         } else {
           buffer.get(dst, offset, toRead);
           break;
diff --git a/lucene/misc/src/test/org/apache/lucene/misc/store/TestDirectIODirectory.java b/lucene/misc/src/test/org/apache/lucene/misc/store/TestDirectIODirectory.java
index 25279ff..1c11782 100644
--- a/lucene/misc/src/test/org/apache/lucene/misc/store/TestDirectIODirectory.java
+++ b/lucene/misc/src/test/org/apache/lucene/misc/store/TestDirectIODirectory.java
@@ -16,6 +16,8 @@
  */
 package org.apache.lucene.misc.store;
 
+import com.carrotsearch.randomizedtesting.RandomizedTest;
+import java.io.EOFException;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -77,10 +79,68 @@ public class TestDirectIODirectory extends BaseDirectoryTestCase {
       o.close();
       IndexInput i = dir.openInput("out", newIOContext(random()));
       i.seek(fileSize);
+
+      // Seeking past EOF should always throw EOFException
+      expectThrows(
+          EOFException.class, () -> i.seek(fileSize + RandomizedTest.randomIntBetween(1, 2048)));
+
+      // Reading immediately after seeking past EOF should throw EOFException
+      expectThrows(EOFException.class, () -> i.readByte());
       i.close();
     }
   }
 
+  public void testReadPastEOFShouldThrowEOFExceptionWithEmptyFile() throws Exception {
+    // fileSize needs to be 0 to test this condition. Do not randomized.
+    final int fileSize = 0;
+    try (Directory dir = getDirectory(createTempDir("testReadPastEOF"))) {
+      try (IndexOutput o = dir.createOutput("out", newIOContext(random()))) {
+        o.writeBytes(new byte[fileSize], 0, fileSize);
+      }
+
+      try (IndexInput i = dir.openInput("out", newIOContext(random()))) {
+        i.seek(fileSize);
+        expectThrows(EOFException.class, () -> i.readByte());
+        expectThrows(EOFException.class, () -> i.readBytes(new byte[1], 0, 1));
+      }
+
+      try (IndexInput i = dir.openInput("out", newIOContext(random()))) {
+        expectThrows(
+            EOFException.class, () -> i.seek(fileSize + RandomizedTest.randomIntBetween(1, 2048)));
+        expectThrows(EOFException.class, () -> i.readByte());
+        expectThrows(EOFException.class, () -> i.readBytes(new byte[1], 0, 1));
+      }
+
+      try (IndexInput i = dir.openInput("out", newIOContext(random()))) {
+        expectThrows(EOFException.class, () -> i.readByte());
+      }
+
+      try (IndexInput i = dir.openInput("out", newIOContext(random()))) {
+        expectThrows(EOFException.class, () -> i.readBytes(new byte[1], 0, 1));
+      }
+    }
+  }
+
+  public void testSeekPastEOFAndRead() throws Exception {
+    try (Directory dir = getDirectory(createTempDir("testSeekPastEOF"))) {
+      final int len = random().nextInt(2048);
+
+      try (IndexOutput o = dir.createOutput("out", newIOContext(random()))) {
+        byte[] b = new byte[len];
+        o.writeBytes(b, 0, len);
+      }
+
+      try (IndexInput i = dir.openInput("out", newIOContext(random()))) {
+        // Seeking past EOF should always throw EOFException
+        expectThrows(
+            EOFException.class, () -> i.seek(len + RandomizedTest.randomIntBetween(1, 2048)));
+
+        // Reading immediately after seeking past EOF should throw EOFException
+        expectThrows(EOFException.class, () -> i.readByte());
+      }
+    }
+  }
+
   public void testUseDirectIODefaults() throws Exception {
     Path path = createTempDir("testUseDirectIODefaults");
     try (DirectIODirectory dir = new DirectIODirectory(FSDirectory.open(path))) {