From 1576190fc2002bec189b7222a310759d504cdcfc Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Wed, 4 Oct 2023 22:55:49 +0100 Subject: [PATCH] Fix null termination (#183) Fixes #181 In general, we should accept `[]const u8` and return `[:0]const u8` where possible. We switch from using PyObject_GetAttrString and equivalents to PyObject_GetAttr since the former simply creates a PyUnicode object anyway. By creating the object ourselves, we're able to construct it from non-null terminated slices without making a second copy. The actual bug we had, was the `PyModule.from_code` function was using `[]const u8` slices as `value.ptr` without requirement them to be null terminated. --- pydust/src/types/error.zig | 5 +---- pydust/src/types/module.zig | 12 ++++++++++-- pydust/src/types/obj.zig | 30 +++++++++++++++++++----------- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/pydust/src/types/error.zig b/pydust/src/types/error.zig index 72af67e2..291b878b 100644 --- a/pydust/src/types/error.zig +++ b/pydust/src/types/error.zig @@ -170,10 +170,7 @@ const PyExc = struct { const fake_module = try py.PyModule.fromCode(code, line_info.file_name, symbol_info.compile_unit_name); defer fake_module.decref(); - const func_name = try py.allocator.dupeZ(u8, symbol_info.symbol_name); - defer py.allocator.free(func_name); - - _ = fake_module.obj.call(void, func_name, .{}, .{}) catch null; + _ = fake_module.obj.call(void, symbol_info.symbol_name, .{}, .{}) catch null; // Grab our forced exception info. // We can ignore qtype and qvalue, we just want to get the traceback object. diff --git a/pydust/src/types/module.zig b/pydust/src/types/module.zig index e9e05430..51e9fbc4 100644 --- a/pydust/src/types/module.zig +++ b/pydust/src/types/module.zig @@ -65,10 +65,18 @@ pub const PyModule = extern struct { /// Create and insantiate a PyModule object from a Python code string. pub fn fromCode(code: []const u8, filename: []const u8, module_name: []const u8) !PyModule { - const pycode = ffi.Py_CompileString(code.ptr, filename.ptr, ffi.Py_file_input) orelse return PyError.PyRaised; + // Ensure null-termination of all strings + const codeZ = try py.allocator.dupeZ(u8, code); + defer py.allocator.free(codeZ); + const filenameZ = try py.allocator.dupeZ(u8, filename); + defer py.allocator.free(filenameZ); + const module_nameZ = try py.allocator.dupeZ(u8, module_name); + defer py.allocator.free(module_nameZ); + + const pycode = ffi.Py_CompileString(codeZ.ptr, filenameZ.ptr, ffi.Py_file_input) orelse return PyError.PyRaised; defer ffi.Py_DECREF(pycode); - const pymod = ffi.PyImport_ExecCodeModuleEx(module_name.ptr, pycode, filename.ptr) orelse return PyError.PyRaised; + const pymod = ffi.PyImport_ExecCodeModuleEx(module_nameZ.ptr, pycode, filenameZ.ptr) orelse return PyError.PyRaised; return .{ .obj = .{ .py = pymod } }; } }; diff --git a/pydust/src/types/obj.zig b/pydust/src/types/obj.zig index e2156075..fe47f86f 100644 --- a/pydust/src/types/obj.zig +++ b/pydust/src/types/obj.zig @@ -34,21 +34,23 @@ pub const PyObject = extern struct { } /// Call a method on this object with no arguments. - pub fn call0(self: PyObject, comptime T: type, method: [:0]const u8) !T { + pub fn call0(self: PyObject, comptime T: type, method: []const u8) !T { return py.call0(T, try self.get(method)); } /// Call a method on this object with the given args and kwargs. - pub fn call(self: PyObject, comptime T: type, method: [:0]const u8, args: anytype, kwargs: anytype) !T { - const methodObj = try self.get(method); - return py.call(T, methodObj, args, kwargs); + pub fn call(self: PyObject, comptime T: type, method: []const u8, args: anytype, kwargs: anytype) !T { + return py.call(T, try self.get(method), args, kwargs); } - pub fn get(self: PyObject, attr: [:0]const u8) !py.PyObject { - return .{ .py = ffi.PyObject_GetAttrString(self.py, attr) orelse return PyError.PyRaised }; + pub fn get(self: PyObject, attr: []const u8) !py.PyObject { + const attrStr = try py.PyString.create(attr); + defer attrStr.decref(); + + return .{ .py = ffi.PyObject_GetAttr(self.py, attrStr.obj.py) orelse return PyError.PyRaised }; } - pub fn getAs(self: PyObject, comptime T: type, attr: [:0]const u8) !T { + pub fn getAs(self: PyObject, comptime T: type, attr: []const u8) !T { return try py.as(T, try self.get(attr)); } @@ -65,15 +67,21 @@ pub const PyObject = extern struct { return buffer; } - pub fn set(self: PyObject, attr: [:0]const u8, value: PyObject) !PyObject { - if (ffi.PyObject_SetAttrString(self.py, attr, value.py) < 0) { + pub fn set(self: PyObject, attr: []const u8, value: PyObject) !PyObject { + const attrStr = try py.PyString.create(attr); + defer attrStr.decref(); + + if (ffi.PyObject_SetAttr(self.py, attrStr.obj.py, value.py) < 0) { return PyError.PyRaised; } return self; } - pub fn del(self: PyObject, attr: [:0]const u8) !PyObject { - if (ffi.PyObject_DelAttrString(self.py, attr) < 0) { + pub fn del(self: PyObject, attr: []const u8) !PyObject { + const attrStr = try py.PyString.create(attr); + defer attrStr.decref(); + + if (ffi.PyObject_DelAttr(self.py, attrStr.obj.py) < 0) { return PyError.PyRaised; } return self;