From b078d77eaad1815f2da8af1c6579afe92058a3fb Mon Sep 17 00:00:00 2001 From: Jordan Owens Date: Sat, 2 Dec 2023 18:50:02 -0500 Subject: [PATCH 1/5] Allow loading html from file uri --- lib/grover/js/processor.cjs | 4 +++- lib/grover/options_builder.rb | 2 +- spec/fixtures/test.html | 6 ++++++ spec/grover/processor_spec.rb | 29 +++++++++++++++++++++++++++++ spec/spec_helper.rb | 4 ++++ 5 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 spec/fixtures/test.html diff --git a/lib/grover/js/processor.cjs b/lib/grover/js/processor.cjs index 3604f2e..0d9cbd4 100644 --- a/lib/grover/js/processor.cjs +++ b/lib/grover/js/processor.cjs @@ -173,7 +173,9 @@ const _processPage = (async (convertAction, urlOrHtml, options) => { } const waitUntil = options.waitUntil; delete options.waitUntil; - if (urlOrHtml.match(/^http/i)) { + const allowFileUrl = options.allowFileUrl; delete options.allowFileUrl; + const urlRegex = allowFileUrl ? /^(http|file)/i : /^http/i; + if (urlOrHtml.match(urlRegex)) { // Request is for a URL, so request it requestOptions.waitUntil = waitUntil || 'networkidle2'; await page.goto(urlOrHtml, requestOptions); diff --git a/lib/grover/options_builder.rb b/lib/grover/options_builder.rb index 7dff268..3f84e7c 100644 --- a/lib/grover/options_builder.rb +++ b/lib/grover/options_builder.rb @@ -45,7 +45,7 @@ def meta_tags end def url_source? - @url.match(/\Ahttp/i) + @url.match(/\A(http|file)/i) end end end diff --git a/spec/fixtures/test.html b/spec/fixtures/test.html new file mode 100644 index 0000000..68ece95 --- /dev/null +++ b/spec/fixtures/test.html @@ -0,0 +1,6 @@ + + + +

Hello World!

+ + \ No newline at end of file diff --git a/spec/grover/processor_spec.rb b/spec/grover/processor_spec.rb index dd32c56..5e5c76c 100644 --- a/spec/grover/processor_spec.rb +++ b/spec/grover/processor_spec.rb @@ -58,6 +58,35 @@ end end + context 'when passing through a valid file URL' do + let(:url_or_html) { "file:///#{fixture_path('test.html')}" } + let(:options) { { allowFileUrl: true } } + + it { is_expected.to start_with "%PDF-1.4\n" } + it { expect(pdf_reader.page_count).to eq 1 } + it { expect(pdf_text_content).to include 'Hello World!' } + end + + context 'when passing through an invalid file URL' do + let(:url_or_html) { 'file:///fake/invalid.html' } + let(:options) { { allowFileUrl: true } } + + it 'raises a JavaScript error that the file could not be found' do + expect do + convert + end.to raise_error Grover::JavaScript::Error, %r{net::ERR_FILE_NOT_FOUND at file:///fake/invalid.html} + end + end + + context 'when passing a file URL and allow_file_url is disabled' do + let(:url_or_html) { "file:///#{fixture_path('test.html')}" } + + it { is_expected.to start_with "%PDF-1.4\n" } + it { expect(pdf_reader.page_count).to eq 1 } + it { expect(pdf_text_content).not_to include 'Hello World!' } + it { expect(pdf_text_content).to include "file:///#{fixture_path('test.html')}" } + end + context 'when passing through an empty string' do let(:url_or_html) { '' } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f42dd22..77cb921 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -29,6 +29,10 @@ MiniMagick.validate_on_create = false +def fixture_path(file) + File.join(File.expand_path(__dir__), 'fixtures', file) +end + def puppeteer_version_on_or_after?(version) puppeteer_version = ENV.fetch('PUPPETEER_VERSION', '') puppeteer_version.empty? || Gem::Version.new(puppeteer_version) >= Gem::Version.new(version) From 539adc8bb939d8bfbca405003b3d06bbd060471d Mon Sep 17 00:00:00 2001 From: Jordan Owens Date: Tue, 16 Jan 2024 11:30:03 -0500 Subject: [PATCH 2/5] Document allow_file_url option in readme --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index b96414c..67a9451 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,8 @@ by passing it to the `execute_script` option. Grover.new(, { execute_script: 'document.getElementsByTagName("footer")[0].innerText = "Hey"' }).to_pdf ``` +The `allow_file_url` option can be used to render an html document from the file system. This should be used with *EXTREME CAUTION*. If used improperly it could potentially be manipulated to reveal sensitive files on the system. Do not enable if rendering content from outside entities (user uploads, external URLs, etc). + #### Basic authentication For requesting a page with basic authentication, `username` and `password` options can be provided. Note that this only really makes sense if you're calling Grover directly (and not via middleware). From f21a347dc8d5c2227139e1a89a4ec8521775f90b Mon Sep 17 00:00:00 2001 From: Jordan Owens Date: Mon, 18 Mar 2024 15:19:16 -0400 Subject: [PATCH 3/5] Apply patch to prevent unsafe configuration --- lib/grover.rb | 4 ++-- lib/grover/errors.rb | 1 + lib/grover/middleware.rb | 2 +- lib/grover/options_builder.rb | 8 +++++++- spec/grover/middleware_spec.rb | 23 +++++++++++++++++------ spec/grover/options_builder_spec.rb | 29 ++++++++++++++++++++++++++++- 6 files changed, 56 insertions(+), 11 deletions(-) diff --git a/lib/grover.rb b/lib/grover.rb index 13b71da..bce4f92 100644 --- a/lib/grover.rb +++ b/lib/grover.rb @@ -33,9 +33,9 @@ class Grover # see https://github.com/puppeteer/puppeteer/blob/main/docs/api/puppeteer.pdfoptions.md # and https://github.com/puppeteer/puppeteer/blob/main/docs/api/puppeteer.screenshotoptions.md # - def initialize(url, **options) + def initialize(url, middleware: false, **options) @url = url.to_s - @options = OptionsBuilder.new(options, @url) + @options = OptionsBuilder.new(options, @url, middleware: middleware) @root_path = @options.delete 'root_path' @front_cover_path = @options.delete 'front_cover_path' @back_cover_path = @options.delete 'back_cover_path' diff --git a/lib/grover/errors.rb b/lib/grover/errors.rb index 367c347..fe999a3 100644 --- a/lib/grover/errors.rb +++ b/lib/grover/errors.rb @@ -15,4 +15,5 @@ def self.const_missing(name) const_set name, Class.new(Error) end end + UnsafeConfigurationError = Class.new(Error) end diff --git a/lib/grover/middleware.rb b/lib/grover/middleware.rb index ec5782a..cbd7e99 100644 --- a/lib/grover/middleware.rb +++ b/lib/grover/middleware.rb @@ -111,7 +111,7 @@ def create_grover_for_response(response) # rubocop:disable Metrics/AbcSize body = body.join if body.is_a?(Array) body = HTMLPreprocessor.process body, root_url, protocol - options = { display_url: request_url } + options = { display_url: request_url, middleware: true } cookies = Rack::Utils.parse_cookies(env).map do |name, value| { name: name, value: Rack::Utils.escape(value), domain: env['HTTP_HOST'] } end diff --git a/lib/grover/options_builder.rb b/lib/grover/options_builder.rb index 3f84e7c..afdd0af 100644 --- a/lib/grover/options_builder.rb +++ b/lib/grover/options_builder.rb @@ -8,7 +8,7 @@ class Grover # Build options from Grover.configuration, meta_options, and passed-in options # class OptionsBuilder < Hash - def initialize(options, url) + def initialize(options, url, middleware:) super() @url = url combined = grover_configuration @@ -16,6 +16,12 @@ def initialize(options, url) Utils.deep_merge! combined, meta_options unless url_source? update OptionsFixer.new(combined).run + + # The combination of middleware and allowing file URLs is exceptionally + # unsafe as it can lead to data exfiltration from the host system. + return unless middleware && (self['allow_file_url'] || self['allowFileUrl']) + + raise UnsafeConfigurationError, 'using `allow_file_url` option with middleware is exceptionally unsafe' end private diff --git a/spec/grover/middleware_spec.rb b/spec/grover/middleware_spec.rb index 13d7ff2..76935ea 100644 --- a/spec/grover/middleware_spec.rb +++ b/spec/grover/middleware_spec.rb @@ -537,11 +537,14 @@ it 'passes through the request URL (sans extension) to Grover' do allow(Grover).to( receive(:new). - with('Grover McGroveryface', display_url: 'http://www.example.org/test'). + with('Grover McGroveryface', display_url: 'http://www.example.org/test', middleware: true). and_return(grover) ) allow(grover).to receive(:to_pdf).with(no_args).and_return 'A converted PDF' - expect(Grover).to receive(:new).with('Grover McGroveryface', display_url: 'http://www.example.org/test') + expect(Grover).to( + receive(:new). + with('Grover McGroveryface', display_url: 'http://www.example.org/test', middleware: true) + ) expect(grover).to receive(:to_pdf).with(no_args) get 'http://www.example.org/test.pdf' expect(last_response.body).to eq 'A converted PDF' @@ -552,12 +555,14 @@ allow(Grover).to receive(:new).with( 'Grover McGroveryface', display_url: 'http://www.example.org/test', + middleware: true, cookies: [{ domain: 'www.example.org', name: k, value: v }] ).and_return(grover) allow(grover).to receive(:to_pdf).with(no_args).and_return 'A converted PDF' expect(Grover).to receive(:new).with( 'Grover McGroveryface', display_url: 'http://www.example.org/test', + middleware: true, cookies: [{ domain: 'www.example.org', name: k, value: v }] ) expect(grover).to receive(:to_pdf).with(no_args) @@ -592,11 +597,14 @@ it 'passes through the request URL (sans extension) to Grover' do allow(Grover).to( receive(:new). - with('Grover McGroveryface', display_url: 'http://www.example.org/test'). + with('Grover McGroveryface', display_url: 'http://www.example.org/test', middleware: true). and_return(grover) ) allow(grover).to receive(:to_png).with(no_args).and_return 'A converted PNG' - expect(Grover).to receive(:new).with('Grover McGroveryface', display_url: 'http://www.example.org/test') + expect(Grover).to( + receive(:new). + with('Grover McGroveryface', display_url: 'http://www.example.org/test', middleware: true) + ) expect(grover).to receive(:to_png).with(no_args) get 'http://www.example.org/test.png' expect(last_response.body).to eq 'A converted PNG' @@ -619,11 +627,14 @@ it 'passes through the request URL (sans extension) to Grover' do allow(Grover).to( receive(:new). - with('Grover McGroveryface', display_url: 'http://www.example.org/test'). + with('Grover McGroveryface', display_url: 'http://www.example.org/test', middleware: true). and_return(grover) ) allow(grover).to receive(:to_jpeg).with(no_args).and_return 'A converted JPEG' - expect(Grover).to receive(:new).with('Grover McGroveryface', display_url: 'http://www.example.org/test') + expect(Grover).to( + receive(:new). + with('Grover McGroveryface', display_url: 'http://www.example.org/test', middleware: true) + ) expect(grover).to receive(:to_jpeg).with(no_args) get 'http://www.example.org/test.jpeg' expect(last_response.body).to eq 'A converted JPEG' diff --git a/spec/grover/options_builder_spec.rb b/spec/grover/options_builder_spec.rb index 821fb46..40fcaba 100644 --- a/spec/grover/options_builder_spec.rb +++ b/spec/grover/options_builder_spec.rb @@ -4,11 +4,12 @@ require 'grover/options_builder' describe Grover::OptionsBuilder do - subject(:built_options) { described_class.new(options, url_or_html) } + subject(:built_options) { described_class.new(options, url_or_html, middleware: middleware) } let(:url_or_html) { 'https://google.com' } let(:options) { {} } let(:global_config) { { cache: false, quality: 95 } } + let(:middleware) { false } before { allow(Grover.configuration).to receive(:options).and_return(global_config) } @@ -157,4 +158,30 @@ it { is_expected.to include('viewport' => { 'height' => 100, 'width' => 200, 'device_scale_factor' => 2.5 }) } end + + context 'when the middleware flag is enabled' do + let(:middleware) { true } + + it 'returns combined passed-in/global options' do + expect(built_options).to eq('cache' => false, 'quality' => 95) + end + + context 'when providing `allow_file_url` option inline' do + let(:options) { { allow_file_url: true } } + + it 'raises `Grover::UnsafeConfigurationError`' do + expect { built_options }.to raise_error Grover::UnsafeConfigurationError, + 'using `allow_file_url` option with middleware is exceptionally unsafe' + end + end + + context 'when providing `allow_file_url` option through global options' do + let(:global_config) { { allow_file_url: true } } + + it 'raises `Grover::UnsafeConfigurationError`' do + expect { built_options }.to raise_error Grover::UnsafeConfigurationError, + 'using `allow_file_url` option with middleware is exceptionally unsafe' + end + end + end end From 126a971ff36f2ad6e6bd367cad83268de8ead5ca Mon Sep 17 00:00:00 2001 From: Andrew Bromwich Date: Tue, 2 Jul 2024 00:47:57 +1000 Subject: [PATCH 4/5] Switch allow_file_uri to non-overridable Grover.configuration option --- README.md | 30 +- lib/grover.rb | 29 +- lib/grover/configuration.rb | 3 +- lib/grover/js/processor.cjs | 12 +- lib/grover/middleware.rb | 12 +- lib/grover/options_builder.rb | 18 +- spec/grover/middleware_spec.rb | 14 +- spec/grover/options_builder_spec.rb | 29 +- spec/grover/processor_spec.rb | 12 +- spec/grover_spec.rb | 490 ++++++++++++++++++++++++---- 10 files changed, 503 insertions(+), 146 deletions(-) diff --git a/README.md b/README.md index 67a9451..37c85a0 100644 --- a/README.md +++ b/README.md @@ -171,8 +171,6 @@ by passing it to the `execute_script` option. Grover.new(, { execute_script: 'document.getElementsByTagName("footer")[0].innerText = "Hey"' }).to_pdf ``` -The `allow_file_url` option can be used to render an html document from the file system. This should be used with *EXTREME CAUTION*. If used improperly it could potentially be manipulated to reveal sensitive files on the system. Do not enable if rendering content from outside entities (user uploads, external URLs, etc). - #### Basic authentication For requesting a page with basic authentication, `username` and `password` options can be provided. Note that this only really makes sense if you're calling Grover directly (and not via middleware). @@ -354,7 +352,7 @@ Grover.configure do |config| end ``` -#### ignore_path +### ignore_path The `ignore_path` configuration option can be used to tell Grover's middleware whether it should handle/modify the response. There are three ways to set up the `ignore_path`: * a `String` which matches the start of the request path. @@ -380,7 +378,7 @@ Grover.configure do |config| end ``` -#### ignore_request +### ignore_request The `ignore_request` configuration option can be used to tell Grover's middleware whether it should handle/modify the response. It should be set with a `Proc` which accepts the request (Rack::Request) as a parameter. @@ -400,6 +398,30 @@ Grover.configure do |config| end ``` +### allow_file_uris +The `allow_file_uris` option can be used to render an html document from the file system. +This should be used with *EXTREME CAUTION*. If used improperly it could potentially be manipulated to reveal +sensitive files on the system. Do not enable if rendering content from outside entities +(user uploads, external URLs, etc). + +It defaults to `false` preventing local system files from being read. + +```ruby +# config/initializers/grover.rb +Grover.configure do |config| + config.allow_file_uris = true +end +``` + +And used as such: +```ruby +# Grover.new accepts a file URI and optional parameters for Puppeteer +grover = Grover.new('file:///some/local/file.html', format: 'A4') + +# Get an inline PDF of the local file +pdf = grover.to_pdf +``` + ## Cover pages Since the header/footer for Puppeteer is configured globally, displaying of front/back cover diff --git a/lib/grover.rb b/lib/grover.rb index bce4f92..3d09f8f 100644 --- a/lib/grover.rb +++ b/lib/grover.rb @@ -28,40 +28,40 @@ class Grover attr_reader :front_cover_path, :back_cover_path # - # @param [String] url URL of the page to convert + # @param [String] uri URI of the page to convert # @param [Hash] options Optional parameters to pass to PDF processor # see https://github.com/puppeteer/puppeteer/blob/main/docs/api/puppeteer.pdfoptions.md # and https://github.com/puppeteer/puppeteer/blob/main/docs/api/puppeteer.screenshotoptions.md # - def initialize(url, middleware: false, **options) - @url = url.to_s - @options = OptionsBuilder.new(options, @url, middleware: middleware) + def initialize(uri, **options) + @uri = uri.to_s + @options = OptionsBuilder.new(options, @uri) @root_path = @options.delete 'root_path' @front_cover_path = @options.delete 'front_cover_path' @back_cover_path = @options.delete 'back_cover_path' end # - # Request URL with provided options and create PDF + # Request URI with provided options and create PDF # # @param [String] path Optional path to write the PDF to # @return [String] The resulting PDF data # def to_pdf(path = nil) - processor.convert :pdf, @url, normalized_options(path: path) + processor.convert :pdf, @uri, normalized_options(path: path) end # - # Request URL with provided options and render HTML + # Request URI with provided options and render HTML # # @return [String] The resulting HTML string # def to_html - processor.convert :content, @url, normalized_options(path: nil) + processor.convert :content, @uri, normalized_options(path: nil) end # - # Request URL with provided options and create screenshot + # Request URI with provided options and create screenshot # # @param [String] path Optional path to write the screenshot to # @param [String] format Optional format of the screenshot @@ -70,11 +70,11 @@ def to_html def screenshot(path: nil, format: nil) options = normalized_options(path: path) options['type'] = format if %w[png jpeg].include? format - processor.convert :screenshot, @url, options + processor.convert :screenshot, @uri, options end # - # Request URL with provided options and create PNG + # Request URI with provided options and create PNG # # @param [String] path Optional path to write the screenshot to # @return [String] The resulting PNG data @@ -84,7 +84,7 @@ def to_png(path = nil) end # - # Request URL with provided options and create JPEG + # Request URI with provided options and create JPEG # # @param [String] path Optional path to write the screenshot to # @return [String] The resulting JPEG data @@ -116,10 +116,10 @@ def show_back_cover? # def inspect format( - '#<%s:0x%p @url="%s">', + '#<%s:0x%p @uri="%s">', class_name: self.class.name, object_id: object_id, - url: @url + uri: @uri ) end @@ -147,6 +147,7 @@ def processor def normalized_options(path:) normalized_options = Utils.normalize_object @options, excluding: ['extraHTTPHeaders'] normalized_options['path'] = path if path.is_a? ::String + normalized_options['allowFileUri'] = Grover.configuration.allow_file_uris == true normalized_options end end diff --git a/lib/grover/configuration.rb b/lib/grover/configuration.rb index b03e17e..d015f0f 100644 --- a/lib/grover/configuration.rb +++ b/lib/grover/configuration.rb @@ -7,7 +7,7 @@ class Grover class Configuration attr_accessor :options, :meta_tag_prefix, :ignore_path, :ignore_request, :root_url, :use_pdf_middleware, :use_png_middleware, - :use_jpeg_middleware, :node_env_vars + :use_jpeg_middleware, :node_env_vars, :allow_file_uris def initialize @options = {} @@ -19,6 +19,7 @@ def initialize @use_png_middleware = false @use_jpeg_middleware = false @node_env_vars = {} + @allow_file_uris = false end end end diff --git a/lib/grover/js/processor.cjs b/lib/grover/js/processor.cjs index 0d9cbd4..587498d 100644 --- a/lib/grover/js/processor.cjs +++ b/lib/grover/js/processor.cjs @@ -26,7 +26,7 @@ const fs = require('fs'); const os = require('os'); const path = require('path'); -const _processPage = (async (convertAction, urlOrHtml, options) => { +const _processPage = (async (convertAction, uriOrHtml, options) => { let browser, page, errors = [], tmpDir, wsConnection = false; try { @@ -173,12 +173,12 @@ const _processPage = (async (convertAction, urlOrHtml, options) => { } const waitUntil = options.waitUntil; delete options.waitUntil; - const allowFileUrl = options.allowFileUrl; delete options.allowFileUrl; - const urlRegex = allowFileUrl ? /^(http|file)/i : /^http/i; - if (urlOrHtml.match(urlRegex)) { + const allowFileUri = options.allowFileUri; delete options.allowFileUri; + const uriRegex = allowFileUri ? /^(https?|file):\/\//i : /^https?:\/\//i; + if (uriOrHtml.match(uriRegex)) { // Request is for a URL, so request it requestOptions.waitUntil = waitUntil || 'networkidle2'; - await page.goto(urlOrHtml, requestOptions); + await page.goto(uriOrHtml, requestOptions); } else { // Request is some HTML content. Use request interception to assign the body requestOptions.waitUntil = waitUntil || 'networkidle0'; @@ -190,7 +190,7 @@ const _processPage = (async (convertAction, urlOrHtml, options) => { request.continue(); else { htmlIntercepted = true - request.respond({ body: urlOrHtml === '' ? ' ' : urlOrHtml }); + request.respond({ body: uriOrHtml === '' ? ' ' : uriOrHtml }); } }); const displayUrl = options.displayUrl; delete options.displayUrl; diff --git a/lib/grover/middleware.rb b/lib/grover/middleware.rb index cbd7e99..65784bd 100644 --- a/lib/grover/middleware.rb +++ b/lib/grover/middleware.rb @@ -25,6 +25,8 @@ def call(env) end def _call(env) + check_file_uri_configuration + @request = Rack::Request.new(env) identify_request_type @@ -45,6 +47,14 @@ def _call(env) attr_reader :pdf_request, :png_request, :jpeg_request + def check_file_uri_configuration + return unless Grover.configuration.allow_file_uris + + # The combination of middleware and allowing file URLs is exceptionally + # unsafe as it can lead to data exfiltration from the host system. + raise UnsafeConfigurationError, 'using `allow_file_uris` configuration with middleware is exceptionally unsafe' + end + def identify_request_type @pdf_request = Grover.configuration.use_pdf_middleware && path_matches?(PDF_REGEX) @png_request = Grover.configuration.use_png_middleware && path_matches?(PNG_REGEX) @@ -111,7 +121,7 @@ def create_grover_for_response(response) # rubocop:disable Metrics/AbcSize body = body.join if body.is_a?(Array) body = HTMLPreprocessor.process body, root_url, protocol - options = { display_url: request_url, middleware: true } + options = { display_url: request_url } cookies = Rack::Utils.parse_cookies(env).map do |name, value| { name: name, value: Rack::Utils.escape(value), domain: env['HTTP_HOST'] } end diff --git a/lib/grover/options_builder.rb b/lib/grover/options_builder.rb index afdd0af..729a70b 100644 --- a/lib/grover/options_builder.rb +++ b/lib/grover/options_builder.rb @@ -8,20 +8,14 @@ class Grover # Build options from Grover.configuration, meta_options, and passed-in options # class OptionsBuilder < Hash - def initialize(options, url, middleware:) + def initialize(options, uri) super() - @url = url + @uri = uri combined = grover_configuration Utils.deep_merge! combined, Utils.deep_stringify_keys(options) - Utils.deep_merge! combined, meta_options unless url_source? + Utils.deep_merge! combined, meta_options unless uri_source? update OptionsFixer.new(combined).run - - # The combination of middleware and allowing file URLs is exceptionally - # unsafe as it can lead to data exfiltration from the host system. - return unless middleware && (self['allow_file_url'] || self['allowFileUrl']) - - raise UnsafeConfigurationError, 'using `allow_file_url` option with middleware is exceptionally unsafe' end private @@ -47,11 +41,11 @@ def meta_options end def meta_tags - Nokogiri::HTML(@url).xpath('//meta') + Nokogiri::HTML(@uri).xpath('//meta') end - def url_source? - @url.match(/\A(http|file)/i) + def uri_source? + @uri.match?(%r{\A(https?|file)://}i) end end end diff --git a/spec/grover/middleware_spec.rb b/spec/grover/middleware_spec.rb index 76935ea..1ddf1b2 100644 --- a/spec/grover/middleware_spec.rb +++ b/spec/grover/middleware_spec.rb @@ -537,13 +537,13 @@ it 'passes through the request URL (sans extension) to Grover' do allow(Grover).to( receive(:new). - with('Grover McGroveryface', display_url: 'http://www.example.org/test', middleware: true). + with('Grover McGroveryface', display_url: 'http://www.example.org/test'). and_return(grover) ) allow(grover).to receive(:to_pdf).with(no_args).and_return 'A converted PDF' expect(Grover).to( receive(:new). - with('Grover McGroveryface', display_url: 'http://www.example.org/test', middleware: true) + with('Grover McGroveryface', display_url: 'http://www.example.org/test') ) expect(grover).to receive(:to_pdf).with(no_args) get 'http://www.example.org/test.pdf' @@ -555,14 +555,12 @@ allow(Grover).to receive(:new).with( 'Grover McGroveryface', display_url: 'http://www.example.org/test', - middleware: true, cookies: [{ domain: 'www.example.org', name: k, value: v }] ).and_return(grover) allow(grover).to receive(:to_pdf).with(no_args).and_return 'A converted PDF' expect(Grover).to receive(:new).with( 'Grover McGroveryface', display_url: 'http://www.example.org/test', - middleware: true, cookies: [{ domain: 'www.example.org', name: k, value: v }] ) expect(grover).to receive(:to_pdf).with(no_args) @@ -597,13 +595,13 @@ it 'passes through the request URL (sans extension) to Grover' do allow(Grover).to( receive(:new). - with('Grover McGroveryface', display_url: 'http://www.example.org/test', middleware: true). + with('Grover McGroveryface', display_url: 'http://www.example.org/test'). and_return(grover) ) allow(grover).to receive(:to_png).with(no_args).and_return 'A converted PNG' expect(Grover).to( receive(:new). - with('Grover McGroveryface', display_url: 'http://www.example.org/test', middleware: true) + with('Grover McGroveryface', display_url: 'http://www.example.org/test') ) expect(grover).to receive(:to_png).with(no_args) get 'http://www.example.org/test.png' @@ -627,13 +625,13 @@ it 'passes through the request URL (sans extension) to Grover' do allow(Grover).to( receive(:new). - with('Grover McGroveryface', display_url: 'http://www.example.org/test', middleware: true). + with('Grover McGroveryface', display_url: 'http://www.example.org/test'). and_return(grover) ) allow(grover).to receive(:to_jpeg).with(no_args).and_return 'A converted JPEG' expect(Grover).to( receive(:new). - with('Grover McGroveryface', display_url: 'http://www.example.org/test', middleware: true) + with('Grover McGroveryface', display_url: 'http://www.example.org/test') ) expect(grover).to receive(:to_jpeg).with(no_args) get 'http://www.example.org/test.jpeg' diff --git a/spec/grover/options_builder_spec.rb b/spec/grover/options_builder_spec.rb index 40fcaba..821fb46 100644 --- a/spec/grover/options_builder_spec.rb +++ b/spec/grover/options_builder_spec.rb @@ -4,12 +4,11 @@ require 'grover/options_builder' describe Grover::OptionsBuilder do - subject(:built_options) { described_class.new(options, url_or_html, middleware: middleware) } + subject(:built_options) { described_class.new(options, url_or_html) } let(:url_or_html) { 'https://google.com' } let(:options) { {} } let(:global_config) { { cache: false, quality: 95 } } - let(:middleware) { false } before { allow(Grover.configuration).to receive(:options).and_return(global_config) } @@ -158,30 +157,4 @@ it { is_expected.to include('viewport' => { 'height' => 100, 'width' => 200, 'device_scale_factor' => 2.5 }) } end - - context 'when the middleware flag is enabled' do - let(:middleware) { true } - - it 'returns combined passed-in/global options' do - expect(built_options).to eq('cache' => false, 'quality' => 95) - end - - context 'when providing `allow_file_url` option inline' do - let(:options) { { allow_file_url: true } } - - it 'raises `Grover::UnsafeConfigurationError`' do - expect { built_options }.to raise_error Grover::UnsafeConfigurationError, - 'using `allow_file_url` option with middleware is exceptionally unsafe' - end - end - - context 'when providing `allow_file_url` option through global options' do - let(:global_config) { { allow_file_url: true } } - - it 'raises `Grover::UnsafeConfigurationError`' do - expect { built_options }.to raise_error Grover::UnsafeConfigurationError, - 'using `allow_file_url` option with middleware is exceptionally unsafe' - end - end - end end diff --git a/spec/grover/processor_spec.rb b/spec/grover/processor_spec.rb index 5e5c76c..ac02d55 100644 --- a/spec/grover/processor_spec.rb +++ b/spec/grover/processor_spec.rb @@ -59,8 +59,8 @@ end context 'when passing through a valid file URL' do - let(:url_or_html) { "file:///#{fixture_path('test.html')}" } - let(:options) { { allowFileUrl: true } } + let(:url_or_html) { "file://#{fixture_path('test.html')}" } + let(:options) { { 'allowFileUri' => true } } it { is_expected.to start_with "%PDF-1.4\n" } it { expect(pdf_reader.page_count).to eq 1 } @@ -69,7 +69,7 @@ context 'when passing through an invalid file URL' do let(:url_or_html) { 'file:///fake/invalid.html' } - let(:options) { { allowFileUrl: true } } + let(:options) { { 'allowFileUri' => true } } it 'raises a JavaScript error that the file could not be found' do expect do @@ -78,13 +78,13 @@ end end - context 'when passing a file URL and allow_file_url is disabled' do - let(:url_or_html) { "file:///#{fixture_path('test.html')}" } + context 'when passing a file URL and allow_file_uri is disabled' do + let(:url_or_html) { "file://#{fixture_path('test.html')}" } it { is_expected.to start_with "%PDF-1.4\n" } it { expect(pdf_reader.page_count).to eq 1 } it { expect(pdf_text_content).not_to include 'Hello World!' } - it { expect(pdf_text_content).to include "file:///#{fixture_path('test.html')}" } + it { expect(pdf_text_content).to include "file://#{fixture_path('test.html')}" } end context 'when passing through an empty string' do diff --git a/spec/grover_spec.rb b/spec/grover_spec.rb index 0566771..68e1a1c 100644 --- a/spec/grover_spec.rb +++ b/spec/grover_spec.rb @@ -10,7 +10,7 @@ describe '.new' do subject(:new) { described_class.new('http://google.com') } - it { expect(new.instance_variable_get('@url')).to eq 'http://google.com' } + it { expect(new.instance_variable_get('@uri')).to eq 'http://google.com' } it { expect(new.instance_variable_get('@root_path')).to be_nil } it { expect(new.instance_variable_get('@options')).to eq({}) } @@ -19,14 +19,14 @@ let(:options) { { page_size: 'A4' } } - it { expect(new.instance_variable_get('@url')).to eq 'http://happyfuntimes.com' } + it { expect(new.instance_variable_get('@uri')).to eq 'http://happyfuntimes.com' } it { expect(new.instance_variable_get('@root_path')).to be_nil } it { expect(new.instance_variable_get('@options')).to eq('page_size' => 'A4') } context 'with root path specified' do let(:options) { { page_size: 'A4', root_path: 'foo/bar/baz' } } - it { expect(new.instance_variable_get('@url')).to eq 'http://happyfuntimes.com' } + it { expect(new.instance_variable_get('@uri')).to eq 'http://happyfuntimes.com' } it { expect(new.instance_variable_get('@root_path')).to eq 'foo/bar/baz' } it { expect(new.instance_variable_get('@options')).to eq('page_size' => 'A4') } end @@ -35,7 +35,7 @@ context 'when url provided is `nil`' do subject(:new) { described_class.new(nil) } - it { expect(new.instance_variable_get('@url')).to eq '' } + it { expect(new.instance_variable_get('@uri')).to eq '' } it { expect(new.instance_variable_get('@root_path')).to be_nil } it { expect(new.instance_variable_get('@options')).to eq({}) } end @@ -49,8 +49,12 @@ before { allow(Grover::Processor).to receive(:new).with(Dir.pwd).and_return processor } it 'calls to Grover::Processor' do - allow(processor).to receive(:convert).with(:pdf, url_or_html, {}).and_return 'some PDF content' - expect(processor).to receive(:convert).with(:pdf, url_or_html, {}) + allow(processor).to( + receive(:convert). + with(:pdf, url_or_html, { 'allowFileUri' => false }). + and_return('some PDF content') + ) + expect(processor).to receive(:convert).with(:pdf, url_or_html, { 'allowFileUri' => false }) expect(to_pdf).to eq 'some PDF content' end @@ -62,10 +66,13 @@ it 'calls to Grover::Processor with the path specified' do allow(processor).to( receive(:convert). - with(:pdf, url_or_html, { 'path' => '/foo/bar' }). + with(:pdf, url_or_html, { 'path' => '/foo/bar', 'allowFileUri' => false }). and_return('some PDF content') ) - expect(processor).to receive(:convert).with(:pdf, url_or_html, { 'path' => '/foo/bar' }) + expect(processor).to( + receive(:convert). + with(:pdf, url_or_html, { 'path' => '/foo/bar', 'allowFileUri' => false }) + ) expect(to_pdf).to eq 'some PDF content' end @@ -75,10 +82,10 @@ it 'calls to Grover::Processor without the path specified' do allow(processor).to( receive(:convert). - with(:pdf, url_or_html, {}). + with(:pdf, url_or_html, { 'allowFileUri' => false }). and_return('some PDF content') ) - expect(processor).to receive(:convert).with(:pdf, url_or_html, {}) + expect(processor).to receive(:convert).with(:pdf, url_or_html, { 'allowFileUri' => false }) expect(to_pdf).to eq 'some PDF content' end end @@ -89,9 +96,13 @@ it 'calls to Grover::Processor with overridden path' do allow(Grover::Processor).to receive(:new).with('foo/bar/baz').and_return processor - allow(processor).to receive(:convert).with(:pdf, url_or_html, {}).and_return 'some PDF content' + allow(processor).to( + receive(:convert). + with(:pdf, url_or_html, { 'allowFileUri' => false }). + and_return('some PDF content') + ) expect(Grover::Processor).to receive(:new).with('foo/bar/baz') - expect(processor).to receive(:convert).with(:pdf, url_or_html, {}) + expect(processor).to receive(:convert).with(:pdf, url_or_html, { 'allowFileUri' => false }) expect(to_pdf).to eq 'some PDF content' end end @@ -104,10 +115,13 @@ it 'builds options and passes them through to the processor' do allow(processor).to( receive(:convert). - with(:pdf, url_or_html, { 'headerTemplate' => 'Some header' }). + with(:pdf, url_or_html, { 'headerTemplate' => 'Some header', 'allowFileUri' => false }). and_return('some PDF content') ) - expect(processor).to receive(:convert).with(:pdf, url_or_html, { 'headerTemplate' => 'Some header' }) + expect(processor).to( + receive(:convert). + with(:pdf, url_or_html, { 'headerTemplate' => 'Some header', 'allowFileUri' => false }) + ) expect(to_pdf).to eq 'some PDF content' end @@ -115,8 +129,12 @@ let(:global_options) { { front_cover_path: '/front', back_cover_path: '/back' } } it 'excludes front and back cover paths from options passed to processor' do - allow(processor).to receive(:convert).with(:pdf, url_or_html, {}).and_return 'some PDF content' - expect(processor).to receive(:convert).with(:pdf, url_or_html, {}) + allow(processor).to( + receive(:convert). + with(:pdf, url_or_html, { 'allowFileUri' => false }). + and_return('some PDF content') + ) + expect(processor).to receive(:convert).with(:pdf, url_or_html, { 'allowFileUri' => false }) expect(to_pdf).to eq 'some PDF content' expect(grover.front_cover_path).to eq '/front' expect(grover.back_cover_path).to eq '/back' @@ -126,15 +144,31 @@ context 'when instance options are provided' do let(:options) { { header_template: 'instance header', footer_template: 'instance footer' } } - it 'builds options, overriding global options, and passes them through to the processor' do + it 'builds options, overriding global options, and passes them through to the processor' do # rubocop:disable RSpec/ExampleLength allow(processor).to( receive(:convert). - with(:pdf, url_or_html, { 'headerTemplate' => 'instance header', 'footerTemplate' => 'instance footer' }). + with( + :pdf, + url_or_html, + { + 'headerTemplate' => 'instance header', + 'footerTemplate' => 'instance footer', + 'allowFileUri' => false + } + ). and_return('some PDF content') ) expect(processor).to( receive(:convert). - with(:pdf, url_or_html, { 'headerTemplate' => 'instance header', 'footerTemplate' => 'instance footer' }) + with( + :pdf, + url_or_html, + { + 'headerTemplate' => 'instance header', + 'footerTemplate' => 'instance footer', + 'allowFileUri' => false + } + ) ) expect(to_pdf).to eq 'some PDF content' end @@ -145,8 +179,12 @@ let(:options) { { front_cover_path: '/front', back_cover_path: '/back' } } it 'excludes front and back cover paths from options passed to processor' do - allow(processor).to receive(:convert).with(:pdf, url_or_html, {}).and_return 'some PDF content' - expect(processor).to receive(:convert).with(:pdf, url_or_html, {}) + allow(processor).to( + receive(:convert). + with(:pdf, url_or_html, { 'allowFileUri' => false }). + and_return('some PDF content') + ) + expect(processor).to receive(:convert).with(:pdf, url_or_html, { 'allowFileUri' => false }) expect(to_pdf).to eq 'some PDF content' expect(grover.front_cover_path).to eq '/front' expect(grover.back_cover_path).to eq '/back' @@ -168,15 +206,29 @@ HTML end - it 'builds options and passes them through to the processor' do + it 'builds options and passes them through to the processor' do # rubocop:disable RSpec/ExampleLength allow(processor).to( receive(:convert). - with(:pdf, url_or_html, { 'footerTemplate' => "
Footer with \"quotes\" in it
" }). + with( + :pdf, + url_or_html, + { + 'footerTemplate' => "
Footer with \"quotes\" in it
", + 'allowFileUri' => false + } + ). and_return('some PDF content') ) expect(processor).to( receive(:convert). - with(:pdf, url_or_html, { 'footerTemplate' => "
Footer with \"quotes\" in it
" }) + with( + :pdf, + url_or_html, + { + 'footerTemplate' => "
Footer with \"quotes\" in it
", + 'allowFileUri' => false + } + ) ) expect(to_pdf).to eq 'some PDF content' end @@ -196,10 +248,13 @@ it 'builds options and passes them through to the processor' do allow(processor).to( receive(:convert). - with(:pdf, url_or_html, { 'launchArgs' => ['--disable-speech-api'] }). + with(:pdf, url_or_html, { 'launchArgs' => ['--disable-speech-api'], 'allowFileUri' => false }). and_return('some PDF content') ) - expect(processor).to receive(:convert).with(:pdf, url_or_html, { 'launchArgs' => ['--disable-speech-api'] }) + expect(processor).to( + receive(:convert). + with(:pdf, url_or_html, { 'launchArgs' => ['--disable-speech-api'], 'allowFileUri' => false }) + ) expect(to_pdf).to eq 'some PDF content' end end @@ -218,10 +273,13 @@ it 'builds options and passes them through to the processor' do allow(processor).to( receive(:convert). - with(:pdf, url_or_html, { 'displayHeaderFooter' => false }). + with(:pdf, url_or_html, { 'displayHeaderFooter' => false, 'allowFileUri' => false }). and_return('some PDF content') ) - expect(processor).to receive(:convert).with(:pdf, url_or_html, { 'displayHeaderFooter' => false }) + expect(processor).to( + receive(:convert). + with(:pdf, url_or_html, { 'displayHeaderFooter' => false, 'allowFileUri' => false }) + ) expect(to_pdf).to eq 'some PDF content' end end @@ -243,12 +301,20 @@ it 'builds options and passes them through to the processor' do allow(processor).to( receive(:convert). - with(:pdf, url_or_html, { 'viewport' => { 'height' => 100, 'width' => 200, 'deviceScaleFactor' => 2.5 } }). + with( + :pdf, + url_or_html, + { 'viewport' => { 'height' => 100, 'width' => 200, 'deviceScaleFactor' => 2.5 }, 'allowFileUri' => false } + ). and_return('some PDF content') ) expect(processor).to( receive(:convert). - with(:pdf, url_or_html, { 'viewport' => { 'height' => 100, 'width' => 200, 'deviceScaleFactor' => 2.5 } }) + with( + :pdf, + url_or_html, + { 'viewport' => { 'height' => 100, 'width' => 200, 'deviceScaleFactor' => 2.5 }, 'allowFileUri' => false } + ) ) expect(to_pdf).to eq 'some PDF content' end @@ -260,13 +326,63 @@ it 'does not transform the header keys' do allow(processor).to( receive(:convert). - with(:pdf, url_or_html, { 'extraHTTPHeaders' => { 'Foo' => 'Bar', 'baz' => 'Qux' } }). + with( + :pdf, + url_or_html, + { 'extraHTTPHeaders' => { 'Foo' => 'Bar', 'baz' => 'Qux' }, 'allowFileUri' => false } + ). and_return('some PDF content') ) expect(processor).to( receive(:convert). - with(:pdf, url_or_html, { 'extraHTTPHeaders' => { 'Foo' => 'Bar', 'baz' => 'Qux' } }) + with( + :pdf, + url_or_html, + { 'extraHTTPHeaders' => { 'Foo' => 'Bar', 'baz' => 'Qux' }, 'allowFileUri' => false } + ) + ) + expect(to_pdf).to eq 'some PDF content' + end + end + + context 'when providing `allow_file_uri` option inline' do + let(:options) { { allow_file_uri: true } } + + it 'does not overload `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:pdf, url_or_html, { 'allowFileUri' => false }). + and_return('some PDF content') + ) + expect(processor).to receive(:convert).with(:pdf, url_or_html, { 'allowFileUri' => false }) + expect(to_pdf).to eq 'some PDF content' + end + end + + context 'when providing `allow_file_uri` option through global options' do + let(:global_config) { { allow_file_uri: true } } + + it 'does not overload `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:pdf, url_or_html, { 'allowFileUri' => false }). + and_return('some PDF content') ) + expect(processor).to receive(:convert).with(:pdf, url_or_html, { 'allowFileUri' => false }) + expect(to_pdf).to eq 'some PDF content' + end + end + + context 'when `allow_file_uris` configuration is enabled' do + before { allow(described_class.configuration).to receive(:allow_file_uris).and_return true } + + it 'overloads `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:pdf, url_or_html, { 'allowFileUri' => true }). + and_return('some PDF content') + ) + expect(processor).to receive(:convert).with(:pdf, url_or_html, { 'allowFileUri' => true }) expect(to_pdf).to eq 'some PDF content' end end @@ -280,8 +396,12 @@ before { allow(Grover::Processor).to receive(:new).with(Dir.pwd).and_return processor } it 'calls to Grover::Processor' do - allow(processor).to receive(:convert).with(:screenshot, url_or_html, {}).and_return 'some image content' - expect(processor).to receive(:convert).with(:screenshot, url_or_html, {}) + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'allowFileUri' => false }). + and_return('some image content') + ) + expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'allowFileUri' => false }) expect(screenshot).to eq 'some image content' end @@ -291,10 +411,13 @@ it 'calls to Grover::Processor with the path specified' do allow(processor).to( receive(:convert). - with(:screenshot, url_or_html, { 'path' => '/foo/bar' }). + with(:screenshot, url_or_html, { 'path' => '/foo/bar', 'allowFileUri' => false }). and_return('some image content') ) - expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'path' => '/foo/bar' }) + expect(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'path' => '/foo/bar', 'allowFileUri' => false }) + ) expect(screenshot).to eq 'some image content' end end @@ -308,10 +431,13 @@ it 'calls to Grover::Processor with the type specified' do allow(processor).to( receive(:convert). - with(:screenshot, url_or_html, { 'type' => 'png' }). + with(:screenshot, url_or_html, { 'type' => 'png', 'allowFileUri' => false }). and_return('some image content') ) - expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'type' => 'png' }) + expect(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'png', 'allowFileUri' => false }) + ) expect(screenshot).to eq 'some image content' end end @@ -322,10 +448,13 @@ it 'calls to Grover::Processor with the type specified' do allow(processor).to( receive(:convert). - with(:screenshot, url_or_html, { 'type' => 'jpeg' }). + with(:screenshot, url_or_html, { 'type' => 'jpeg', 'allowFileUri' => false }). and_return('some image content') ) - expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'type' => 'jpeg' }) + expect(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'jpeg', 'allowFileUri' => false }) + ) expect(screenshot).to eq 'some image content' end end @@ -334,8 +463,12 @@ let(:format) { 'bmp' } it 'calls to Grover::Processor without the type specified' do - allow(processor).to receive(:convert).with(:screenshot, url_or_html, {}).and_return 'some image content' - expect(processor).to receive(:convert).with(:screenshot, url_or_html, {}) + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'allowFileUri' => false }). + and_return('some image content') + ) + expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'allowFileUri' => false }) expect(screenshot).to eq 'some image content' end end @@ -346,9 +479,13 @@ it 'calls to Grover::Processor with overridden path' do allow(Grover::Processor).to receive(:new).with('foo/bar/baz').and_return processor - allow(processor).to receive(:convert).with(:screenshot, url_or_html, {}).and_return 'some image content' + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'allowFileUri' => false }). + and_return('some image content') + ) expect(Grover::Processor).to receive(:new).with('foo/bar/baz') - expect(processor).to receive(:convert).with(:screenshot, url_or_html, {}) + expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'allowFileUri' => false }) expect(screenshot).to eq 'some image content' end end @@ -361,10 +498,13 @@ it 'builds options and passes them through to the processor' do allow(processor).to( receive(:convert). - with(:screenshot, url_or_html, { 'headerTemplate' => 'Some header' }). + with(:screenshot, url_or_html, { 'headerTemplate' => 'Some header', 'allowFileUri' => false }). and_return('some image content') ) - expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'headerTemplate' => 'Some header' }) + expect(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'headerTemplate' => 'Some header', 'allowFileUri' => false }) + ) expect(screenshot).to eq 'some image content' end @@ -372,8 +512,12 @@ let(:global_options) { { front_cover_path: '/front', back_cover_path: '/back' } } it 'excludes front and back cover paths from options passed to processor' do - allow(processor).to receive(:convert).with(:screenshot, url_or_html, {}).and_return 'some image content' - expect(processor).to receive(:convert).with(:screenshot, url_or_html, {}) + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'allowFileUri' => false }). + and_return('some image content') + ) + expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'allowFileUri' => false }) expect(screenshot).to eq 'some image content' expect(grover.front_cover_path).to eq '/front' expect(grover.back_cover_path).to eq '/back' @@ -383,13 +527,17 @@ context 'when instance options are provided' do let(:options) { { header_template: 'instance header', footer_template: 'instance footer' } } - it 'builds options, overriding global options, and passes them through to the processor' do + it 'builds options, overriding global options, and passes them through to the processor' do # rubocop:disable RSpec/ExampleLength allow(processor).to( receive(:convert). with( :screenshot, url_or_html, - { 'headerTemplate' => 'instance header', 'footerTemplate' => 'instance footer' } + { + 'headerTemplate' => 'instance header', + 'footerTemplate' => 'instance footer', + 'allowFileUri' => false + } ). and_return('some image content') ) @@ -398,7 +546,11 @@ with( :screenshot, url_or_html, - { 'headerTemplate' => 'instance header', 'footerTemplate' => 'instance footer' } + { + 'headerTemplate' => 'instance header', + 'footerTemplate' => 'instance footer', + 'allowFileUri' => false + } ) ) expect(screenshot).to eq 'some image content' @@ -410,8 +562,12 @@ let(:options) { { front_cover_path: '/front', back_cover_path: '/back' } } it 'excludes front and back cover paths from options passed to processor' do - allow(processor).to receive(:convert).with(:screenshot, url_or_html, {}).and_return 'some image content' - expect(processor).to receive(:convert).with(:screenshot, url_or_html, {}) + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'allowFileUri' => false }). + and_return('some image content') + ) + expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'allowFileUri' => false }) expect(screenshot).to eq 'some image content' expect(grover.front_cover_path).to eq '/front' expect(grover.back_cover_path).to eq '/back' @@ -438,7 +594,7 @@ with( :screenshot, url_or_html, - { 'viewport' => { 'height' => 100, 'width' => 200, 'deviceScaleFactor' => 2.5 } } + { 'viewport' => { 'height' => 100, 'width' => 200, 'deviceScaleFactor' => 2.5 }, 'allowFileUri' => false } ). and_return('some image content') ) @@ -447,12 +603,54 @@ with( :screenshot, url_or_html, - { 'viewport' => { 'height' => 100, 'width' => 200, 'deviceScaleFactor' => 2.5 } } + { 'viewport' => { 'height' => 100, 'width' => 200, 'deviceScaleFactor' => 2.5 }, 'allowFileUri' => false } ) ) expect(screenshot).to eq 'some image content' end end + + context 'when providing `allow_file_uri` option inline' do + let(:options) { { allow_file_uri: true } } + + it 'does not overload `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'allowFileUri' => false }). + and_return('some image content') + ) + expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'allowFileUri' => false }) + expect(screenshot).to eq 'some image content' + end + end + + context 'when providing `allow_file_uri` option through global options' do + let(:global_config) { { allow_file_uri: true } } + + it 'does not overload `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'allowFileUri' => false }). + and_return('some image content') + ) + expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'allowFileUri' => false }) + expect(screenshot).to eq 'some image content' + end + end + + context 'when `allow_file_uris` configuration is enabled' do + before { allow(described_class.configuration).to receive(:allow_file_uris).and_return true } + + it 'overloads `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'allowFileUri' => true }). + and_return('some image content') + ) + expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'allowFileUri' => true }) + expect(screenshot).to eq 'some image content' + end + end end describe '#to_png' do @@ -465,10 +663,13 @@ it 'calls to Grover::Processor' do allow(processor).to( receive(:convert). - with(:screenshot, url_or_html, { 'type' => 'png' }). + with(:screenshot, url_or_html, { 'type' => 'png', 'allowFileUri' => false }). and_return('some PNG content') ) - expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'type' => 'png' }) + expect(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'png', 'allowFileUri' => false }) + ) expect(to_png).to eq 'some PNG content' end @@ -478,10 +679,64 @@ it 'calls to Grover::Processor with the path specified' do allow(processor).to( receive(:convert). - with(:screenshot, url_or_html, { 'path' => '/foo/bar', 'type' => 'png' }). + with(:screenshot, url_or_html, { 'path' => '/foo/bar', 'type' => 'png', 'allowFileUri' => false }). and_return('some PNG content') ) - expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'path' => '/foo/bar', 'type' => 'png' }) + expect(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'path' => '/foo/bar', 'type' => 'png', 'allowFileUri' => false }) + ) + expect(to_png).to eq 'some PNG content' + end + end + + context 'when providing `allow_file_uri` option inline' do + let(:options) { { allow_file_uri: true } } + + it 'does not overload `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'png', 'allowFileUri' => false }). + and_return('some PNG content') + ) + expect(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'png', 'allowFileUri' => false }) + ) + expect(to_png).to eq 'some PNG content' + end + end + + context 'when providing `allow_file_uri` option through global options' do + let(:global_config) { { allow_file_uri: true } } + + it 'does not overload `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'png', 'allowFileUri' => false }). + and_return('some PNG content') + ) + expect(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'png', 'allowFileUri' => false }) + ) + expect(to_png).to eq 'some PNG content' + end + end + + context 'when `allow_file_uris` configuration is enabled' do + before { allow(described_class.configuration).to receive(:allow_file_uris).and_return true } + + it 'overloads `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'png', 'allowFileUri' => true }). + and_return('some PNG content') + ) + expect(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'png', 'allowFileUri' => true }) + ) expect(to_png).to eq 'some PNG content' end end @@ -497,10 +752,13 @@ it 'calls to Grover::Processor' do allow(processor).to( receive(:convert). - with(:screenshot, url_or_html, { 'type' => 'jpeg' }). + with(:screenshot, url_or_html, { 'type' => 'jpeg', 'allowFileUri' => false }). and_return('some JPG content') ) - expect(processor).to receive(:convert).with(:screenshot, url_or_html, { 'type' => 'jpeg' }) + expect(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'jpeg', 'allowFileUri' => false }) + ) expect(to_jpeg).to eq 'some JPG content' end @@ -511,16 +769,70 @@ it 'calls to Grover::Processor with the path specified' do allow(processor).to( receive(:convert). - with(:screenshot, url_or_html, { 'path' => '/foo/bar', 'type' => 'jpeg' }). + with(:screenshot, url_or_html, { 'path' => '/foo/bar', 'type' => 'jpeg', 'allowFileUri' => false }). and_return('some JPG content') ) expect(processor).to( receive(:convert). - with(:screenshot, url_or_html, { 'path' => '/foo/bar', 'type' => 'jpeg' }) + with(:screenshot, url_or_html, { 'path' => '/foo/bar', 'type' => 'jpeg', 'allowFileUri' => false }) ) expect(to_jpeg).to eq 'some JPG content' end end + + context 'when providing `allow_file_uri` option inline' do + let(:options) { { allow_file_uri: true } } + + it 'does not overload `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'jpeg', 'allowFileUri' => false }). + and_return('some JPG content') + ) + expect(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'jpeg', 'allowFileUri' => false }) + ) + + expect(to_jpeg).to eq 'some JPG content' + end + end + + context 'when providing `allow_file_uri` option through global options' do + let(:global_config) { { allow_file_uri: true } } + + it 'does not overload `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'jpeg', 'allowFileUri' => false }). + and_return('some JPG content') + ) + expect(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'jpeg', 'allowFileUri' => false }) + ) + + expect(to_jpeg).to eq 'some JPG content' + end + end + + context 'when `allow_file_uris` configuration is enabled' do + before { allow(described_class.configuration).to receive(:allow_file_uris).and_return true } + + it 'overloads `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'jpeg', 'allowFileUri' => true }). + and_return('some JPG content') + ) + expect(processor).to( + receive(:convert). + with(:screenshot, url_or_html, { 'type' => 'jpeg', 'allowFileUri' => true }) + ) + + expect(to_jpeg).to eq 'some JPG content' + end + end end describe '#to_html' do @@ -532,10 +844,56 @@ before { allow(Grover::Processor).to receive(:new).with(Dir.pwd).and_return processor } it 'calls to Grover::Processor' do - allow(processor).to receive(:convert).with(:content, url_or_html, {}).and_return expected_html - expect(processor).to receive(:convert).with(:content, url_or_html, {}) + allow(processor).to( + receive(:convert). + with(:content, url_or_html, { 'allowFileUri' => false }). + and_return(expected_html) + ) + expect(processor).to receive(:convert).with(:content, url_or_html, { 'allowFileUri' => false }) expect(to_html).to eq expected_html end + + context 'when providing `allow_file_uri` option inline' do + let(:options) { { allow_file_uri: true } } + + it 'does not overload `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:content, url_or_html, { 'allowFileUri' => false }). + and_return(expected_html) + ) + expect(processor).to receive(:convert).with(:content, url_or_html, { 'allowFileUri' => false }) + expect(to_html).to eq expected_html + end + end + + context 'when providing `allow_file_uri` option through global options' do + let(:global_config) { { allow_file_uri: true } } + + it 'does not overload `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:content, url_or_html, { 'allowFileUri' => false }). + and_return(expected_html) + ) + expect(processor).to receive(:convert).with(:content, url_or_html, { 'allowFileUri' => false }) + expect(to_html).to eq expected_html + end + end + + context 'when `allow_file_uris` configuration is enabled' do + before { allow(described_class.configuration).to receive(:allow_file_uris).and_return true } + + it 'overloads `allowFileUri` option' do + allow(processor).to( + receive(:convert). + with(:content, url_or_html, { 'allowFileUri' => true }). + and_return(expected_html) + ) + expect(processor).to receive(:convert).with(:content, url_or_html, { 'allowFileUri' => true }) + expect(to_html).to eq expected_html + end + end end describe '#front_cover_path' do @@ -649,7 +1007,7 @@ describe '#inspect' do subject(:inspect) { grover.inspect } - it { is_expected.to eq "#" } + it { is_expected.to eq "#" } end describe '.configuration' do From c594995ae25782e42fc903926eaff3ed7a4654ae Mon Sep 17 00:00:00 2001 From: Andrew Bromwich Date: Tue, 2 Jul 2024 01:02:26 +1000 Subject: [PATCH 5/5] Only raise Grover::UnsafeConfigurationError for grover requests Add specs to test the middleware error behaviour --- lib/grover/middleware.rb | 7 +++-- spec/grover/middleware_spec.rb | 56 ++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/lib/grover/middleware.rb b/lib/grover/middleware.rb index 65784bd..819426b 100644 --- a/lib/grover/middleware.rb +++ b/lib/grover/middleware.rb @@ -25,12 +25,13 @@ def call(env) end def _call(env) - check_file_uri_configuration - @request = Rack::Request.new(env) identify_request_type - configure_env_for_grover_request(env) if grover_request? + if grover_request? + check_file_uri_configuration + configure_env_for_grover_request(env) + end status, headers, response = @app.call(env) response = update_response response, headers if grover_request? && html_content?(headers) diff --git a/spec/grover/middleware_spec.rb b/spec/grover/middleware_spec.rb index 1ddf1b2..37e3043 100644 --- a/spec/grover/middleware_spec.rb +++ b/spec/grover/middleware_spec.rb @@ -52,6 +52,21 @@ expect(last_response.body.bytesize).to eq response_size expect(last_response.headers['Content-Length']).to eq response_size.to_s end + + context 'when `allow_file_uris` configuration option is set' do + before { allow(Grover.configuration).to receive(:allow_file_uris).and_return true } + + it 'raises an `UnsafeConfigurationError`' do + expect do + get 'http://www.example.org/test.PDF' + end.to( + raise_error( + Grover::UnsafeConfigurationError, + 'using `allow_file_uris` configuration with middleware is exceptionally unsafe' + ) + ) + end + end end context 'when requesting a PNG' do @@ -72,6 +87,21 @@ expect(last_response.body.bytesize).to eq response_size expect(last_response.headers['Content-Length']).to eq response_size.to_s end + + context 'when `allow_file_uris` configuration option is set' do + before { allow(Grover.configuration).to receive(:allow_file_uris).and_return true } + + it 'raises an `UnsafeConfigurationError`' do + expect do + get 'http://www.example.org/test.PNG' + end.to( + raise_error( + Grover::UnsafeConfigurationError, + 'using `allow_file_uris` configuration with middleware is exceptionally unsafe' + ) + ) + end + end end context 'when requesting a JPEG' do @@ -100,6 +130,21 @@ expect(last_response.body.bytesize).to eq response_size expect(last_response.headers['Content-Length']).to eq response_size.to_s end + + context 'when `allow_file_uris` configuration option is set' do + before { allow(Grover.configuration).to receive(:allow_file_uris).and_return true } + + it 'raises an `UnsafeConfigurationError`' do + expect do + get 'http://www.example.org/test.JPG' + end.to( + raise_error( + Grover::UnsafeConfigurationError, + 'using `allow_file_uris` configuration with middleware is exceptionally unsafe' + ) + ) + end + end end context 'when request doesnt have an extension' do @@ -109,6 +154,17 @@ expect(last_response.body).to eq 'Grover McGroveryface' expect(last_response.headers['Content-Length']).to eq '20' end + + context 'when `allow_file_uris` configuration option is set' do + before { allow(Grover.configuration).to receive(:allow_file_uris).and_return true } + + it 'returns the downstream content and content type' do + get 'http://www.example.org/test' + expect(last_response.headers['Content-Type']).to eq 'text/html' + expect(last_response.body).to eq 'Grover McGroveryface' + expect(last_response.headers['Content-Length']).to eq '20' + end + end end context 'when request has a non-PDF/PNG/JPEG extension' do