FFI::NullPointerError raised in write_to_memory on invalid image data

[dloebl]

image.write_to_memory sometimes raises a FFI::NullPointerError if the image has invalid image data.

To Reproduce
Steps to reproduce the behavior:
coffee-19613_128.gif

null_pointer_error.rb:

require "vips"

img = Vips::Image.new_from_file "coffee-19613_128.gif"
img.write_to_memory

$ ruby null_pointer_error.rb

(process:32892): VIPS-WARNING **: 11:54:17.971: Unexpected end of GIF source data

(process:32892): VIPS-WARNING **: 11:54:17.972: error in tile 0 x 0
ruby-vips-2.2.3/lib/vips/image.rb:708:in 'FFI::AbstractMemory#get_bytes': invalid memory read at address=0x0000000000000000 (FFI::NullPointerError)
	from ruby-vips-2.2.3/lib/vips/image.rb:708:in 'Vips::Image#write_to_memory'
	from null_pointer_error.rb:4:in '<main>

Expected behavior
Vips::Error raised instead

ruby-vips version: v2.2.3
libvips version: v8.16.1

[jcupitt]

Hi @dloebl,

How odd! write_to_memory is:

    def write_to_memory
      len = Vips::SizeStruct.new
      ptr = Vips.vips_image_write_to_memory self, len
      raise Vips::Error if ptr.nil?
    
      # wrap up as an autopointer
      ptr = FFI::AutoPointer.new(ptr, GLib::G_FREE)

      ptr.get_bytes 0, len[:value]
    end

So it's checking for nil. If add a puts:

    def write_to_memory
      len = Vips::SizeStruct.new
      ptr = Vips.vips_image_write_to_memory self, len
      raise Vips::Error if ptr.nil?
      puts "ptr = #{ptr}"

      ...

I see:

$ ./coffee.rb ~/pics/coffee.gif 

(process:257035): VIPS-WARNING **: 12:05:55.492: Unexpected end of GIF source data

(process:257035): VIPS-WARNING **: 12:05:55.494: error in tile 0 x 0
ptr = #<FFI::Pointer address=0x0000000000000000>
/home/john/packages/gems/gems/ruby-vips-2.2.4/lib/vips/image.rb:710:in `get_bytes': invalid memory read at address=0x0000000000000000 (FFI::NullPointerError)

So vips_image_write_to_memory() is returning a NULL pointer, but ptr.nil? is passing.

Looking at the FFI docs, they seem to use ptr.null?:

https://www.rubydoc.info/gems/ffi/FFI/Pointer#null%3F-instance_method

I'm sure that's new, we'd have picked it if it were available.

It looks like this will need changing everywhere, argh!

john@banana ~/GIT/ruby-vips (master) $ find . -name "*.rb" | xargs grep 'nil?' | wc
     28     152    1779

[jcupitt]

I only found one other nil that should have been null, so it's not so bad. Phew!

I'd forgotten, when ruby-ffi turns a NULL pointer into a string it becomes nil, so .nil? is correct in that case. It's just pointer returns that need the .null? method.

How confusing!

I've made a PR, let's move any discussion there.

#420