Browse Source

Fixes #4028 - Freshdesk import error when a inline image source attribute contains no clean url.

Dominik Klein 3 years ago
parent
commit
3b993d8308

+ 7 - 4
lib/sequencer/unit/import/freshdesk/conversation/inline_images.rb

@@ -21,14 +21,17 @@ class Sequencer
             end
 
             def self.inline_data(freshdesk_url)
+              clean_freshdesk_url = freshdesk_url.gsub(%r{^cid:}, '')
+              return if !%r{^(http|https)://.+?$}.match?(clean_freshdesk_url)
+
               @cache ||= {}
-              return @cache[freshdesk_url] if @cache[freshdesk_url]
+              return @cache[clean_freshdesk_url] if @cache[clean_freshdesk_url]
 
-              image_data = download(freshdesk_url)
+              image_data = download(clean_freshdesk_url)
               return if image_data.blank?
 
-              @cache[freshdesk_url] = "data:image/png;base64,#{Base64.strict_encode64(image_data)}"
-              @cache[freshdesk_url]
+              @cache[clean_freshdesk_url] = "data:image/png;base64,#{Base64.strict_encode64(image_data)}"
+              @cache[clean_freshdesk_url]
             end
 
             def self.download(freshdesk_url)

+ 39 - 11
spec/lib/sequencer/sequence/import/freshdesk/conversation_spec.rb

@@ -5,10 +5,10 @@ require 'rails_helper'
 RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::Conversation, sequencer: :sequence do
 
   context 'when importing conversations from Freshdesk' do
-
+    let(:inline_image_url) { 'https://eucattachment.freshdesk.com/inline/attachment?token=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6ODAwMTIyMjY4NTMsImRvbWFpbiI6InphbW1hZC5mcmVzaGRlc2suY29tIiwiYWNjb3VudF9pZCI6MTg5MDU2MH0.705lNehzm--aO36CGFg0SW73j0NG3UWcRcN1_DXgtwc' }
     let(:resource) do
       {
-        'body' => "<div style=\"font-family:-apple-system, BlinkMacSystemFont, Segoe UI, Roboto, Helvetica Neue, Arial, sans-serif; font-size:14px\">\n<div dir=\"ltr\">Let's see if inline images work in a subsequent article:</div>\n<div dir=\"ltr\"><img src=\"https://eucattachment.freshdesk.com/inline/attachment?token=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6ODAwMTIyMjY4NTMsImRvbWFpbiI6InphbW1hZC5mcmVzaGRlc2suY29tIiwiYWNjb3VudF9pZCI6MTg5MDU2MH0.705lNehzm--aO36CGFg0SW73j0NG3UWcRcN1_DXgtwc\" style=\"width: auto\" class=\"fr-fil fr-dib\" data-id=\"80012226853\"></div>\n</div>", 'body_text' => "Let's see if inline images work in a subsequent article:",
+        'body' => "<div style=\"font-family:-apple-system, BlinkMacSystemFont, Segoe UI, Roboto, Helvetica Neue, Arial, sans-serif; font-size:14px\">\n<div dir=\"ltr\">Let's see if inline images work in a subsequent article:</div>\n<div dir=\"ltr\"><img src=\"#{inline_image_url}\" style=\"width: auto\" class=\"fr-fil fr-dib\" data-id=\"80012226853\"></div>\n</div>", 'body_text' => "Let's see if inline images work in a subsequent article:",
         'id' => 80_027_218_656,
         'incoming' => false,
         'private' => true,
@@ -76,18 +76,22 @@ RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::Conversation, sequencer
       end
     end
 
-    it 'adds article with inline image' do
-      expect { process(process_payload) }.to change(Ticket::Article, :count).by(1)
-    end
+    shared_examples 'import article' do
+      it 'adds article with inline image' do
+        expect { process(process_payload) }.to change(Ticket::Article, :count).by(1)
+      end
 
-    it 'correct attributes for added article' do
-      process(process_payload)
-      expect(Ticket::Article.last).to have_attributes(
-        to:   'info@zammad.org',
-        body: "\n<div>\n<div dir=\"ltr\">Let's see if inline images work in a subsequent article:</div>\n<div dir=\"ltr\"><img src=\"data:image/png;base64,MTIz\" style=\"width: auto;\"></div>\n</div>\n",
-      )
+      it 'correct attributes for added article' do
+        process(process_payload)
+        expect(Ticket::Article.last).to have_attributes(
+          to:   'info@zammad.org',
+          body: "\n<div>\n<div dir=\"ltr\">Let's see if inline images work in a subsequent article:</div>\n<div dir=\"ltr\"><img src=\"data:image/png;base64,MTIz\" style=\"width: auto;\"></div>\n</div>\n",
+        )
+      end
     end
 
+    include_examples 'import article'
+
     it 'updates already existing article' do
       expect do
         process(process_payload)
@@ -111,5 +115,29 @@ RSpec.describe ::Sequencer::Sequence::Import::Freshdesk::Conversation, sequencer
         }
       )
     end
+
+    context 'when handling special inline images' do
+      context 'when inline image source contains special urls (e.g. "cid:https://...")' do
+        let(:inline_image_url) { 'cid:https://eucattachment.freshdesk.com/inline/attachment?token=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6ODAwMTIyMjY4NTMsImRvbWFpbiI6InphbW1hZC5mcmVzaGRlc2suY29tIiwiYWNjb3VudF9pZCI6MTg5MDU2MH0.705lNehzm--aO36CGFg0SW73j0NG3UWcRcN1_DXgtwc' }
+
+        include_examples 'import article'
+      end
+
+      context 'when inline image source contains broken urls' do
+        let(:inline_image_url) { 'broken_image_url' }
+
+        it 'skips image download with broken inline image url' do
+          expect { process(process_payload) }.to change(Ticket::Article, :count).by(1)
+        end
+
+        it 'correct attributes for added article' do
+          process(process_payload)
+          expect(Ticket::Article.last).to have_attributes(
+            to:   'info@zammad.org',
+            body: "<div>\n<div dir=\"ltr\">Let's see if inline images work in a subsequent article:</div>\n<div dir=\"ltr\"><img src=\"broken_image_url\" style=\"width: auto;\"></div>\n</div>",
+          )
+        end
+      end
+    end
   end
 end