puzzle / prawn-markup

Parse simple HTML markup to include in Prawn PDFs
MIT License
65 stars 16 forks source link

Malformed style attribute breaks the `style_properties` method #40

Closed JanekNaymspace closed 2 years ago

JanekNaymspace commented 2 years ago

Imagine you got a WYSIWYG editor for your customers. This editor sets style attributes and because it is a WYSIWYG editor, the resulting HTML kind of sucks.

In my case this editor sometimes builds HTML like this:

<td style="text-align: right; ">yooooooo</td>

The method

      def style_properties
        style = current_attrs['style']
        if style
          style.split(';').map { |p| p.split(':', 2).map(&:strip) }.to_h
        else
          {}
        end
      end

fails there because "text-align: right; ".split(';').map { |p| p.split(':', 2).map(&:strip) } results in [["text-align", "right"], [""]] and this is not transformable into a hash.

The error message is:

ArgumentError wrong array length at 1 (expected 2, was 1)
prawn-markup (0.3.6) lib/prawn/markup/processor.rb:128:in `to_h'
prawn-markup (0.3.6) lib/prawn/markup/processor.rb:128:in `style_properties' 
[...]
JanekNaymspace commented 2 years ago

I'm not permitted to create a new branch :(

The code to resolve this issue is this:

 lib/prawn/markup/processor.rb              | 2 +-
 spec/prawn/markup/processor/tables_spec.rb | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/prawn/markup/processor.rb b/lib/prawn/markup/processor.rb
index 02d8cec..0604344 100644
--- a/lib/prawn/markup/processor.rb
+++ b/lib/prawn/markup/processor.rb
@@ -125,7 +125,7 @@ module Prawn
       def style_properties
         style = current_attrs['style']
         if style
-          style.split(';').map { |p| p.split(':', 2).map(&:strip) }.to_h
+          style.split(';').map { |p| p.split(':', 2).map(&:strip) }.select { |splitted_style| splitted_style.size == 2 }.to_h
         else
           {}
         end
diff --git a/spec/prawn/markup/processor/tables_spec.rb b/spec/prawn/markup/processor/tables_spec.rb
index d1030ce..46605d8 100644
--- a/spec/prawn/markup/processor/tables_spec.rb
+++ b/spec/prawn/markup/processor/tables_spec.rb
@@ -400,4 +400,10 @@ RSpec.describe Prawn::Markup::Processor::Tables do
     end
   end

+  context 'with invalid style attribute' do
+    it 'parses the text and raises no error' do
+      processor.parse('<table><tr><td style="text-align: right; blorg">bananas are great</td></tr></table>')
+      expect(text.strings).to eq(['bananas are great'])
+    end
+  end
 end
codez commented 2 years ago

Good catch, thank you for the report. I guess style.split(';').reject(&:blank?).map { ... }.to_h would also do it.

Of course you are not permitted to create branches here. You are supposed to fork the repo and then create a pull request from there.

JanekNaymspace commented 2 years ago
"text-align: right; blorg".split(';').reject(&:blank?)
=> ["text-align: right", " blorg"]

The select { ... } way seems more reliable.


Oh, that's the way. Sorry, first try to create a PR on Github.

JanekNaymspace commented 2 years ago

Thank you <3