[Jython-checkins] jython: Support "readall" on files reported as having size=0. Fixes #2358

[jim.baker]

https://hg.python.org/jython/rev/c8245364e407
changeset:   7842:c8245364e407
user:        Jim Baker <jim.baker at rackspace.com>
date:        Mon Dec 28 10:30:36 2015 -0700
summary:
  Support "readall" on files reported as having size=0. Fixes #2358

On Linux for files in the /proc virtual filesystem, nearly all files
have a reported size of zero. Previously the implementation of
"readall" functionality (eg, file.read()) depended on this size as
being accurate. Now if the size=0, Jython attempts to read the file in
chunks.

files:
  Lib/test/test_file_jy.py           |  15 +++++-
  src/org/python/core/io/FileIO.java |  47 ++++++++++++++++-
  2 files changed, 57 insertions(+), 5 deletions(-)


diff --git a/Lib/test/test_file_jy.py b/Lib/test/test_file_jy.py
--- a/Lib/test/test_file_jy.py
+++ b/Lib/test/test_file_jy.py
@@ -2,10 +2,10 @@
 
 Made for Jython.
 """
-from __future__ import with_statement
 import os
 import unittest
 from test import test_support
+from java.lang import System
 
 class FileTestCase(unittest.TestCase):
 
@@ -51,6 +51,19 @@
         f = open(test_support.TESTFN, 'w')      # should succeed, raised IOError (permission denied) prior to fix
         f.close()
 
+    # http://bugs.jython.org/issue2358
+    def test_read_empty_file(self):
+        f = open(test_support.TESTFN, 'w')
+        f.close()
+        f = open(test_support.TESTFN)
+        self.assertEqual(f.read(), '')
+
+    # http://bugs.jython.org/issue2358
+    @unittest.skipUnless(System.getProperty('os.name') == u'Linux', 'Linux required')
+    def test_can_read_proc_filesystem(self):
+        with open('/proc/{}/cmdline'.format(os.getpid())) as f:
+            self.assertIn('org.python.util.jython', f.read())
+
 
 def test_main():
     test_support.run_unittest(FileTestCase)
diff --git a/src/org/python/core/io/FileIO.java b/src/org/python/core/io/FileIO.java
--- a/src/org/python/core/io/FileIO.java
+++ b/src/org/python/core/io/FileIO.java
@@ -13,10 +13,11 @@
 import java.nio.ByteBuffer;
 import java.nio.channels.Channels;
 import java.nio.channels.FileChannel;
+import java.util.ArrayList;
+import java.util.List;
 
 import jnr.constants.platform.Errno;
 import jnr.posix.util.FieldAccess;
-import jnr.posix.util.Platform;
 import org.python.core.Py;
 import org.python.core.PyObject;
 import org.python.core.PyString;
@@ -281,7 +282,9 @@
      * Read until EOF with one readinto() call.
      *
      * Takes advantage of the fact that the underlying file's size is
-     * available.
+     * available. However, we have to special case if file size is 0,
+     * as seen in the /proc virtual file system - in this case we cannot
+     * assume the file is truly empty.
      *
      * @return {@inheritDoc}
      */
@@ -301,10 +304,46 @@
         if (toRead > Integer.MAX_VALUE) {
             throw Py.OverflowError("requested number of bytes is more than a Python string can "
                                    + "hold");
+        } else if (toRead == 0) {
+            // Support files that the underlying OS has labeled with size=0, even though they have content,
+            // eg, /proc files on Linux
+            return readallInChunks();
+        } else {
+            ByteBuffer all = ByteBuffer.allocate((int) toRead);
+            readinto(all);
+            all.flip();
+            return all;
+        }
+    }
+
+    private ByteBuffer readallInChunks() {
+        // assumes checks have been performed
+        final List<ByteBuffer> chunks = new ArrayList<>();
+        final int MAX_CHUNK_SIZE = 8192;
+        int length = 0;
+        while (true) {
+            final ByteBuffer chunk = ByteBuffer.allocate(MAX_CHUNK_SIZE);
+            readinto(chunk);
+            int chunkSize = chunk.position();
+            length += chunkSize;
+            chunk.flip();
+            chunks.add(chunk);
+            if (chunkSize < MAX_CHUNK_SIZE) {
+                break;
+            }
         }
 
-        ByteBuffer all = ByteBuffer.allocate((int)toRead);
-        readinto(all);
+        // given the size of MAX_CHUNK_SIZE, perhaps most or even all files in /proc,
+        // or similar virtual filesystems, if any, might fit into one chunk
+        if (chunks.size() == 1) {
+            return chunks.get(0);
+        }
+
+        // otherwise append together into a single ByteBuffer
+        final ByteBuffer all = ByteBuffer.allocate(length);
+        for (ByteBuffer chunk : chunks) {
+            all.put(chunk);
+        }
         all.flip();
         return all;
     }

-- 
Repository URL: https://hg.python.org/jython