Lucene - Core
  1. Lucene - Core
  2. LUCENE-5160

NIOFSDirectory, SimpleFSDirectory (others?) don't properly handle valid file and FileChannel read conditions

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.4, 6.0
    • Fix Version/s: 4.5, 6.0
    • Component/s: None
    • Labels:
      None

      Description

      Around line 190 of NIOFSDirectory, the loop to read in bytes doesn't properly handle the -1 condition that can be returned from FileChannel.read(). If it returns -1, then it will move the file pointer back and you will enter an infinite loop. SimpleFSDirectory displays the same characteristics, although I have only seen the issue on NIOFSDirectory.

      The code in question from NIOFSDirectory:

      try {
              while (readLength > 0) {
                final int limit;
                if (readLength > chunkSize) {
                  // LUCENE-1566 - work around JVM Bug by breaking
                  // very large reads into chunks
                  limit = readOffset + chunkSize;
                } else {
                  limit = readOffset + readLength;
                }
                bb.limit(limit);
                int i = channel.read(bb, pos);
                pos += i;
                readOffset += i;
                readLength -= i;
              }
      
      1. LUCENE-5160.patch
        2 kB
        Grant Ingersoll

        Issue Links

          Activity

          Hide
          Uwe Schindler added a comment -

          This is a bug, which never is hit by lucene because we never read sequentially until end of file.

          +1 to fix this. Theoretically to comply with MMapDirectory it should throw EOFException if it gets -1, because Lucene code should not read beyond file end.

          Show
          Uwe Schindler added a comment - This is a bug, which never is hit by lucene because we never read sequentially until end of file. +1 to fix this. Theoretically to comply with MMapDirectory it should throw EOFException if it gets -1, because Lucene code should not read beyond file end.
          Hide
          Grant Ingersoll added a comment -

          Patch adds the -1 check and throws an EOF

          Show
          Grant Ingersoll added a comment - Patch adds the -1 check and throws an EOF
          Hide
          Uwe Schindler added a comment -

          +1 to commit. Looks good. Writing a test is a bit hard.

          MMapDirectory is not affected as it already has a check for the length of the MappedByteBuffers.

          Show
          Uwe Schindler added a comment - +1 to commit. Looks good. Writing a test is a bit hard. MMapDirectory is not affected as it already has a check for the length of the MappedByteBuffers.
          Hide
          ASF subversion and git services added a comment -

          Commit 1512011 from Grant Ingersoll in branch 'dev/trunk'
          [ https://svn.apache.org/r1512011 ]

          LUCENE-5160: check for -1 return conditions in file reads

          Show
          ASF subversion and git services added a comment - Commit 1512011 from Grant Ingersoll in branch 'dev/trunk' [ https://svn.apache.org/r1512011 ] LUCENE-5160 : check for -1 return conditions in file reads
          Hide
          ASF subversion and git services added a comment -

          Commit 1512016 from Grant Ingersoll in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1512016 ]

          LUCENE-5160: merge from trunk

          Show
          ASF subversion and git services added a comment - Commit 1512016 from Grant Ingersoll in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1512016 ] LUCENE-5160 : merge from trunk
          Hide
          ASF subversion and git services added a comment -

          Commit 1512728 from Robert Muir in branch 'dev/trunk'
          [ https://svn.apache.org/r1512728 ]

          LUCENE-5160: move CHANGES entry to the correct version

          Show
          ASF subversion and git services added a comment - Commit 1512728 from Robert Muir in branch 'dev/trunk' [ https://svn.apache.org/r1512728 ] LUCENE-5160 : move CHANGES entry to the correct version
          Hide
          Adrien Grand added a comment -

          4.5 release -> bulk close

          Show
          Adrien Grand added a comment - 4.5 release -> bulk close

            People

            • Assignee:
              Grant Ingersoll
              Reporter:
              Grant Ingersoll
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development