From b766f9b879995d56f477dd63186faeb94d3c915a Mon Sep 17 00:00:00 2001 From: David Nichols Date: Mon, 16 Feb 2026 09:12:13 +0100 Subject: [PATCH 1/2] Add sandboxing, interruptible I/O, and fix memory leaks - Add QoreSandboxManager filesystem access checks before all file I/O operations in CairoSurface (SVG/PS create, PNG load/write) and CairoSvgReader (SVG file load) - Add qore_check_io_interrupt() calls before blocking I/O for interruptible I/O support - Fix memory leaks: add ReferenceHolder for QPP object parameters in setSourceSurface, setSource, createForSurface, and renderTo per QPP Object Parameter Handling pattern - Fix double-ref leak in CairoContext constructor (HARD_QORE_VALUE_OBJ_DATA already adds reference via getReferencedPrivateData) - Add private constructors to CairoSurface and CairoPattern for proper QPP lifecycle management Co-Authored-By: Claude Opus 4.6 --- src/CairoContext.cpp | 3 ++- src/CairoSurface.cpp | 28 ++++++++++++++++++++++++++++ src/CairoSvgReader.cpp | 7 +++++++ src/QC_CairoContext.qpp | 2 ++ src/QC_CairoPattern.qpp | 8 ++++++++ src/QC_CairoSurface.qpp | 7 +++++++ src/QC_CairoSvgReader.qpp | 1 + src/cairo-module.h | 1 + 8 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/CairoContext.cpp b/src/CairoContext.cpp index 1923c6e..326c5f6 100644 --- a/src/CairoContext.cpp +++ b/src/CairoContext.cpp @@ -32,7 +32,8 @@ QoreCairoContext::QoreCairoContext(QoreCairoSurface* surface, ExceptionSink* xsink) : surface_ref(surface) { assert(surface); - surface->ref(); + // NOTE: caller (QPP-generated code via HARD_QORE_VALUE_OBJ_DATA) already holds a reference + // from getReferencedPrivateData(); we take ownership of that reference here cr = cairo_create(surface->getSurface()); checkStatus(xsink); } diff --git a/src/CairoSurface.cpp b/src/CairoSurface.cpp index 6549d9b..b12b230 100644 --- a/src/CairoSurface.cpp +++ b/src/CairoSurface.cpp @@ -37,6 +37,13 @@ QoreCairoSurface::QoreCairoSurface(const std::string& path, double width, double xsink->raiseException("CAIRO-ERROR", "surface dimensions must be positive (%.2f x %.2f)", width, height); return; } + QoreSandboxManagerHelper smh; + if (smh && !smh->checkFilesystemAccess(path.c_str(), QSEC_WRITE | QSEC_CREATE, xsink)) { + return; + } + if (qore_check_io_interrupt(xsink, "SVG surface create")) { + return; + } surface = cairo_svg_surface_create(path.c_str(), width, height); checkStatus(xsink); } @@ -59,6 +66,13 @@ QoreCairoSurface::QoreCairoSurface(const std::string& path, double width, double xsink->raiseException("CAIRO-ERROR", "surface dimensions must be positive (%.2f x %.2f)", width, height); return; } + QoreSandboxManagerHelper smh; + if (smh && !smh->checkFilesystemAccess(path.c_str(), QSEC_WRITE | QSEC_CREATE, xsink)) { + return; + } + if (qore_check_io_interrupt(xsink, "PostScript surface create")) { + return; + } surface = cairo_ps_surface_create(path.c_str(), width, height); checkStatus(xsink); } @@ -87,6 +101,13 @@ QoreCairoSurface::QoreCairoSurface(int width, int height, cairo_format_t format, // Image surface from PNG file QoreCairoSurface::QoreCairoSurface(const std::string& path, ExceptionSink* xsink) : type(CST_IMAGE) { + QoreSandboxManagerHelper smh; + if (smh && !smh->checkFilesystemAccess(path.c_str(), QSEC_READ, xsink)) { + return; + } + if (qore_check_io_interrupt(xsink, "PNG file load")) { + return; + } surface = cairo_image_surface_create_from_png(path.c_str()); cairo_status_t status = cairo_surface_status(surface); if (status != CAIRO_STATUS_SUCCESS) { @@ -172,6 +193,13 @@ void QoreCairoSurface::writeToPng(const std::string& path, ExceptionSink* xsink) xsink->raiseException("CAIRO-ERROR", "writeToPng is only available for image and recording surfaces"); return; } + QoreSandboxManagerHelper smh; + if (smh && !smh->checkFilesystemAccess(path.c_str(), QSEC_WRITE | QSEC_CREATE, xsink)) { + return; + } + if (qore_check_io_interrupt(xsink, "PNG file write")) { + return; + } cairo_status_t status = cairo_surface_write_to_png(surface, path.c_str()); if (status != CAIRO_STATUS_SUCCESS) { xsink->raiseException("CAIRO-ERROR", "failed to write PNG to '%s': %s", diff --git a/src/CairoSvgReader.cpp b/src/CairoSvgReader.cpp index 35a5184..d18d6f3 100644 --- a/src/CairoSvgReader.cpp +++ b/src/CairoSvgReader.cpp @@ -31,6 +31,13 @@ #ifdef HAVE_RSVG QoreCairoSvgReader::QoreCairoSvgReader(const std::string& path, ExceptionSink* xsink) { + QoreSandboxManagerHelper smh; + if (smh && !smh->checkFilesystemAccess(path.c_str(), QSEC_READ, xsink)) { + return; + } + if (qore_check_io_interrupt(xsink, "SVG file load")) { + return; + } GError* error = nullptr; GFile* file = g_file_new_for_path(path.c_str()); handle = rsvg_handle_new_from_gfile_sync(file, RSVG_HANDLE_FLAGS_NONE, nullptr, &error); diff --git a/src/QC_CairoContext.qpp b/src/QC_CairoContext.qpp index 134a6b0..60d2aad 100644 --- a/src/QC_CairoContext.qpp +++ b/src/QC_CairoContext.qpp @@ -214,6 +214,7 @@ nothing CairoContext::setSourceRgba(float r, float g, float b, float a) { @param y Y offset for the surface origin */ nothing CairoContext::setSourceSurface(CairoSurface[QoreCairoSurface] surface, float x, float y) { + ReferenceHolder holder(surface, xsink); ctx->setSourceSurface(surface, x, y, xsink); } @@ -221,6 +222,7 @@ nothing CairoContext::setSourceSurface(CairoSurface[QoreCairoSurface] surface, f /** @param pattern the source pattern */ nothing CairoContext::setSource(CairoPattern[QoreCairoPattern] pattern) { + ReferenceHolder holder(pattern, xsink); ctx->setSource(pattern->getPattern(), xsink); } diff --git a/src/QC_CairoPattern.qpp b/src/QC_CairoPattern.qpp index 6384e09..389826a 100644 --- a/src/QC_CairoPattern.qpp +++ b/src/QC_CairoPattern.qpp @@ -37,6 +37,13 @@ */ qclass CairoPattern [arg=QoreCairoPattern* pattern; ns=Qore::Cairo]; +//! Private constructor - use static factory methods instead +/** @throw CAIRO-ERROR always; CairoPattern objects must be created using static factory methods +*/ +private CairoPattern::constructor() { + xsink->raiseException("CAIRO-ERROR", "CairoPattern cannot be constructed directly; use static factory methods"); +} + //! Creates a linear gradient pattern /** @param x0 start point X @param y0 start point Y @@ -102,6 +109,7 @@ static CairoPattern CairoPattern::createRgba(float r, float g, float b, float a) /** @param surface the source surface */ static CairoPattern CairoPattern::createForSurface(CairoSurface[QoreCairoSurface] surface) { + ReferenceHolder surface_holder(surface, xsink); ReferenceHolder holder(new QoreCairoPattern(surface, xsink), xsink); if (*xsink) { return QoreValue(); diff --git a/src/QC_CairoSurface.qpp b/src/QC_CairoSurface.qpp index 97e93d2..0be28c9 100644 --- a/src/QC_CairoSurface.qpp +++ b/src/QC_CairoSurface.qpp @@ -117,6 +117,13 @@ hashdecl Qore::Cairo::CairoTextExtents { */ qclass CairoSurface [arg=QoreCairoSurface* surface; ns=Qore::Cairo]; +//! Private constructor - use static factory methods instead +/** @throw CAIRO-ERROR always; CairoSurface objects must be created using static factory methods +*/ +private CairoSurface::constructor() { + xsink->raiseException("CAIRO-ERROR", "CairoSurface cannot be constructed directly; use static factory methods"); +} + //! Creates an SVG surface writing to a file /** @param path output file path @param width surface width in points diff --git a/src/QC_CairoSvgReader.qpp b/src/QC_CairoSvgReader.qpp index 0f02de2..12f6d40 100644 --- a/src/QC_CairoSvgReader.qpp +++ b/src/QC_CairoSvgReader.qpp @@ -95,6 +95,7 @@ hash CairoSvgReader::getDimensions() { /** @param ctx the target drawing context */ nothing CairoSvgReader::renderTo(CairoContext[QoreCairoContext] ctx) { + ReferenceHolder holder(ctx, xsink); reader->renderTo(ctx, xsink); } diff --git a/src/cairo-module.h b/src/cairo-module.h index 4c55307..dc50cc8 100644 --- a/src/cairo-module.h +++ b/src/cairo-module.h @@ -27,6 +27,7 @@ #define QORE_CAIRO_MODULE_H #include +#include extern const TypedHashDecl* hashdeclCairoSurfaceInfo; extern const TypedHashDecl* hashdeclCairoMatrix; From 0af252f9632ed66dd896f1583bc35cd3520f0184 Mon Sep 17 00:00:00 2001 From: David Nichols Date: Mon, 16 Feb 2026 09:57:04 +0100 Subject: [PATCH 2/2] Fix documentation build: add footer template, two-phase doc build, and fix warnings - Add docs/footer_template.html required by Doxygen template - Implement two-phase doc build in CMakeLists.txt per qore-module-structure.md: initial pass suppresses WARN_IF_DOC_ERROR, final pass includes CairoDataProvider.tag for cross-module references - Escape in data provider doc comments to prevent Doxygen HTML tag warnings - Note: build requires cmake -DCMAKE_INSTALL_PREFIX=/usr to match Qore installation prefix Co-Authored-By: Claude Opus 4.6 --- CMakeLists.txt | 24 +++++++++++++++++++ docs/footer_template.html | 8 +++++++ .../CairoImageDataProvider.qc | 2 +- .../CairoPostScriptDataProvider.qc | 2 +- .../CairoDataProvider/CairoSvgDataProvider.qc | 2 +- 5 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 docs/footer_template.html diff --git a/CMakeLists.txt b/CMakeLists.txt index 2d78af3..6fbd52d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -143,4 +143,28 @@ if (DOXYGEN_FOUND) qore_wrap_dox(QORE_DOX_SRC ${QORE_DOX_TMPL_SRC}) add_custom_target(QORE_MOD_DOX_FILES DEPENDS ${QORE_DOX_SRC}) add_dependencies(docs-module QORE_MOD_DOX_FILES) + + # Two-phase doc build: the initial pass (docs-module) generates cairo.tag with empty + # TAGFILES. User module docs (docs-CairoDataProvider) are then built using cairo.tag, + # generating their own tag files. The final pass (docs-module-final) rebuilds the binary + # module docs with user module tag files in TAGFILES, enabling @ref cross-references + # from the mainpage into user module symbols. + + # Suppress unresolved cross-reference warnings in the initial pass + file(APPEND ${CMAKE_BINARY_DIR}/Doxyfile "\n# Suppress warnings for initial pass (no user module TAGFILES available)\nWARN_IF_DOC_ERROR = NO\n") + + # Create final-pass Doxyfile from the binary module's Doxyfile with user module tag files + file(COPY_FILE ${CMAKE_BINARY_DIR}/Doxyfile ${CMAKE_BINARY_DIR}/Doxyfile.final) + file(APPEND ${CMAKE_BINARY_DIR}/Doxyfile.final + "\n# Final pass: enable user module cross-references and re-enable doc error warnings\nTAGFILES = ${CMAKE_BINARY_DIR}/CairoDataProvider.tag=../CairoDataProvider/html\nWARN_IF_DOC_ERROR = YES\n") + + add_custom_target(docs-module-final + COMMAND ${DOXYGEN_EXECUTABLE} ${CMAKE_BINARY_DIR}/Doxyfile.final + COMMAND ${QORE_DOCS_ENV} ${QORE_QDX_COMMAND} --post ${CMAKE_BINARY_DIR}/docs/${module_name}/html ${CMAKE_BINARY_DIR}/docs/${module_name}/html/search + WORKING_DIRECTORY ${CMAKE_BINARY_DIR} + COMMENT "Generating API documentation with Doxygen (final pass with user module cross-references)" + VERBATIM + ) + add_dependencies(docs-module-final docs-CairoDataProvider) + add_dependencies(docs docs-module-final) endif() diff --git a/docs/footer_template.html b/docs/footer_template.html new file mode 100644 index 0000000..afbabdb --- /dev/null +++ b/docs/footer_template.html @@ -0,0 +1,8 @@ + + + + + diff --git a/qlib/CairoDataProvider/CairoImageDataProvider.qc b/qlib/CairoDataProvider/CairoImageDataProvider.qc index 740deb8..7f301e6 100644 --- a/qlib/CairoDataProvider/CairoImageDataProvider.qc +++ b/qlib/CairoDataProvider/CairoImageDataProvider.qc @@ -32,7 +32,7 @@ public namespace CairoDataProvider { - @b draw - Draw on existing PNG @par Data Provider Path - Access via: cairo{}/image/ + Access via: @verbatim cairo{}/image/ @endverbatim */ public class CairoImageDataProvider inherits AbstractDataProvider { public { diff --git a/qlib/CairoDataProvider/CairoPostScriptDataProvider.qc b/qlib/CairoDataProvider/CairoPostScriptDataProvider.qc index 4731c0e..07baa63 100644 --- a/qlib/CairoDataProvider/CairoPostScriptDataProvider.qc +++ b/qlib/CairoDataProvider/CairoPostScriptDataProvider.qc @@ -30,7 +30,7 @@ public namespace CairoDataProvider { - @b create - Create PostScript or EPS from draw commands @par Data Provider Path - Access via: cairo{}/postscript/ + Access via: @verbatim cairo{}/postscript/ @endverbatim */ public class CairoPostScriptDataProvider inherits AbstractDataProvider { public { diff --git a/qlib/CairoDataProvider/CairoSvgDataProvider.qc b/qlib/CairoDataProvider/CairoSvgDataProvider.qc index c418c60..73668f2 100644 --- a/qlib/CairoDataProvider/CairoSvgDataProvider.qc +++ b/qlib/CairoDataProvider/CairoSvgDataProvider.qc @@ -34,7 +34,7 @@ public namespace CairoDataProvider { - @b to-eps - Convert SVG to EPS (requires librsvg) @par Data Provider Path - Access via: cairo{}/svg/ + Access via: @verbatim cairo{}/svg/ @endverbatim */ public class CairoSvgDataProvider inherits AbstractDataProvider { public {