Dix/more fixes

.github/workflows/ci.yml

@@ -111,6 +111,74 @@ jobs:
             --with-plutobook-lib="$PAGE_PRINT_PLUTOBOOK_PREFIX/lib"
           bundle exec rake test
 
+  native-sanitizers:
+    runs-on: ubuntu-22.04
+
+    env:
+      PAGE_PRINT_PLUTOBOOK_PREFIX: ${{ github.workspace }}/tmp/plutobook-sanitizer-install
+      PKG_CONFIG_PATH: ${{ github.workspace }}/tmp/plutobook-sanitizer-install/lib/pkgconfig
+      LD_LIBRARY_PATH: ${{ github.workspace }}/tmp/plutobook-sanitizer-install/lib
+      ASAN_OPTIONS: detect_leaks=0:halt_on_error=1:use_sigaltstack=0
+      UBSAN_OPTIONS: halt_on_error=1:print_stacktrace=1
+      PAGE_PRINT_CONCURRENCY_ITERATIONS: 10
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install native dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y --no-install-recommends \
+            build-essential \
+            git \
+            pkg-config \
+            ninja-build \
+            pipx \
+            python3-pip \
+            libcairo2-dev \
+            libexpat1-dev \
+            libicu-dev \
+            libfreetype6-dev \
+            libfontconfig1-dev \
+            libharfbuzz-dev
+          pipx install meson==1.3.2
+          echo "$HOME/.local/bin" >> "$GITHUB_PATH"
+
+      - uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: "3.4"
+          bundler-cache: true
+
+      - name: Build PlutoBook with sanitizers
+        run: |
+          git clone --depth 1 --branch v0.19.0 https://github.com/plutoprint/plutobook.git tmp/plutobook-sanitizer-src
+          meson setup tmp/plutobook-sanitizer-build tmp/plutobook-sanitizer-src \
+            --prefix="$PAGE_PRINT_PLUTOBOOK_PREFIX" \
+            --libdir=lib \
+            --buildtype=debugoptimized \
+            -Db_sanitize=address,undefined \
+            -Dcpp_args="['-include', 'memory_resource']" \
+            --force-fallback-for=harfbuzz \
+            -Dcurl=disabled \
+            -Dturbojpeg=disabled \
+            -Dwebp=disabled \
+            -Dtools=disabled \
+            -Dtests=disabled \
+            -Dexamples=disabled
+          meson compile -C tmp/plutobook-sanitizer-build
+          meson install -C tmp/plutobook-sanitizer-build
+
+      - name: Compile extension and run tests with sanitizers
+        env:
+          CFLAGS: -fsanitize=address,undefined -fno-omit-frame-pointer
+          LDFLAGS: -fsanitize=address,undefined
+        run: |
+          export LD_PRELOAD="$(gcc -print-file-name=libasan.so)"
+          bundle exec rake compile -- \
+            --with-plutobook-include="$PAGE_PRINT_PLUTOBOOK_PREFIX/include" \
+            --with-plutobook-lib="$PAGE_PRINT_PLUTOBOOK_PREFIX/lib"
+          bundle exec rake test
+
   build-plutobook-macos:
     runs-on: macos-14
 

ext/page_print/page_print.c

@@ -2,6 +2,7 @@
 #include "ruby/encoding.h"
 #include "ruby/thread.h"
 #include <limits.h>
+#include <math.h>
 #include <stdint.h>
 #include <string.h>
 #include <plutobook/plutobook.h>
