From e6d424ef66659dd7ce2230fb9e55f33bd9aa2949 Mon Sep 17 00:00:00 2001 From: Dino Maric Date: Tue, 18 Aug 2026 13:42:19 +0200 Subject: [PATCH 1/5] Preserve first callback exception --- ext/page_print/page_print.c | 72 +++++++++++++++++++++++++++++-------- test/page_print_test.rb | 23 ++++++++++++ 2 files changed, 80 insertions(+), 15 deletions(-) diff --git a/ext/page_print/page_print.c b/ext/page_print/page_print.c index de4ef8c..8d86dee 100644 --- a/ext/page_print/page_print.c +++ b/ext/page_print/page_print.c @@ -56,6 +56,7 @@ typedef struct { typedef struct { VALUE object; + VALUE error; int state; } pageprint_resource_fetcher_t; @@ -67,10 +68,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 +171,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; @@ -450,11 +459,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 +547,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 +711,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 +744,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 +789,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 +846,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 +908,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 +941,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()); diff --git a/test/page_print_test.rb b/test/page_print_test.rb index 7b20b1a..6382c96 100644 --- a/test/page_print_test.rb +++ b/test/page_print_test.rb @@ -517,6 +517,29 @@ def test_render_reraises_resource_fetcher_errors assert_equal 'fetch failed', error.message end + def test_render_preserves_first_resource_fetcher_error + calls = [] + html = <<~HTML + + + +

Hello

+ HTML + + error = assert_raises(RuntimeError) do + PagePrint.render( + html, + resource_fetcher: lambda { |url| + calls << url + raise calls.one? ? 'first fetch failed' : 'later fetch failed' + } + ) + end + + assert_equal 'first fetch failed', error.message + assert_equal 1, calls.length + end + def test_render_denies_network_fetch_when_resource_fetcher_returns_nil html = '' urls = [] From 41e17d2024e087e578d00f22b09e121d22188432 Mon Sep 17 00:00:00 2001 From: Dino Maric Date: Tue, 18 Aug 2026 13:42:19 +0200 Subject: [PATCH 2/5] Prevent Rails asset symlink escapes --- lib/page_print/rails_resource_fetcher.rb | 5 ++++- test/page_print_test.rb | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/page_print/rails_resource_fetcher.rb b/lib/page_print/rails_resource_fetcher.rb index 90d10d9..041ec09 100644 --- a/lib/page_print/rails_resource_fetcher.rb +++ b/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) diff --git a/test/page_print_test.rb b/test/page_print_test.rb index 6382c96..3827d23 100644 --- a/test/page_print_test.rb +++ b/test/page_print_test.rb @@ -62,6 +62,20 @@ def test_rails_resource_fetcher_rejects_path_traversal_outside_public_path end end + def test_rails_resource_fetcher_rejects_symlinks_outside_public_path + Dir.mktmpdir do |dir| + public_dir = File.join(dir, 'public') + assets_dir = File.join(public_dir, 'assets') + FileUtils.mkdir_p(assets_dir) + File.binwrite(File.join(dir, 'secret.css'), 'body { content: "secret"; }') + File.symlink(File.join(dir, 'secret.css'), File.join(assets_dir, 'secret.css')) + + fetcher = PagePrint::RailsResourceFetcher.new(rails: fake_rails(public_dir)) + + assert_nil fetcher.call('http://example.com/assets/secret.css') + end + end + def test_rails_resource_fetcher_uses_propshaft_load_path_for_digested_assets asset = Struct.new(:logical_path).new('pdf.css') load_path = Class.new do From 1057f2d77395796028bcf876db8c9274bcb5b37e Mon Sep 17 00:00:00 2001 From: Dino Maric Date: Tue, 18 Aug 2026 13:42:19 +0200 Subject: [PATCH 3/5] Reject non-finite dimensions --- ext/page_print/page_print.c | 13 +++++++++++++ test/page_print_test.rb | 28 ++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/ext/page_print/page_print.c b/ext/page_print/page_print.c index 8d86dee..cabe92d 100644 --- a/ext/page_print/page_print.c +++ b/ext/page_print/page_print.c @@ -2,6 +2,7 @@ #include "ruby/encoding.h" #include "ruby/thread.h" #include +#include #include #include #include @@ -224,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"); } @@ -296,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"); } diff --git a/test/page_print_test.rb b/test/page_print_test.rb index 3827d23..884d3a3 100644 --- a/test/page_print_test.rb +++ b/test/page_print_test.rb @@ -784,6 +784,20 @@ def test_render_to_file_rejects_non_positive_custom_page_size assert_equal 'page_size width must be greater than 0', error.message end + def test_render_to_file_rejects_non_finite_custom_page_size + [[Float::NAN, 150, 'width'], [100, Float::INFINITY, 'height']].each do |width, height, dimension| + error = assert_raises(ArgumentError) do + PagePrint.render_to_file( + '

Hello

', + 'output.pdf', + page_size: { width: width, height: height, unit: :mm } + ) + end + + assert_equal "page_size #{dimension} must be finite", error.message + end + end + def test_render_to_file_rejects_invalid_margins error = assert_raises(ArgumentError) do PagePrint.render_to_file('

Hello

', 'output.pdf', margins: :compact) @@ -816,6 +830,20 @@ def test_render_to_file_rejects_negative_custom_margins assert_equal 'margins values must be greater than or equal to 0', error.message end + def test_render_to_file_rejects_non_finite_custom_margins + [Float::NAN, Float::INFINITY, -Float::INFINITY].each do |value| + error = assert_raises(ArgumentError) do + PagePrint.render_to_file( + '

Hello

', + 'output.pdf', + margins: { top: 1, right: value, bottom: 1, left: 1, unit: :mm } + ) + end + + assert_equal 'margins values must be finite', error.message + end + end + def test_render_to_file_rejects_invalid_media error = assert_raises(ArgumentError) do PagePrint.render_to_file('

Hello

', 'output.pdf', media: :speech) From c42a504e13e7a08062644ed4db8ffc60d71b9e05 Mon Sep 17 00:00:00 2001 From: Dino Maric Date: Tue, 18 Aug 2026 13:42:19 +0200 Subject: [PATCH 4/5] Add sanitizer and concurrency CI coverage --- .github/workflows/ci.yml | 68 ++++++++++++++++++++++++++++++++++++++++ test/concurrency_test.rb | 46 +++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 test/concurrency_test.rb diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e83db2f..3421aa4 100644 --- a/.github/workflows/ci.yml +++ b/.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 + 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 diff --git a/test/concurrency_test.rb b/test/concurrency_test.rb new file mode 100644 index 0000000..5e778e8 --- /dev/null +++ b/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 = %(

Hello

) + 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 From 1cf105df3f0e8928179579c581a0426d02a5f28a Mon Sep 17 00:00:00 2001 From: Dino Maric Date: Tue, 18 Aug 2026 14:12:36 +0200 Subject: [PATCH 5/5] Avoid ASan signal stack conflict --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3421aa4..21f2d53 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -118,7 +118,7 @@ jobs: 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 + 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