diff --git a/lib/pdfkit/configuration.rb b/lib/pdfkit/configuration.rb index 0b3b4d7..6eaa7e0 100644 --- a/lib/pdfkit/configuration.rb +++ b/lib/pdfkit/configuration.rb @@ -45,7 +45,7 @@ def wkhtmltopdf=(path) end def executable - using_xvfb? ? "xvfb-run #{wkhtmltopdf}" : wkhtmltopdf + using_xvfb? ? ['xvfb-run', wkhtmltopdf] : wkhtmltopdf end def using_xvfb? diff --git a/lib/pdfkit/pdfkit.rb b/lib/pdfkit/pdfkit.rb index c733f0d..39f0b29 100644 --- a/lib/pdfkit/pdfkit.rb +++ b/lib/pdfkit/pdfkit.rb @@ -46,16 +46,11 @@ def initialize(url_file_or_html, options = {}) end def command(path = nil) - args = @renderer.options_for_command - shell_escaped_command = [executable, OS::shell_escape_for_os(args)].join ' ' - - # In order to allow for URL parameters (e.g. https://www.google.com/search?q=pdfkit) we do - # not escape the source. The user is responsible for ensuring that no vulnerabilities exist - # in the source. Please see https://github.com/pdfkit/pdfkit/issues/164. - input_for_command = @source.to_input_for_command - output_for_command = path ? Shellwords.shellescape(path) : '-' - - "#{shell_escaped_command} #{input_for_command} #{output_for_command}" + args = [*executable] + args.concat(@renderer.options_for_command) + args << @source.to_input_for_command + args << (path ? path : '-') + args end def options diff --git a/lib/pdfkit/source.rb b/lib/pdfkit/source.rb index 84371e9..5238acc 100644 --- a/lib/pdfkit/source.rb +++ b/lib/pdfkit/source.rb @@ -29,7 +29,7 @@ def to_input_for_command if file? @source.path elsif url? - %{"#{shell_safe_url}"} + escaped_url else SOURCE_FROM_STDIN end @@ -41,7 +41,7 @@ def to_s private - def shell_safe_url + def escaped_url url_needs_escaping? ? URI::DEFAULT_PARSER.escape(@source) : @source end diff --git a/lib/pdfkit/wkhtmltopdf.rb b/lib/pdfkit/wkhtmltopdf.rb index 8a2fb06..537e153 100644 --- a/lib/pdfkit/wkhtmltopdf.rb +++ b/lib/pdfkit/wkhtmltopdf.rb @@ -64,7 +64,7 @@ def normalize_value(value) when Array value.flatten.collect{|x| x.to_s} else - (OS::host_is_windows? && value.to_s.index(' ')) ? "'#{ value.to_s }'" : value.to_s + value.to_s end end diff --git a/spec/pdfkit_spec.rb b/spec/pdfkit_spec.rb index 6a8fcff..2600093 100644 --- a/spec/pdfkit_spec.rb +++ b/spec/pdfkit_spec.rb @@ -175,22 +175,22 @@ it "constructs the correct command" do pdfkit = PDFKit.new('html', :page_size => 'Letter', :toc_l1_font_size => 12, :replace => {'foo' => 'bar'}) command = pdfkit.command - expect(command).to include "wkhtmltopdf" - expect(command).to include "--page-size Letter" - expect(command).to include "--toc-l1-font-size 12" - expect(command).to include "--replace foo bar" + expect(command.first).to match(/wkhtmltopdf/) + expect(command).to contain %w[--page-size Letter] + expect(command).to contain %w[--toc-l1-font-size 12] + expect(command).to contain %w[--replace foo bar] end it "sets up one cookie when hash has only one cookie" do pdfkit = PDFKit.new('html', cookie: {cookie_name: :cookie_value}) command = pdfkit.command - expect(command).to include "--cookie cookie_name cookie_value" + expect(command).to contain %w[--cookie cookie_name cookie_value] end - it "does not break Windows paths" do + it "does not split Windows paths that contain spaces" do pdfkit = PDFKit.new('html') allow(PDFKit.configuration).to receive(:wkhtmltopdf).and_return 'c:/Program Files/wkhtmltopdf/wkhtmltopdf.exe' - expect(pdfkit.command).not_to include('Program\ Files') + expect(pdfkit.command).not_to contain(%w[c:/Program Files/wkhtmltopdf/wkhtmltopdf.exe]) end it "does not shell escape source URLs" do @@ -207,15 +207,15 @@ it "sets up multiple cookies when passed multiple cookies" do pdfkit = PDFKit.new('html', :cookie => {:cookie_name1 => :cookie_val1, :cookie_name2 => :cookie_val2}) command = pdfkit.command - expect(command).to include "--cookie cookie_name1 cookie_val1" - expect(command).to include "--cookie cookie_name2 cookie_val2" + expect(command).to contain %w[--cookie cookie_name1 cookie_val1] + expect(command).to contain %w[--cookie cookie_name2 cookie_val2] end it "sets up multiple cookies when passed an array of tuples" do pdfkit = PDFKit.new('html', :cookie => [[:cookie_name1, :cookie_val1], [:cookie_name2, :cookie_val2]]) command = pdfkit.command - expect(command).to include "--cookie cookie_name1 cookie_val1" - expect(command).to include "--cookie cookie_name2 cookie_val2" + expect(command).to contain %w[--cookie cookie_name1 cookie_val1] + expect(command).to contain %w[--cookie cookie_name2 cookie_val2] end it "will not include default options it is told to omit" do @@ -229,48 +229,57 @@ expect(pdfkit.command).not_to include('--disable-smart-shrinking') end - it "encapsulates string arguments in quotes" do + it "must not split string arguments containing spaces" do pdfkit = PDFKit.new('html', :header_center => "foo [page]") - expect(pdfkit.command).to include "--header-center foo\\ \\[page\\]" + expect(pdfkit.command).to contain ['--header-center', 'foo [page]'] end - it "sanitizes string arguments" do + it "paramatarizes string arguments" do pdfkit = PDFKit.new('html', :header_center => "$(ls)") - expect(pdfkit.command).to include "--header-center \\$\\(ls\\)" + expect(pdfkit.command).to contain %w[--header-center $(ls)] end it "read the source from stdin if it is html" do pdfkit = PDFKit.new('html') - expect(pdfkit.command).to match(/- -$/) + command = pdfkit.command + expect(command[-2]).to eq('-') + expect(command[-1]).to eq('-') end it "specifies the URL to the source if it is a url" do pdfkit = PDFKit.new('http://google.com') - expect(pdfkit.command).to match(/"http:\/\/google.com" -$/) + command = pdfkit.command + expect(command[-2]).to eq("http://google.com") + expect(command[-1]).to eq("-") end it "does not break Windows paths" do pdfkit = PDFKit.new('html') allow(PDFKit.configuration).to receive(:wkhtmltopdf).and_return 'c:/Program Files/wkhtmltopdf/wkhtmltopdf.exe' - expect(pdfkit.command).not_to include('Program\ Files') + expect(pdfkit.command).not_to contain ['Program', 'Files'] end it "specifies the path to the source if it is a file" do file_path = File.join(SPEC_ROOT,'fixtures','example.html') pdfkit = PDFKit.new(File.new(file_path)) - expect(pdfkit.command).to match(/#{file_path} -$/) + command = pdfkit.command + expect(command[-2]).to eq(file_path) + expect(command[-1]).to eq('-') end it "specifies the path to the source if it is a tempfile" do file_path = File.join(SPEC_ROOT,'fixtures','example.html') pdfkit = PDFKit.new(Tempfile.new(file_path)) - expect(pdfkit.command).to match(/#{Dir.tmpdir}\S+ -$/) + command = pdfkit.command + expect(command[-2]).to start_with(Dir.tmpdir) + expect(command[-1]).to eq('-') end it "specifies the path for the ouput if a path is given" do file_path = "/path/to/output.pdf" pdfkit = PDFKit.new("html") - expect(pdfkit.command(file_path)).to match(/#{file_path}$/) + command = pdfkit.command(file_path) + expect(command.last).to eq(file_path) end it "detects special pdfkit meta tags" do @@ -284,8 +293,8 @@ } pdfkit = PDFKit.new(body) command = pdfkit.command - expect(command).to include "--page-size Legal" - expect(command).to include "--orientation Landscape" + expect(command).to contain %w[--page-size Legal] + expect(command).to contain %w[--orientation Landscape] end it "detects cookies meta tag" do @@ -299,7 +308,7 @@ } pdfkit = PDFKit.new(body) command = pdfkit.command - expect(command).to include "--cookie rails_session rails_session_value --cookie cookie_variable cookie_variable_value" + expect(command).to contain %w[--cookie rails_session rails_session_value --cookie cookie_variable cookie_variable_value] end it "detects disable_smart_shrinking meta tag" do @@ -313,7 +322,7 @@ pdfkit = PDFKit.new(body) command = pdfkit.command expect(command).to include "--disable-smart-shrinking" - expect(command).not_to include "--disable-smart-shrinking true" + expect(command).not_to contain %w[--disable-smart-shrinking true] end it "detects names with hyphens instead of underscores" do @@ -342,8 +351,8 @@ } pdfkit = PDFKit.new(body) command = pdfkit.command - expect(command).to include "--page-size Legal" - expect(command).to include "--orientation Landscape" + expect(command).to contain %w[--page-size Legal] + expect(command).to contain %w[--orientation Landscape] end it "skips non-pdfkit meta tags" do @@ -358,8 +367,8 @@ } pdfkit = PDFKit.new(body) command = pdfkit.command - expect(command).not_to include "--page-size Legal" - expect(command).to include "--orientation Landscape" + expect(command).not_to contain %w[--page-size Legal] + expect(command).to contain %w[--orientation Landscape] end it "does not use quiet when told to" do @@ -422,14 +431,9 @@ allow(PDFKit::OS).to receive(:host_is_windows?).and_return(true) end - it "escapes special windows characters" do - pdf = PDFKit.new('html', :title => 'hello(world)') - expect(pdf.command).to include 'hello^(world^)' - end - it "quotes spaces in options" do pdf = PDFKit.new('html', :title => 'hello world') - expect(pdf.command).to include "--title 'hello world'" + expect(pdf.command).to contain ['--title', "hello world"] end end end diff --git a/spec/source_spec.rb b/spec/source_spec.rb index 7e3d4f3..b018819 100644 --- a/spec/source_spec.rb +++ b/spec/source_spec.rb @@ -75,19 +75,14 @@ end describe "#to_input_for_command" do - it "URI escapes source URLs and encloses them in quotes to accomodate ampersands" do - source = PDFKit::Source.new("https://www.google.com/search?q='cat/dev/null'") - expect(source.to_input_for_command).to eq "\"https://www.google.com/search?q='cat%3Cdev/zero%3E/dev/null'\"" - end - - it "URI escapes source URI only escape part of it" do - source = PDFKit::Source.new("https://www.google.com/search?q='%20 sleep 5'") - expect(source.to_input_for_command).to eq "\"https://www.google.com/search?q='%2520%20sleep%205'\"" + it "URI escapes source URI" do + source = PDFKit::Source.new("https://www.google.com/search?q=foo bar") + expect(source.to_input_for_command).to eq "https://www.google.com/search?q=foo%20bar" end it "does not URI escape previously escaped source URLs" do - source = PDFKit::Source.new("https://www.google.com/search?q='cat%3Cdev/zero%3E/dev/null'") - expect(source.to_input_for_command).to eq "\"https://www.google.com/search?q='cat%3Cdev/zero%3E/dev/null'\"" + source = PDFKit::Source.new("https://www.google.com/search?q=foo%20bar") + expect(source.to_input_for_command).to eq "https://www.google.com/search?q=foo%20bar" end it "returns a '-' for HTML strings to indicate that we send that content through STDIN" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1f1530b..cbd488c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -21,3 +21,13 @@ RSpec.configure do |config| include Rack::Test::Methods end + +RSpec::Matchers.define :contain do |expected| + match do |actual| + (0..(actual.length - expected.length)).any? do |base_index| + expected.each_with_index.all? do |expected_element,index| + actual[base_index+index] == expected_element + end + end + end +end