diff --git a/.gitignore b/.gitignore index ad7431c..79b4ae6 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,9 @@ /ext/gd/*.so /ext/gd/*.bundle demo/rsn_events_bot/rsn_events.png +.aider.chat.history.md +.aider.input.history +.aider.tags.cache.v4/ +examples/jupyter-notebooks/.ipynb_checkpoints/Hist2D-Copy1-checkpoint.ipynb +examples/jupyter-notebooks/Hist2D-Copy1.ipynb + diff --git a/CHANGELOG.md b/CHANGELOG.md index 8296cf5..46b935d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,44 @@ # Changelog +## v0.3.1 + +### Security + +- Hardened `GD::Image` initialization to prevent creation of partially initialized image objects. +- `GD::Image.new` now requires explicit `width` and `height` arguments. +- Added validation to reject zero or negative image dimensions. +- Added NULL-pointer guards across image operations to prevent dereferencing uninitialized image structures. +- Improved defensive programming around image cloning and copy operations. + +### Fixed + +#### `GD::Image.new` + +- `GD::Image.new` no longer returns an object with `img == NULL`. +- Calling `GD::Image.new` without dimensions now raises: + +```ruby +ArgumentError: width and height are required + +### Fixed + +#### Text Bounding Box Rendering + +- Fixed an issue where `GD::Image#text_bbox` could unexpectedly render text onto the target image while calculating text metrics. +- Bounding box calculations are now guaranteed to be side-effect free. +- Text measurement no longer modifies image pixels. +- Prevents "ghost text" artifacts appearing in generated images when only text dimensions are requested. + +Example: + +```ruby +img.text_bbox( + "text", + font: font, + size: 45, + angle: 0 +) +``` + ## v0.3.0 ### Added diff --git a/ext/gd/image.c b/ext/gd/image.c index fd069d3..f62baf6 100644 --- a/ext/gd/image.c +++ b/ext/gd/image.c @@ -1,24 +1,42 @@ #include "ruby_gd.h" -static VALUE gd_image_initialize(int argc, VALUE *argv, VALUE self) { +static VALUE gd_image_alloc(VALUE klass) { gd_image_wrapper *wrap; - TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); - + VALUE obj = TypedData_Make_Struct(klass, gd_image_wrapper, &gd_image_type, wrap); wrap->img = NULL; + return obj; +} + +/* --------------------------------------------------------- + * Initialize + * --------------------------------------------------------- */ +static VALUE gd_image_initialize(int argc, VALUE *argv, VALUE self) { if (argc == 0) { + rb_raise(rb_eArgError, "width and height are required"); return self; } + + gd_image_wrapper *wrap; + TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); + + wrap->img = NULL; + if (argc != 2) { rb_raise(rb_eArgError, "expected 0 or 2 arguments"); } int w = NUM2INT(argv[0]); int h = NUM2INT(argv[1]); + if (w <= 0 || h <= 0) rb_raise(rb_eArgError, "width and height must be positive"); wrap->img = gdImageCreateTrueColor(w,h); + + if (!wrap->img) + rb_raise(rb_eRuntimeError, "gdImageCreateTrueColor failed"); + gdImageSaveAlpha(wrap->img, 1); gdImageAlphaBlending(wrap->img, 0); @@ -27,12 +45,6 @@ static VALUE gd_image_initialize(int argc, VALUE *argv, VALUE self) { gdImageAlphaBlending(wrap->img, 1); - int t = gdTrueColorAlpha(0,0,0,127); - gdImageFilledRectangle(wrap->img, 0, 0, w-1, h-1, t); - - if (!wrap->img) - rb_raise(rb_eRuntimeError, "gdImageCreateTrueColor failed"); - return self; } @@ -58,7 +70,7 @@ static VALUE gd_image_initialize_true_color(VALUE self, VALUE width, VALUE heigh // Opcional: desactivar el fondo transparente si no lo necesitas gdImageSaveAlpha(wrap->img, 1); gdImageAlphaBlending(wrap->img, 0); - + int t = gdTrueColorAlpha(0,0,0,127); gdImageFilledRectangle(wrap->img, 0,0, w-1, h-1, t); @@ -66,30 +78,47 @@ static VALUE gd_image_initialize_true_color(VALUE self, VALUE width, VALUE heigh } static VALUE gd_image_s_new_true_color(VALUE klass, VALUE width, VALUE height) { - VALUE obj = rb_class_new_instance(0, NULL, klass); - gd_image_initialize_true_color(obj, width, height); - return obj; + VALUE img = rb_obj_alloc(klass); + rb_funcall(img, rb_intern("initialize"), 2, width, height); + return img; } -static VALUE gd_image_clone(VALUE self) { - gd_image_wrapper *wrap; - TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); +/* --------------------------------------------------------- + * initialize_copy (correct way for clone/dup) + * --------------------------------------------------------- */ - if (!wrap->img) rb_raise(rb_eRuntimeError, "cannot clone: image is NULL"); +static VALUE gd_image_initialize_copy(VALUE self, VALUE orig) { + gd_image_wrapper *src; + gd_image_wrapper *dst; - gdImagePtr copy = gdImageClone(wrap->img); - if (!copy) rb_raise(rb_eRuntimeError, "gdImageClone failed"); + if (self == orig) return self; - VALUE obj = rb_class_new_instance(0, NULL, CLASS_OF(self)); - gd_image_wrapper *w2; - TypedData_Get_Struct(obj, gd_image_wrapper, &gd_image_type, w2); - w2->img = copy; + TypedData_Get_Struct(orig, gd_image_wrapper, &gd_image_type, src); + TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, dst); - return obj; + if (!src->img) + rb_raise(rb_eRuntimeError, "cannot copy: image is NULL"); + + if (dst->img) { + gdImageDestroy(dst->img); + dst->img = NULL; + } + + dst->img = gdImageClone(src->img); + + if (!dst->img) + rb_raise(rb_eRuntimeError, "gdImageClone failed"); + + return self; } +/* --------------------------------------------------------- + * Free + * --------------------------------------------------------- */ + static void gd_image_free(void *ptr) { gd_image_wrapper *wrap = (gd_image_wrapper *)ptr; + if (!wrap) return; if (wrap->img) { @@ -104,17 +133,14 @@ static size_t gd_image_memsize(const void *ptr) { const rb_data_type_t gd_image_type = { "GD::Image", - { 0, gd_image_free, gd_image_memsize, 0 }, // <-- dcompact agregado + { 0, gd_image_free, gd_image_memsize, }, 0, 0, RUBY_TYPED_FREE_IMMEDIATELY }; -static VALUE gd_image_alloc(VALUE klass) { - gd_image_wrapper *wrap; - VALUE obj = TypedData_Make_Struct(klass, gd_image_wrapper, &gd_image_type, wrap); - wrap->img = NULL; - return obj; -} +/* --------------------------------------------------------- + * Image loading + * --------------------------------------------------------- */ static VALUE gd_image_open(VALUE klass, VALUE path) { const char *filename = StringValueCStr(path); @@ -141,44 +167,44 @@ static VALUE gd_image_open(VALUE klass, VALUE path) { } fclose(f); - if (!img) rb_raise(rb_eRuntimeError, "image decode failed"); - VALUE obj = rb_class_new_instance(0, NULL, klass); + if (!img) + rb_raise(rb_eRuntimeError, "image decode failed"); + + VALUE obj = rb_obj_alloc(klass); + gd_image_wrapper *wrap; TypedData_Get_Struct(obj, gd_image_wrapper, &gd_image_type, wrap); + wrap->img = img; return obj; } -static VALUE gd_image_alpha_blending(VALUE self, VALUE flag) { - gd_image_wrapper *wrap; - TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); - - gdImageAlphaBlending(wrap->img, RTEST(flag)); - return self; -} - -static VALUE gd_image_save_alpha(VALUE self, VALUE flag) { - gd_image_wrapper *wrap; - TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); - - gdImageSaveAlpha(wrap->img, RTEST(flag)); - return self; -} +/* --------------------------------------------------------- + * Image info + * --------------------------------------------------------- */ static VALUE gd_image_width(VALUE self) { - gd_image_wrapper *wrap; - TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); - return INT2NUM(gdImageSX(wrap->img)); + gd_image_wrapper *wrap; + TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); + if (!wrap->img) + rb_raise(rb_eRuntimeError, "image not initialized"); + return INT2NUM(gdImageSX(wrap->img)); } static VALUE gd_image_height(VALUE self) { - gd_image_wrapper *wrap; - TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); - return INT2NUM(gdImageSY(wrap->img)); + gd_image_wrapper *wrap; + TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); + if (!wrap->img) + rb_raise(rb_eRuntimeError, "image not initialized"); + return INT2NUM(gdImageSY(wrap->img)); } +/* --------------------------------------------------------- + * Copy operations + * --------------------------------------------------------- */ + static VALUE gd_image_copy ( VALUE self, VALUE src, VALUE dx, VALUE dy, @@ -191,6 +217,9 @@ static VALUE gd_image_copy ( TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, dst); TypedData_Get_Struct(src, gd_image_wrapper, &gd_image_type, srcw); + if (!dst->img) rb_raise(rb_eRuntimeError, "image not initialized"); + if (!srcw->img) rb_raise(rb_eRuntimeError, "source image not initialized"); + gdImageCopy( dst->img, srcw->img, @@ -216,6 +245,9 @@ static VALUE gd_image_copy_resize(int argc, VALUE *argv, VALUE self) { TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, dst); TypedData_Get_Struct(src_img, gd_image_wrapper, &gd_image_type, src); + if (!dst->img) rb_raise(rb_eRuntimeError, "image not initialized"); + if (!src->img) rb_raise(rb_eRuntimeError, "source image not initialized"); + int dx = NUM2INT(dst_x); int dy = NUM2INT(dst_y); int sx = NUM2INT(src_x); @@ -225,9 +257,7 @@ static VALUE gd_image_copy_resize(int argc, VALUE *argv, VALUE self) { int dw = NUM2INT(dst_w); int dh = NUM2INT(dst_h); - int do_resample = RTEST(resample); - - if (do_resample) { + if (RTEST(resample)) { gdImageCopyResampled( dst->img, src->img, dx, dy, @@ -248,12 +278,34 @@ static VALUE gd_image_copy_resize(int argc, VALUE *argv, VALUE self) { return self; } +/* --------------------------------------------------------- + * Flags + * --------------------------------------------------------- */ + +static VALUE gd_image_alpha_blending(VALUE self, VALUE flag) { + gd_image_wrapper *wrap; + TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); + if (!wrap->img) + rb_raise(rb_eRuntimeError, "image not initialized"); + gdImageAlphaBlending(wrap->img, RTEST(flag)); + return self; +} + +static VALUE gd_image_save_alpha(VALUE self, VALUE flag) { + gd_image_wrapper *wrap; + TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); + if (!wrap->img) + rb_raise(rb_eRuntimeError, "image not initialized"); + gdImageSaveAlpha(wrap->img, RTEST(flag)); + return self; +} + static VALUE gd_image_antialias(VALUE self, VALUE flag) { gd_image_wrapper *wrap; TypedData_Get_Struct(self, gd_image_wrapper, &gd_image_type, wrap); if (!wrap->img) - rb_raise(rb_eRuntimeError, "image is NULL"); + rb_raise(rb_eRuntimeError, "image not initialized"); if (RTEST(flag)) { int aa = gdTrueColorAlpha(255,255,255,0); @@ -266,23 +318,31 @@ static VALUE gd_image_antialias(VALUE self, VALUE flag) { return flag; } +/* --------------------------------------------------------- + * Class definition + * --------------------------------------------------------- */ void gd_define_image(VALUE mGD) { + VALUE cGDImage = rb_define_class_under(mGD, "Image", rb_cObject); rb_define_alloc_func(cGDImage, gd_image_alloc); + rb_define_method(cGDImage, "initialize", gd_image_initialize, -1); + rb_define_method(cGDImage, "initialize_copy", gd_image_initialize_copy, 1); + rb_define_method(cGDImage, "width", gd_image_width, 0); rb_define_method(cGDImage, "height", gd_image_height, 0); - rb_define_method(cGDImage, "initialize", gd_image_initialize, -1); + rb_define_method(cGDImage, "copy_resize", gd_image_copy_resize, -1); rb_define_method(cGDImage, "copy", gd_image_copy, 7); - rb_define_method(cGDImage, "clone", gd_image_clone, 0); + rb_define_singleton_method(cGDImage, "open", gd_image_open, 1); rb_define_singleton_method(cGDImage, "new_true_color", gd_image_s_new_true_color, 2); + rb_define_method(cGDImage, "alpha_blending=", gd_image_alpha_blending, 1); rb_define_method(cGDImage, "save_alpha=", gd_image_save_alpha, 1); + rb_define_method(cGDImage, "antialias=", gd_image_antialias, 1); rb_define_method(cGDImage, "antialias", gd_image_antialias, 1); - } diff --git a/ext/gd/text.c b/ext/gd/text.c index 238eebd..0483fdb 100644 --- a/ext/gd/text.c +++ b/ext/gd/text.c @@ -56,7 +56,7 @@ static VALUE gd_image_text_bbox(VALUE self, VALUE text, VALUE opts) { memset(&extra, 0, sizeof(extra)); char *err = gdImageStringFTEx( - wrap->img, + NULL, brect, 0, /* no color, we don't draw */ font, diff --git a/ruby-libgd.gemspec b/ruby-libgd.gemspec index 1f177cc..578a50f 100644 --- a/ruby-libgd.gemspec +++ b/ruby-libgd.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |s| s.name = "ruby-libgd" - s.version = "0.3.0" + s.version = "0.3.1" s.summary = "Native Ruby bindings for libgd" s.description = "High-performance native Ruby bindings to libgd for image generation, drawing, filters, alpha blending, and transformations." s.authors = ["Germán Alberto Giménez Silva"] diff --git a/spec/image/initialize_spec.rb b/spec/image/initialize_spec.rb index 06a8924..2796408 100644 --- a/spec/image/initialize_spec.rb +++ b/spec/image/initialize_spec.rb @@ -14,9 +14,12 @@ expect { GD::Image.new(-1, 10) }.to raise_error(ArgumentError) end - it "allows empty initialize" do + it "requires width and height" do img = GD::Image.allocate - expect { img.send(:initialize) }.not_to raise_error + + expect { + img.send(:initialize) + }.to raise_error(ArgumentError, /width and height are required/) end end end diff --git a/spec/image_spec.rb b/spec/image_spec.rb index b162b85..59b5466 100644 --- a/spec/image_spec.rb +++ b/spec/image_spec.rb @@ -1,6 +1,67 @@ require "spec_helper" RSpec.describe GD::Image do + # ───────────────────────────────────────────────────────────── + # initialize + # ───────────────────────────────────────────────────────────── + + describe ".new" do + context "with no arguments" do + it "raises ArgumentError" do + expect { GD::Image.new }.to raise_error(ArgumentError, /width and height are required/) + end + end + + context "with a single argument" do + it "raises ArgumentError" do + expect { GD::Image.new(100) }.to raise_error(ArgumentError, /expected 0 or 2 arguments/) + end + end + + context "with valid dimensions" do + it "creates the image without error" do + expect { GD::Image.new(100, 100) }.not_to raise_error + end + end + + context "with dimensions <= 0" do + it "raises ArgumentError when width is 0" do + expect { GD::Image.new(0, 100) }.to raise_error(ArgumentError, /must be positive/) + end + + it "raises ArgumentError when height is negative" do + expect { GD::Image.new(100, -1) }.to raise_error(ArgumentError, /must be positive/) + end + end + end + + # ───────────────────────────────────────────────────────────── + # NULL guard on instance methods + # ───────────────────────────────────────────────────────────── + + describe "methods on an uninitialized image" do + let(:img) { GD::Image.allocate } # wrap->img = NULL + + it "#width raises RuntimeError" do + expect { img.width }.to raise_error(RuntimeError, /not initialized/) + end + + it "#height raises RuntimeError" do + expect { img.height }.to raise_error(RuntimeError, /not initialized/) + end + + it "#alpha_blending= raises RuntimeError" do + expect { img.alpha_blending = true }.to raise_error(RuntimeError, /not initialized/) + end + + it "#save_alpha= raises RuntimeError" do + expect { img.save_alpha = true }.to raise_error(RuntimeError, /not initialized/) + end + + it "#antialias= raises RuntimeError" do + expect { img.antialias = true }.to raise_error(RuntimeError, /not initialized/) + end + end it "creates and saves a blank image" do img = GD::Image.new(100,100) img.save("#{TMP}/blank.png")