Handle attachments with spaces and duplicated attachments

dradis · Jun 24, 2024 · f44129c · f44129c
1 parent 8645017
commit f44129c
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 9 deletions.
diff --git a/app/controllers/concerns/attachments_copier.rb b/app/controllers/concerns/attachments_copier.rb
@@ -1,18 +1,19 @@
 module AttachmentsCopier
   def copy_attachments(record)
-    record.content.scan(Attachment::SCREENSHOT_REGEX).each do |screenshot_url|
-      _, _, _, _, project_id, node_id, filename, _ = screenshot_url
+    record.content.scan(Attachment::SCREENSHOT_REGEX).each do |screenshot_path|
+      full_screenshot_path, _, _, _, project_id, node_id, filename, _ = screenshot_path
 
-      attachment = Attachment.find_by(filename: filename, node_id: record.node_id_was)
+      attachment = Attachment.find_by(filename: CGI::unescape(filename), node_id: record.node_id_was)
 
       if attachment
         new_attachment = attachment.copy_to(record.node)
-        new_url = screenshot_url[0].gsub(
-          /nodes\/[0-9]+\/attachments/,
-          "nodes/#{new_attachment.node_id}/attachments"
+        new_filename = ERB::Util.url_encode(new_attachment.filename)
+        new_path = full_screenshot_path.gsub(
+          /nodes\/[0-9]+\/attachments\/.+/,
+          "nodes/#{new_attachment.node_id}/attachments/#{new_filename}"
         )
 
-        record.content = record.content.gsub(screenshot_url[0], new_url)
+        record.content = record.content.gsub(full_screenshot_path, new_path)
       end
     end
   end

diff --git a/spec/features/evidence_moving_spec.rb b/spec/features/evidence_moving_spec.rb
@@ -33,10 +33,13 @@ def create_node(label, parent = nil)
   let(:current_evidence) { @evidence = create(:evidence, content: content, node: @node_5) }
 
   describe 'moving an evidence to a different node' do
-    let(:attachment) { create(:attachment, node: @node_5) }
+    let(:attachment) { create(:attachment, filename: 'name with spaces.png', node: @node_5) }
     let(:content) { "#[Description]#\n!/projects/#{current_project.id}/nodes/#{@node_5.id}/attachments/#{attachment.filename}!\n" }
 
     before do
+      # Ensure this works with duplicated attachment
+      create(:attachment, filename: 'name with spaces.png', node: @node_1)
+
       within('#modal_move_evidence') do
         click_link @node_1.label
         click_submit

diff --git a/spec/features/note_moving_spec.rb b/spec/features/note_moving_spec.rb
@@ -34,10 +34,13 @@ def create_node(label, parent = nil)
   let(:current_note) { @note = create(:note, text: text, node: @node_5) }
 
   describe 'moving a note to a different node' do
-    let(:attachment) { create(:attachment, node: @node_5) }
+    let(:attachment) { create(:attachment, filename: 'name with spaces.png', node: @node_5) }
     let(:text) { "#[Description]#\n!/projects/#{current_project.id}/nodes/#{@node_5.id}/attachments/#{attachment.filename}!\n" }
 
     before do
+      # Ensure this works with duplicated attachment
+      create(:attachment, filename: 'name with spaces.png', node: @node_1)
+
       within('#modal_move_note') do
         click_link @node_1.label
         click_submit