Fix SEGV at read/pread on null StringIO

Author: nobu

ext/java/org/jruby/ext/stringio/StringIO.java

@@ -655,8 +655,12 @@ public IRubyObject eof(ThreadContext context) {
     }
 
     private boolean isEndOfString() {
+        return isOutside(ptr.pos);
+    }
+
+    private boolean isOutside(int pos) {
         StringIOData ptr = getPtr();
-        return ptr.string == null || ptr.pos >= ptr.string.size();
+        return ptr.string == null || pos >= ptr.string.size();
     }
 
     @JRubyMethod(name = "getc")
@@ -1107,9 +1111,9 @@ private IRubyObject readCommon(ThreadContext context, int argc, IRubyObject arg0
                         if (len < 0) {
                             throw runtime.newArgumentError("negative length " + len + " given");
                         }
-                        if (len > 0 && isEndOfString()) {
+                        if (isEndOfString()) {
                             if (!str.isNil()) ((RubyString) str).resize(0);
-                            return context.nil;
+                            return len > 0 ? context.nil : runtime.newString();
                         }
                         binary = true;
                         break;
@@ -1224,8 +1228,7 @@ private RubyString preadCommon(ThreadContext context, int argc, IRubyObject arg0
                 throw runtime.newErrnoEINVALError("pread: Invalid offset argument");
             }
 
-            RubyString myString = ptr.string;
-            if (offset >= myString.size()) {
+            if (isOutside(offset)) {
                 throw context.runtime.newEOFError();
             }
 
@@ -1234,6 +1237,7 @@ private RubyString preadCommon(ThreadContext context, int argc, IRubyObject arg0
             }
 
             string = (RubyString) str;
+            RubyString myString = ptr.string;
             int rest = myString.size() - offset;
             if (len > rest) len = rest;
             string.resize(len);

ext/stringio/stringio.c

@@ -203,6 +203,18 @@ check_modifiable(struct StringIO *ptr)
     }
 }
 
+static inline bool
+outside_p(struct StringIO *ptr, long pos)
+{
+    return NIL_P(ptr->string) || pos >= RSTRING_LEN(ptr->string);
+}
+
+static inline bool
+eos_p(struct StringIO *ptr)
+{
+    return outside_p(ptr, ptr->pos);
+}
+
 static VALUE
 strio_s_allocate(VALUE klass)
 {
@@ -628,9 +640,8 @@ static struct StringIO *
 strio_to_read(VALUE self)
 {
     struct StringIO *ptr = readable(self);
-    if (NIL_P(ptr->string)) return NULL;
-    if (ptr->pos < RSTRING_LEN(ptr->string)) return ptr;
-    return NULL;
+    if (eos_p(ptr)) return NULL;
+    return ptr;
 }
 
 /*
@@ -910,7 +921,7 @@ strio_getc(VALUE self)
     int len;
     char *p;
 
-    if (NIL_P(str) || pos >= RSTRING_LEN(str)) {
+    if (eos_p(ptr)) {
 	return Qnil;
     }
     p = RSTRING_PTR(str)+pos;
@@ -931,7 +942,7 @@ strio_getbyte(VALUE self)
 {
     struct StringIO *ptr = readable(self);
     int c;
-    if (NIL_P(ptr->string) || ptr->pos >= RSTRING_LEN(ptr->string)) {
+    if (eos_p(ptr)) {
 	return Qnil;
     }
     c = RSTRING_PTR(ptr->string)[ptr->pos++];
@@ -1609,10 +1620,9 @@ strio_read(int argc, VALUE *argv, VALUE self)
 	    if (len < 0) {
 		rb_raise(rb_eArgError, "negative length %ld given", len);
 	    }
-	    if (len > 0 &&
-		(NIL_P(ptr->string) || ptr->pos >= RSTRING_LEN(ptr->string))) {
+	    if (eos_p(ptr)) {
 		if (!NIL_P(str)) rb_str_resize(str, 0);
-		return Qnil;
+		return len > 0 ? Qnil : rb_str_new(0, 0);
 	    }
 	    binary = 1;
 	    break;
@@ -1688,7 +1698,7 @@ strio_pread(int argc, VALUE *argv, VALUE self)
 
     struct StringIO *ptr = readable(self);
 
-    if (offset >= RSTRING_LEN(ptr->string)) {
+    if (outside_p(ptr, offset)) {
 	rb_eof_error();
     }
 

test/stringio/test_stringio.rb

@@ -70,6 +70,16 @@ def test_null
     assert_nil io.getc
   end
 
+  def test_pread_null
+    io = StringIO.new(nil)
+    assert_raise(EOFError) { io.pread(1, 0) }
+  end
+
+  def test_read_null
+    io = StringIO.new(nil)
+    assert_equal "", io.read(0)
+  end
+
   def test_seek_null
     io = StringIO.new(nil)
     assert_equal(0, io.seek(0, IO::SEEK_SET))