@@ -56,6 +57,7 @@ typedef struct {
 
 typedef struct {
     VALUE object;
+    VALUE error;
     int state;
 } pageprint_resource_fetcher_t;
 
@@ -67,10 +69,12 @@ typedef struct {
 
 typedef struct {
     VALUE output;
+    VALUE error;
     int state;
 } pageprint_pdf_string_output_t;
 
 typedef struct {
+    pageprint_pdf_string_output_t *owner;
     VALUE output;
     const char *data;
     unsigned int length;
@@ -168,6 +172,12 @@ static void PAGEPRINT_NORETURN pageprint_raise_plutobook_error_with_path(VALUE e
     rb_raise(error_class, "%s %s", message, path);
 }
 
+static void PAGEPRINT_NORETURN pageprint_jump_callback_error(int state, VALUE error)
+{
+    rb_set_errinfo(error);
+    rb_jump_tag(state);
+}
+
 static double pageprint_unit_factor_from_value(VALUE value, const char *name)
 {
     ID value_id;
@@ -215,6 +225,14 @@ static plutobook_page_size_t pageprint_page_size_from_value(VALUE value)
         width_number = NUM2DBL(width);
         height_number = NUM2DBL(height);
 
+        if (!isfinite(width_number)) {
+            rb_raise(rb_eArgError, "page_size width must be finite");
+        }
+
+        if (!isfinite(height_number)) {
+            rb_raise(rb_eArgError, "page_size height must be finite");
+        }
+
         if (width_number <= 0) {
             rb_raise(rb_eArgError, "page_size width must be greater than 0");
         }
@@ -287,6 +305,10 @@ static plutobook_page_margins_t pageprint_margins_from_value(VALUE value)
         bottom_number = NUM2DBL(bottom);
         left_number = NUM2DBL(left);
 
+        if (!isfinite(top_number) || !isfinite(right_number) || !isfinite(bottom_number) || !isfinite(left_number)) {
+            rb_raise(rb_eArgError, "margins values must be finite");
+        }
+
         if (top_number < 0 || right_number < 0 || bottom_number < 0 || left_number < 0) {
             rb_raise(rb_eArgError, "margins values must be greater than or equal to 0");
         }
@@ -450,11 +472,17 @@ static void *pageprint_write_pdf_without_gvl(void *ptr)
 static void *pageprint_append_pdf_string_with_gvl(void *ptr)
 {
     pageprint_pdf_string_append_t *append = ptr;
+    pageprint_pdf_string_output_t *output = append->owner;
     int state = 0;
 
     rb_protect(pageprint_append_pdf_string, (VALUE)append, &state);
 
-    return (void *)(intptr_t)state;
+    if (state && !output->state) {
+        output->state = state;
+        output->error = rb_errinfo();
+    }
+
+    return NULL;
 }
 
 static void *pageprint_write_pdf_stream_without_gvl(void *ptr)
@@ -532,24 +560,31 @@ static void *pageprint_call_resource_fetcher_with_gvl(void *ptr)
 
     rb_protect(pageprint_call_resource_fetcher, (VALUE)args, &state);
 
-    return (void *)(intptr_t)state;
+    if (state && !args->fetcher->state) {
+        args->fetcher->state = state;
+        args->fetcher->error = rb_errinfo();
+    }
+
+    return NULL;
 }
 
 static plutobook_resource_data_t *pageprint_fetch_resource(void *closure, const char *url)
 {
     pageprint_resource_fetcher_t *fetcher = closure;
     pageprint_resource_fetch_args_t args;
-    int state;
+
+    if (fetcher->state) {
+        return NULL;
+    }
 
     if (!NIL_P(fetcher->object)) {
         args.fetcher = fetcher;
         args.url = url;
         args.resource = NULL;
 
-        state = (int)(intptr_t)rb_thread_call_with_gvl(pageprint_call_resource_fetcher_with_gvl, &args);
+        rb_thread_call_with_gvl(pageprint_call_resource_fetcher_with_gvl, &args);
 
-        if (state) {
-            fetcher->state = state;
+        if (fetcher->state) {
             plutobook_set_error_message("failed to fetch URL '%s'", url);
             return NULL;
         }
@@ -689,6 +724,7 @@ static void pageprint_create_book_from_html(VALUE html, VALUE options, pageprint
     }
 
     context->resource_fetcher_state.object = context->resource_fetcher;
+    context->resource_fetcher_state.error = Qnil;
     context->resource_fetcher_state.state = 0;
 
     plutobook_set_custom_resource_fetcher(context->book, pageprint_fetch_resource, &context->resource_fetcher_state);
@@ -721,14 +757,20 @@ static void pageprint_create_book_from_html(VALUE html, VALUE options, pageprint
 
     if (!load_args.ok) {
         if (context->resource_fetcher_state.state) {
-            rb_jump_tag(context->resource_fetcher_state.state);
+            pageprint_jump_callback_error(
+                context->resource_fetcher_state.state,
+                context->resource_fetcher_state.error
+            );
         }
 
         pageprint_raise_plutobook_error(rb_eRuntimeError, "failed to load HTML into plutobook");
     }
 
     if (context->resource_fetcher_state.state) {
-        rb_jump_tag(context->resource_fetcher_state.state);
+        pageprint_jump_callback_error(
+            context->resource_fetcher_state.state,
+            context->resource_fetcher_state.error
+        );
     }
 
     {
@@ -760,16 +802,19 @@ static plutobook_stream_status_t pageprint_write_pdf_string(void *closure, const
 {
     pageprint_pdf_string_output_t *output = closure;
     pageprint_pdf_string_append_t append;
-    int state;
 
+    if (output->state) {
+        return PLUTOBOOK_STREAM_STATUS_WRITE_ERROR;
+    }
+
+    append.owner = output;
     append.output = output->output;
     append.data = data;
     append.length = length;
 
-    state = (int)(intptr_t)rb_thread_call_with_gvl(pageprint_append_pdf_string_with_gvl, &append);
+    rb_thread_call_with_gvl(pageprint_append_pdf_string_with_gvl, &append);
 
-    if (state) {
-        output->state = state;
+    if (output->state) {
         return PLUTOBOOK_STREAM_STATUS_WRITE_ERROR;
     }
 
@@ -814,11 +859,15 @@ static VALUE pageprint_render_to_file_body(VALUE value)
 
     RB_GC_GUARD(context->path);
     RB_GC_GUARD(context->book_context.resource_fetcher);
+    RB_GC_GUARD(context->book_context.resource_fetcher_state.error);
     RB_GC_GUARD(context->html);
     RB_GC_GUARD(context->options);
 
     if (context->book_context.resource_fetcher_state.state) {
-        rb_jump_tag(context->book_context.resource_fetcher_state.state);
+        pageprint_jump_callback_error(
+            context->book_context.resource_fetcher_state.state,
+            context->book_context.resource_fetcher_state.error
+        );
     }
 
     if (!write_args.ok) {
@@ -872,16 +921,21 @@ static VALUE pageprint_render_body(VALUE value)
     );
 
     RB_GC_GUARD(context->book_context.resource_fetcher);
+    RB_GC_GUARD(context->book_context.resource_fetcher_state.error);
     RB_GC_GUARD(context->output.output);
+    RB_GC_GUARD(context->output.error);
     RB_GC_GUARD(context->html);
     RB_GC_GUARD(context->options);
 
     if (context->book_context.resource_fetcher_state.state) {
-        rb_jump_tag(context->book_context.resource_fetcher_state.state);
+        pageprint_jump_callback_error(
+            context->book_context.resource_fetcher_state.state,
+            context->book_context.resource_fetcher_state.error
+        );
     }
 
     if (context->output.state) {
-        rb_jump_tag(context->output.state);
+        pageprint_jump_callback_error(context->output.state, context->output.error);
     }
 
     if (!write_args.ok) {
@@ -900,6 +954,7 @@ static VALUE pageprint_render(int argc, VALUE *argv, VALUE self)
     context.html = argv[0];
     context.options = argc == 2 ? argv[1] : Qnil;
     context.output.output = rb_str_new(NULL, 0);
+    context.output.error = Qnil;
     context.output.state = 0;
     rb_enc_associate_index(context.output.output, rb_ascii8bit_encindex());
 

lib/page_print/rails_resource_fetcher.rb

@@ -45,11 +45,14 @@ def read_public_asset(path)
       return unless public_path
 
       relative_path = path.delete_prefix('/')
-      file_path = public_path.join(relative_path).cleanpath
+      public_path = public_path.realpath
+      file_path = public_path.join(relative_path).realpath
       return unless inside_path?(file_path, public_path)
       return unless file_path.file?
 
       resource(File.binread(file_path), file_path.extname)
+    rescue Errno::ENOENT, Errno::EACCES, Errno::ELOOP
+      nil
     end
 
     def read_resolved_asset(path)

test/concurrency_test.rb

@@ -0,0 +1,46 @@
+require 'minitest/autorun'
+require 'tmpdir'
+
+$LOAD_PATH.unshift(File.expand_path('../lib', __dir__))
+require_relative '../lib/page_print'
+
+class PagePrintConcurrencyTest < Minitest::Test
+  def test_concurrent_renders
+    thread_count = 4
+    iterations = Integer(ENV.fetch('PAGE_PRINT_CONCURRENCY_ITERATIONS', '2'))
+
+    threads = thread_count.times.map do |thread_index|
+      Thread.new do
+        iterations.times do |iteration|
+          render_concurrently(thread_index, iteration)
+        end
+      end
+    end
+
+    assert_equal [iterations] * thread_count, threads.map(&:value)
+  end
+
+  private
+    def render_concurrently(thread_index, iteration)
+      asset_url = "custom:style-#{thread_index}-#{iteration}"
+      fetched_urls = []
+      html = %(<html><head><link rel="stylesheet" href="#{asset_url}"></head><body><h1>Hello</h1></body></html>)
+      options = {
+        resource_fetcher: lambda { |url|
+          fetched_urls << url
+          { content: 'body { color: navy; }', mime_type: 'text/css' }
+        }
+      }
+
+      pdf = PagePrint.render(html, **options)
+      raise 'invalid streamed PDF' unless pdf.start_with?('%PDF')
+
+      Dir.mktmpdir do |dir|
+        path = File.join(dir, 'output.pdf')
+        PagePrint.render_to_file(html, path, **options)
+        raise 'invalid file PDF' unless File.binread(path, 4) == '%PDF'
+      end
+
+      raise "resource fetch mismatch: #{fetched_urls.inspect}" unless fetched_urls == [asset_url, asset_url]
+    end
+end