diff --git a/app/views/action_text/attachables/_opengraph_embed.html.erb b/app/views/action_text/attachables/_opengraph_embed.html.erb index f82208a..c5cdd05 100644 --- a/app/views/action_text/attachables/_opengraph_embed.html.erb +++ b/app/views/action_text/attachables/_opengraph_embed.html.erb @@ -3,9 +3,9 @@
- <%= link_to truncate(opengraph_embed.filename, length: 280, omission: "…"), opengraph_embed.href, rel: "noreferrer", target: "_blank" %> + <%= link_to_if opengraph_embed.href.present?, truncate(opengraph_embed.filename, length: 280, omission: "…"), opengraph_embed.href, rel: "noreferrer", target: "_blank" %>
-
<%= truncate(opengraph_embed.caption, length: 560, omission: "…").html_safe %>
+
<%= truncate(opengraph_embed.caption, length: 560, omission: "…") %>
<% if opengraph_embed.url %>
diff --git a/lib/rails_ext/actiontext_opengraph_embeds.rb b/lib/rails_ext/actiontext_opengraph_embeds.rb index 916ab10..102e1d8 100644 --- a/lib/rails_ext/actiontext_opengraph_embeds.rb +++ b/lib/rails_ext/actiontext_opengraph_embeds.rb @@ -16,12 +16,22 @@ class ActionText::Attachment::OpengraphEmbed private def attributes_from_node(node) { - href: node["href"], - url: node["url"], + href: web_url(node["href"]), + url: web_url(node["url"]), filename: node["filename"], description: node["caption"] } end + + # A link preview points at what we unfurled, which is always an absolute + # http or https URL. Drop anything else the message body asks for, so a + # body written by hand can't aim the preview's link or its image at another + # scheme or at a path on this Campfire. + def web_url(value) + value if value.present? && URI.parse(value).is_a?(URI::HTTP) + rescue URI::InvalidURIError + nil + end end attr_accessor :href, :url, :filename, :description diff --git a/test/controllers/rooms_controller_test.rb b/test/controllers/rooms_controller_test.rb index 69b2add..2bcae89 100644 --- a/test/controllers/rooms_controller_test.rb +++ b/test/controllers/rooms_controller_test.rb @@ -20,6 +20,35 @@ class RoomsControllerTest < ActionDispatch::IntegrationTest assert response.cookies[:last_room] = users(:david).rooms.last.id end + test "show renders a link preview written by hand without its off-scheme image and link" do + room = rooms(:watercooler) + post room_messages_url(room, format: :turbo_stream), params: { message: { + body: link_preview_body(href: "javascript:alert(1)", url: "data:image/svg+xml;base64,PHN2Zy8+"), + client_message_id: "hand-written-preview" } } + assert_response :success + + get room_url(room) + + assert_response :success + assert_no_match /javascript:alert/, response.body + assert_no_match /data:image\/svg/, response.body + assert_match "Free cookies", response.body + end + + test "show renders an unfurled link preview" do + room = rooms(:watercooler) + post room_messages_url(room, format: :turbo_stream), params: { message: { + body: link_preview_body(href: "https://example.com/page", url: "https://example.com/image.png"), + client_message_id: "unfurled-preview" } } + assert_response :success + + get room_url(room) + + assert_response :success + assert_match %r{ { Room.count }, -1 do @@ -42,4 +71,10 @@ class RoomsControllerTest < ActionDispatch::IntegrationTest delete room_url(rooms(:designers)) end end + + private + def link_preview_body(href:, url:) + %(
) + end end diff --git a/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb b/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb new file mode 100644 index 0000000..15c1481 --- /dev/null +++ b/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb @@ -0,0 +1,66 @@ +require "test_helper" + +class ActionText::Attachment::OpengraphEmbedTest < ActiveSupport::TestCase + test "keeps absolute http and https links and images" do + embed = embed_from href: "http://example.com/page", url: "https://example.com/image.png" + + assert_equal "http://example.com/page", embed.href + assert_equal "https://example.com/image.png", embed.url + end + + test "drops a link and an image that aren't web URLs" do + [ "javascript:alert(1)", "data:text/html,pwned", "vbscript:msgbox(1)", "//example.com/image.png", + "/rooms/1", "rooms/1", "", "http://exa mple.com/ " ].each do |value| + embed = embed_from href: value, url: value + + assert_nil embed.href, "expected #{value.inspect} to be dropped as a link" + assert_nil embed.url, "expected #{value.inspect} to be dropped as an image" + end + end + + test "renders the image and the link when both are web URLs" do + html = render_embed href: "https://example.com/page", url: "https://example.com/image.png" + + assert_match %r{Title}, html + assert_match %r{Title", caption: "" + + assert_no_match //, html + assert_no_match /) + node = ActionText::Fragment.wrap(html).find_all(ActionText::Attachment.tag_name).first + + ActionText::Attachment.from_node(node) + end + + def embed_from(**attributes) + attachment_for(**attributes).attachable + end + + def render_embed(**attributes) + attachment = attachment_for(**attributes) + + ApplicationController.render partial: attachment.to_partial_path, locals: { opengraph_embed: attachment } + end +end