Merge pull request #806 from eregon/fix-write_string

Fix FFI::Pointer#write_string to always terminate with a \0 byte

ext/ffi_c/AbstractMemory.c

@@ -499,7 +499,7 @@ memory_read_array_of_string(int argc, VALUE* argv, VALUE self)
  * @raise {SecurityError} when writing unsafe string to memory
  * @raise {IndexError} if +offset+ is too great
  * @raise {NullPointerError} if memory not initialized
- * Put a string in memory.
+ * Put a string in memory. Writes a final \0 byte.
  */
 static VALUE
 memory_put_string(VALUE self, VALUE offset, VALUE str)

lib/ffi/pointer.rb

@@ -85,23 +85,37 @@ def read_string_to_null
     # @param [String] str string to write
     # @param [Numeric] len length of string to return
     # @return [self]
-    # Write +len+ first bytes of +str+ in pointer's contents.
+    # Write +len+ first bytes of +str+ in pointer's contents and a final \0 byte.
     #
     # Same as:
     #  ptr.write_string(str, len)   # with len not nil
     def write_string_length(str, len)
-      put_bytes(0, str, 0, len)
+      write_string(str, len)
     end unless method_defined?(:write_string_length)
 
     # @param [String] str string to write
     # @param [Numeric] len length of string to return
     # @return [self]
-    # Write +str+ in pointer's contents, or first +len+ bytes if 
-    # +len+ is not +nil+.
+    # Write +str+ in pointer's contents.
+    # If +len+ is given, write the first +len+ bytes of +str+.
+    # In both cases a final \0 byte is written after the string.
     def write_string(str, len=nil)
-      len = str.bytesize unless len
-      # Write the string data without NUL termination
-      put_bytes(0, str, 0, len)
+      if len
+        if len == size
+          warn "[DEPRECATION] Memory too small to write a final 0-byte in #{caller(1, 1)[0]}. This will raise an error in ffi-2.0. Please use write_bytes instead or enlarge the memory region."
+          write_bytes(str, 0, len)
+        else
+          put_char(len, 0) # Check size before writing str
+          write_bytes(str, 0, len)
+        end
+      else
+        if str.bytesize == size
+          warn "[DEPRECATION] Memory too small to write a final 0-byte in #{caller(1, 1)[0]}. This will raise an error in ffi-2.0. Please use write_bytes instead or enlarge the memory region."
+          write_bytes(str)
+        else
+          put_string(0, str)
+        end
+      end
     end unless method_defined?(:write_string)
 
     # @param [Type] type type of data to read from pointer's contents

spec/ffi/string_spec.rb

@@ -98,4 +98,90 @@ module StrLibTest
     ptrary.write_array_of_pointer(ary)
     expect { ptrary.get_array_of_string(-1) }.to raise_error(IndexError)
   end
+
+  describe "#write_string" do
+    # https://github.com/ffi/ffi/issues/805
+    describe "with no length given" do
+      it "writes a final \\0" do
+        ptr = FFI::MemoryPointer.new(8)
+        ptr.write_int64(-1)
+        ptr.write_string("äbc")
+        expect(ptr.read_bytes(5)).to eq("äbc\x00".b)
+        expect(ptr.read_string).to eq("äbc".b)
+      end
+
+      it "doesn't write anything when size is exceeded" do
+        ptr = FFI::MemoryPointer.new(8)
+        ptr.write_int64(-1)
+        expect do
+          ptr.write_string("äbcdefgh")
+        end.to raise_error(IndexError, /out of bounds/i)
+        expect(ptr.read_int64).to eq(-1)
+      end
+
+      if FFI::VERSION < "2"
+        it "prints a warning if final \\0 doesn't fit into memory" do
+          ptr = FFI::MemoryPointer.new(5)
+          expect do
+            ptr.write_string("äbcd")
+          end.to output(/memory too small/i).to_stderr
+          expect(ptr.read_string).to eq("äbcd".b)
+        end
+      else
+        it "denies writing if final \\0 doesn't fit into memory" do
+          ptr = FFI::MemoryPointer.new(5)
+          expect do
+            ptr.write_string("äbcd")
+          end.to raise_error(IndexError, /out of bounds/i)
+          expect(ptr.read_string).to eq("".b)
+        end
+      end
+    end
+
+    describe "with a length" do
+      it "writes a final \\0" do
+        ptr = FFI::MemoryPointer.new(8)
+        ptr.write_int64(-1)
+        ptr.write_string("äbcd", 3)
+        expect(ptr.read_bytes(5)).to eq("äb\x00\xFF".b)
+      end
+
+      it "doesn't write anything when size is exceeded" do
+        ptr = FFI::MemoryPointer.new(8)
+        ptr.write_int64(-1)
+        expect do
+          ptr.write_string("äbcdefghi", 9)
+        end.to raise_error(IndexError, /out of bounds/i)
+        expect(ptr.read_int64).to eq(-1)
+      end
+
+      if FFI::VERSION < "2"
+        it "prints a warning if final \\0 doesn't fit into memory" do
+          ptr = FFI::MemoryPointer.new(5)
+          expect do
+            ptr.write_string("äbcde", 5)
+          end.to output(/memory too small/i).to_stderr
+          expect(ptr.read_string).to eq("äbcd".b)
+        end
+      else
+        it "denies writing if final \\0 doesn't fit into memory" do
+          ptr = FFI::MemoryPointer.new(5)
+          expect do
+            ptr.write_string("äbcde", 5)
+          end.to raise_error(IndexError, /out of bounds/i)
+          expect(ptr.read_string).to eq("".b)
+        end
+      end
+    end
+  end
+
+  describe "#put_string" do
+    it "writes a final \\0" do
+      ptr = FFI::MemoryPointer.new(8)
+      ptr.write_int64(-1)
+      ptr.put_string(0, "äbc")
+      expect(ptr.read_bytes(5)).to eq("äbc\x00".b)
+      expect(ptr.read_string).to eq("äbc".b)
+    end
+  end
 end