From 97bb96184577c0de4417ba8164fbf1dfad1b587d Mon Sep 17 00:00:00 2001
From: osch <oliver at luced de>
Date: Wed, 11 Dec 2019 17:21:29 +0100
Subject: [PATCH 1/6] fix possible memory leak (issue #8) and improve error
 handling

---
 obj_region.c    | 22 ++++++++++++----------
 oocairo.c       | 17 +++++++++++++++++
 test/region.lua | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 10 deletions(-)

diff --git a/obj_region.c b/obj_region.c
index 32f6a7d..3833975 100644
--- a/obj_region.c
+++ b/obj_region.c
@@ -39,23 +39,25 @@ region_create_rectangle (lua_State *L) {
 
 static int
 region_create_rectangles (lua_State *L) {
-    size_t max_entries = lua_objlen (L, 1);
-    cairo_rectangle_int_t *rects = calloc(max_entries, sizeof(*rects));
-    size_t i = 0;
+    luaL_checktype(L, 1, LUA_TTABLE);
+    lua_settop(L, 1); /* discard other args */
 
+    size_t max_entries = lua_objlen (L, 1);
+    cairo_rectangle_int_t *rects = lua_newuserdata(L, max_entries * sizeof(cairo_rectangle_int_t));
+    size_t i;
+    
     /* Now iterate over the table */
-    lua_pushnil(L);
-    while (lua_next(L, 1) != 0) {
-        if (i >= max_entries)
-            return luaL_error(L, "Cairo.region_create_rectangles(): Internal error, table larger than table?!");
-        from_lua_rectangle (L, &rects[i], 3);
-        i++;
+    for (i = 0; i < max_entries; ++i) {
+        lua_rawgeti(L, 1, i+1);
+        int ok = from_lua_rectangle2 (L, &rects[i], 3);
+        if (!ok) {
+            return luaL_argerror(L, 1, "list contains invalid rectangle");
+        }
         lua_pop(L, 1);
     }
 
     cairo_region_t **reg = create_region_userdata(L);
     *reg = cairo_region_create_rectangles(rects, i);
-    free(rects);
     return 1;
 }
 
diff --git a/oocairo.c b/oocairo.c
index 235ccff..5d05669 100644
--- a/oocairo.c
+++ b/oocairo.c
@@ -371,6 +371,23 @@ from_lua_rectangle (lua_State *L, cairo_rectangle_int_t *rect, int pos) {
     DO(height);
 #undef DO
 }
+
+static int
+from_lua_rectangle2 (lua_State *L, cairo_rectangle_int_t *rect, int pos) {
+    if (lua_type(L, pos) != LUA_TTABLE) return 0;
+#define DO(t) \
+    lua_pushstring(L, #t); \
+    lua_gettable(L, pos); \
+    if (!lua_isnumber(L, -1)) return 0; \
+    rect->t = lua_tointeger(L, -1); \
+    lua_pop(L, 1)
+    DO(x);
+    DO(y);
+    DO(width);
+    DO(height);
+#undef DO
+    return 1;
+}
 #endif
 
 static void
diff --git a/test/region.lua b/test/region.lua
index 7eb8077..8315185 100644
--- a/test/region.lua
+++ b/test/region.lua
@@ -136,6 +136,48 @@ if Cairo.check_version(1, 10, 0) then
         assert_equal(rect[2].y, 10)
         assert_equal(rect[2].width, 1)
         assert_equal(rect[2].height, 1)
+
+        local reg = Cairo.region_create_rectangles(rects, "unused")
+        local rect = reg:get_rectangles()
+        assert_equal(#rect, 2)
+        assert_equal(rect[1].x, 0)
+        assert_equal(rect[1].y, 0)
+        assert_equal(rect[1].width, 1)
+        assert_equal(rect[1].height, 1)
+        assert_equal(rect[2].x, 10)
+        assert_equal(rect[2].y, 10)
+        assert_equal(rect[2].width, 1)
+        assert_equal(rect[2].height, 1)
+
+        local rects = { rect1, rect2, unused = true }
+        local reg = Cairo.region_create_rectangles(rects)
+        local rect = reg:get_rectangles()
+        assert_equal(#rect, 2)
+        assert_equal(rect[1].x, 0)
+        assert_equal(rect[1].y, 0)
+        assert_equal(rect[1].width, 1)
+        assert_equal(rect[1].height, 1)
+        assert_equal(rect[2].x, 10)
+        assert_equal(rect[2].y, 10)
+        assert_equal(rect[2].width, 1)
+        assert_equal(rect[2].height, 1)
+
+        local rect3 = { x = 0, y = 0 }
+        local rects = { rect1, rect2, rect3 }
+        local ok, errmsg = pcall(function() 
+            Cairo.region_create_rectangles(rects) 
+        end)
+        assert_false(ok)
+        assert_match("bad argument %#1 to %'region_create_rectangles%' %(list contains invalid rectangle%)", errmsg)
+
+        local rect3 = { x = 0, y = 0, width = 10, height = {} }
+        local rects = { rect1, rect2, rect3 }
+        local ok, errmsg = pcall(function() 
+            Cairo.region_create_rectangles(rects) 
+        end)
+        assert_false(ok)
+        assert_match("bad argument %#1 to %'region_create_rectangles%' %(list contains invalid rectangle%)", errmsg)
+
     end
 end
 

From 3a8cced082316fc096de4b0698ae5b339db4348d Mon Sep 17 00:00:00 2001
From: osch <oliver at luced de>
Date: Wed, 11 Dec 2019 16:58:16 +0100
Subject: [PATCH 2/6] fix typo in testcase

---
 test/surface.lua | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/surface.lua b/test/surface.lua
index baf7006..9c044f2 100644
--- a/test/surface.lua
+++ b/test/surface.lua
@@ -433,7 +433,9 @@ if Cairo.check_version(1, 10, 0) then
         test("test/data", "test data")
 
         local surface = Cairo.image_surface_create("rgb24", 0, 0)
-        assert_error(function () surface_set_mime_data(nil, "foo") end)
+        local ok, errmsg = pcall(function () surface:set_mime_data(nil, "foo") end)
+        assert_false(ok)
+        assert_match("bad argument %#1 to %'set_mime_data%' %(string expected%, got nil%)", errmsg)
     end
 end
 

From acbf9fc901ace2790a4fa0e4c2a308b61baa56c4 Mon Sep 17 00:00:00 2001
From: osch <oliver at luced de>
Date: Wed, 11 Dec 2019 17:10:20 +0100
Subject: [PATCH 3/6] throw lua error on malloc failures instead calling assert
 (c assert aborts if NDEBUG is not set)

---
 obj_context.c   |  8 ++++++--
 obj_font_face.c |  4 +++-
 obj_surface.c   | 10 +++++++---
 oocairo.c       | 13 +++++++++----
 4 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/obj_context.c b/obj_context.c
index 8f28a81..5a18801 100644
--- a/obj_context.c
+++ b/obj_context.c
@@ -229,7 +229,9 @@ cr_get_dash (lua_State *L) {
     cnt = cairo_get_dash_count(*obj);
     if (cnt > 0) {
         dashes = malloc(sizeof(double) * cnt);
-        assert(dashes);
+        if (!dashes) {
+            return luaL_error(L, "out of memory");
+        }
     }
 
     cairo_get_dash(*obj, dashes, &offset);
@@ -642,7 +644,9 @@ cr_set_dash (lua_State *L) {
     num_dashes = lua_objlen(L, 2);
     if (num_dashes > 0) {
         dashes = malloc(sizeof(double) * num_dashes);
-        assert(dashes);
+        if (!dashes) {
+            return luaL_error(L, "out of memory");
+        }
         dashtotal = 0;
 
         for (i = 0; i < num_dashes; ++i) {
diff --git a/obj_font_face.c b/obj_font_face.c
index 1aeb13f..016b6d9 100644
--- a/obj_font_face.c
+++ b/obj_font_face.c
@@ -197,7 +197,9 @@ user_font_face_create (lua_State *L) {
 
     lua_createtable(L, 4, 0);
     info = malloc(sizeof(UserFontInfo));
-    assert(info);
+    if (!info) {
+        return luaL_error(L, "out of memory");
+    }
     info->L = L;
     info->ref = LUA_NOREF;
 
diff --git a/obj_surface.c b/obj_surface.c
index 1fbda57..fea6aa8 100644
--- a/obj_surface.c
+++ b/obj_surface.c
@@ -68,7 +68,9 @@ image_surface_create_from_data (lua_State *L) {
 
     surface = create_surface_userdata(L);
     surface->image_buffer = malloc(data_len);
-    assert(surface->image_buffer);
+    if (!surface->image_buffer) {
+        return luaL_error(L, "out of memory");
+    }
     memcpy(surface->image_buffer, data, data_len);
     surface->surface = cairo_image_surface_create_for_data(
                             surface->image_buffer, fmt, width, height, stride);
@@ -474,7 +476,9 @@ surface_get_gdk_pixbuf (lua_State *L) {
         strideo = ((3 * width) + 7) & ~7;   /* align to 8 bytes */
     buffer_len = strideo * height;
     buffer = malloc(buffer_len);
-    assert(buffer);
+    if (!buffer) {
+        return luaL_error(L, "out of memory");
+    }
 
     rowpi = (const char *) cairo_image_surface_get_data(*surface);
     rowpo = buffer;
@@ -719,7 +723,7 @@ set_mime_data(lua_State *L)
         mime_data = luaL_checklstring(L, 3, &data_length);
         mime_priv = malloc(data_length);
         if (!mime_priv) {
-            return push_cairo_status(L, CAIRO_STATUS_NO_MEMORY);
+            return luaL_error(L, "out of memory");
         }
         memcpy(mime_priv, mime_data, data_length);
     }
diff --git a/oocairo.c b/oocairo.c
index 5d05669..bdd4882 100644
--- a/oocairo.c
+++ b/oocairo.c
@@ -511,7 +511,9 @@ from_lua_glyph_array (lua_State *L, cairo_glyph_t **glyphs, int *num_glyphs,
         return;
     }
     *glyphs = GLYPHS_ALLOC(*num_glyphs);
-    assert(*glyphs);
+    if (!*glyphs) {
+        return luaL_error(L, "out of memory");
+    }
 
     for (i = 0; i < *num_glyphs; ++i) {
         lua_rawgeti(L, pos, i + 1);
@@ -595,7 +597,9 @@ from_lua_clusters_table (lua_State *L, cairo_text_cluster_t **clusters,
         return;
     }
     *clusters = cairo_text_cluster_allocate(*num);
-    assert(*clusters);
+    if (!*clusters) {
+        return luaL_error(L, "out of memory");
+    }
 
     for (i = 0; i < *num; ++i) {
         lua_rawgeti(L, pos, i + 1);
@@ -795,8 +799,9 @@ free_surface_userdata (SurfaceUserdata *ud) {
 static char *
 my_strdup (const char *s) {
     char *copy = malloc(strlen(s) + 1);
-    assert(copy);
-    strcpy(copy, s);
+    if (copy) {
+        strcpy(copy, s);
+    }
     return copy;
 }
 

From adf832fb01dbdf0ea91486a7b3b4befa10bd243e Mon Sep 17 00:00:00 2001
From: osch <oliver at luced de>
Date: Wed, 11 Dec 2019 17:46:42 +0100
Subject: [PATCH 4/6] improved error message

---
 oocairo.c       | 5 ++++-
 test/region.lua | 5 +++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/oocairo.c b/oocairo.c
index bdd4882..174da06 100644
--- a/oocairo.c
+++ b/oocairo.c
@@ -363,7 +363,10 @@ from_lua_rectangle (lua_State *L, cairo_rectangle_int_t *rect, int pos) {
 #define DO(t) \
     lua_pushstring(L, #t); \
     lua_gettable(L, pos); \
-    rect->t = luaL_checkinteger(L, -1); \
+    if (!lua_isnumber(L, -1)) { \
+        luaL_argerror(L, pos, "invalid rectangle"); \
+    } \
+    rect->t = lua_tointeger(L, -1); \
     lua_pop(L, 1)
     DO(x);
     DO(y);
diff --git a/test/region.lua b/test/region.lua
index 8315185..b971328 100644
--- a/test/region.lua
+++ b/test/region.lua
@@ -52,6 +52,11 @@ if Cairo.check_version(1, 10, 0) then
         assert_equal(reg:contains_rectangle(rect), "overlap-in")
         rect.width = 8
         assert_equal(reg:contains_rectangle(rect), "overlap-part")
+        local ok, errmsg = pcall(function()
+            reg:contains_rectangle({ x = 0, y = 0 })
+        end)
+        assert_false(ok)
+        assert_match("bad argument %#1 to %'contains_rectangle%' %(invalid rectangle%)", errmsg)
     end
 
     function module.test_copy ()

From 271ab5e5691972cc2f88f9fb0e02e2a791b48856 Mon Sep 17 00:00:00 2001
From: osch <oliver at luced de>
Date: Sat, 14 Dec 2019 13:53:31 +0100
Subject: [PATCH 5/6] refactored function "from_lua_rectangle2" to
 "from_lua_rectangle_list_element": the new function has the same semantics
 than other functions in this area (e.g. from_lua_rectangle): a lua error is
 raised instead of returning a return value to indicate failure. The new
 function "from_lua_rectangle_list_element" correctly reports the argument
 number in the error message.

---
 obj_region.c |  7 +------
 oocairo.c    | 16 ++++++++++------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/obj_region.c b/obj_region.c
index 3833975..e8b771a 100644
--- a/obj_region.c
+++ b/obj_region.c
@@ -48,12 +48,7 @@ region_create_rectangles (lua_State *L) {
     
     /* Now iterate over the table */
     for (i = 0; i < max_entries; ++i) {
-        lua_rawgeti(L, 1, i+1);
-        int ok = from_lua_rectangle2 (L, &rects[i], 3);
-        if (!ok) {
-            return luaL_argerror(L, 1, "list contains invalid rectangle");
-        }
-        lua_pop(L, 1);
+        from_lua_rectangle_list_element(L, &rects[i], 1, i+1);
     }
 
     cairo_region_t **reg = create_region_userdata(L);
diff --git a/oocairo.c b/oocairo.c
index 174da06..5acd0b6 100644
--- a/oocairo.c
+++ b/oocairo.c
@@ -375,21 +375,25 @@ from_lua_rectangle (lua_State *L, cairo_rectangle_int_t *rect, int pos) {
 #undef DO
 }
 
-static int
-from_lua_rectangle2 (lua_State *L, cairo_rectangle_int_t *rect, int pos) {
-    if (lua_type(L, pos) != LUA_TTABLE) return 0;
+static void
+from_lua_rectangle_list_element (lua_State *L, cairo_rectangle_int_t *rect, int list_pos, int element_index) {
+    lua_rawgeti(L, list_pos, element_index); /* push rectangle from list */
+    if (lua_type(L, -1) != LUA_TTABLE) goto error;
 #define DO(t) \
     lua_pushstring(L, #t); \
-    lua_gettable(L, pos); \
-    if (!lua_isnumber(L, -1)) return 0; \
+    lua_gettable(L, -2); \
+    if (!lua_isnumber(L, -1)) goto error; \
     rect->t = lua_tointeger(L, -1); \
     lua_pop(L, 1)
     DO(x);
     DO(y);
     DO(width);
     DO(height);
+    lua_pop(L, 1); /* pop rectangle */
 #undef DO
-    return 1;
+    return;
+error:
+    luaL_argerror(L, list_pos, "list contains invalid rectangle");
 }
 #endif
 

From 362f4d498ff33a5b9d142052ca72a05209ba0ba1 Mon Sep 17 00:00:00 2001
From: osch <oliver at luced de>
Date: Sat, 14 Dec 2019 14:27:02 +0100
Subject: [PATCH 6/6] check for rectangle integer coordinates under Lua5.3

---
 oocairo.c       | 21 ++++++++++++++-------
 test/region.lua | 11 +++++++++--
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/oocairo.c b/oocairo.c
index 5acd0b6..6fff917 100644
--- a/oocairo.c
+++ b/oocairo.c
@@ -53,6 +53,12 @@ static int luaL_typerror (lua_State *L, int narg, const char *tname) {
 #define lua_rawlen(a, b) do_not_use_lua_rawlen_use_lua_objlen_instead;
 #endif
 
+#if LUA_VERSION_NUM >= 503
+#define oocairo_lua_isinteger(L, idx) lua_isinteger(L, idx)
+#else
+#define oocairo_lua_isinteger(L, idx) lua_isnumber(L, idx)
+#endif
+
 static const int ENDIANNESS_TEST_VAL = 1;
 #define IS_BIG_ENDIAN (!(*(const char *) &ENDIANNESS_TEST_VAL))
 
@@ -363,8 +369,8 @@ from_lua_rectangle (lua_State *L, cairo_rectangle_int_t *rect, int pos) {
 #define DO(t) \
     lua_pushstring(L, #t); \
     lua_gettable(L, pos); \
-    if (!lua_isnumber(L, -1)) { \
-        luaL_argerror(L, pos, "invalid rectangle"); \
+    if (!oocairo_lua_isinteger(L, -1)) { \
+        luaL_argerror(L, pos, "invalid rectangle: coordinates must be integer values"); \
     } \
     rect->t = lua_tointeger(L, -1); \
     lua_pop(L, 1)
@@ -378,11 +384,15 @@ from_lua_rectangle (lua_State *L, cairo_rectangle_int_t *rect, int pos) {
 static void
 from_lua_rectangle_list_element (lua_State *L, cairo_rectangle_int_t *rect, int list_pos, int element_index) {
     lua_rawgeti(L, list_pos, element_index); /* push rectangle from list */
-    if (lua_type(L, -1) != LUA_TTABLE) goto error;
+    if (lua_type(L, -1) != LUA_TTABLE) {
+        luaL_argerror(L, list_pos, "list contains invalid rectangle");
+    }
 #define DO(t) \
     lua_pushstring(L, #t); \
     lua_gettable(L, -2); \
-    if (!lua_isnumber(L, -1)) goto error; \
+    if (!oocairo_lua_isinteger(L, -1)) { \
+        luaL_argerror(L, list_pos, "list contains invalid rectangle: coordinates must be integer values"); \
+    } \
     rect->t = lua_tointeger(L, -1); \
     lua_pop(L, 1)
     DO(x);
@@ -391,9 +401,6 @@ from_lua_rectangle_list_element (lua_State *L, cairo_rectangle_int_t *rect, int
     DO(height);
     lua_pop(L, 1); /* pop rectangle */
 #undef DO
-    return;
-error:
-    luaL_argerror(L, list_pos, "list contains invalid rectangle");
 }
 #endif
 
diff --git a/test/region.lua b/test/region.lua
index b971328..c655430 100644
--- a/test/region.lua
+++ b/test/region.lua
@@ -56,7 +56,7 @@ if Cairo.check_version(1, 10, 0) then
             reg:contains_rectangle({ x = 0, y = 0 })
         end)
         assert_false(ok)
-        assert_match("bad argument %#1 to %'contains_rectangle%' %(invalid rectangle%)", errmsg)
+        assert_match("bad argument %#1 to %'contains_rectangle%' %(invalid rectangle%: coordinates must be integer values%)", errmsg)
     end
 
     function module.test_copy ()
@@ -173,7 +173,7 @@ if Cairo.check_version(1, 10, 0) then
             Cairo.region_create_rectangles(rects) 
         end)
         assert_false(ok)
-        assert_match("bad argument %#1 to %'region_create_rectangles%' %(list contains invalid rectangle%)", errmsg)
+        assert_match("bad argument %#1 to %'region_create_rectangles%' %(list contains invalid rectangle%: coordinates must be integer values%)", errmsg)
 
         local rect3 = { x = 0, y = 0, width = 10, height = {} }
         local rects = { rect1, rect2, rect3 }
@@ -181,6 +181,13 @@ if Cairo.check_version(1, 10, 0) then
             Cairo.region_create_rectangles(rects) 
         end)
         assert_false(ok)
+        assert_match("bad argument %#1 to %'region_create_rectangles%' %(list contains invalid rectangle%: coordinates must be integer values%)", errmsg)
+
+        local rects = { rect1, rect2, "" }
+        local ok, errmsg = pcall(function() 
+            Cairo.region_create_rectangles(rects) 
+        end)
+        assert_false(ok)
         assert_match("bad argument %#1 to %'region_create_rectangles%' %(list contains invalid rectangle%)", errmsg)
 
     end