- <%= 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: "…") %>
diff --git a/app/views/messages/_message.html.erb b/app/views/messages/_message.html.erb
index b3b1213..d0c491f 100644
--- a/app/views/messages/_message.html.erb
+++ b/app/views/messages/_message.html.erb
@@ -1,7 +1,7 @@
<%# Be sure to check/update messages/_template.html.erb when changing this file %>
<%# Bump this version when the message presentation filters change what they emit. Editing this line changes the template digest, which busts BOTH this fragment cache and the collection cache that keys on this partial's digest (helper Ruby changes alone don't). %>
-<% cache [ message, "presentation-v2" ] do %>
+<% cache [ message, "presentation-v3" ] do %>
<%= message_tag message do %>
<%= local_datetime_tag message.created_at, style: :date %>
diff --git a/lib/rails_ext/actiontext_opengraph_embeds.rb b/lib/rails_ext/actiontext_opengraph_embeds.rb
index 916ab10..d0748cd 100644
--- a/lib/rails_ext/actiontext_opengraph_embeds.rb
+++ b/lib/rails_ext/actiontext_opengraph_embeds.rb
@@ -16,12 +16,54 @@ 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: an absolute http or https URL
+ # on some other host. Drop anything else a message body asks for, so it
+ # can't aim the preview's link or its image at this Campfire and have every
+ # reader's browser fetch it with their session attached.
+ def web_url(value)
+ return if value.blank?
+
+ parsed = URI.parse(value)
+ value if parsed.is_a?(URI::HTTP) && elsewhere?(parsed.host)
+ rescue URI::InvalidURIError
+ nil
+ end
+
+ # "https:/rooms/1" parses as HTTPS with no host at all, and a browser
+ # resolves both that and our own hostname against the origin Campfire is
+ # served from. A percent-escape hides our hostname from this comparison
+ # while a browser still unescapes it back to us, so an escaped host is out
+ # too, and neither case is anything an unfurl could have produced.
+ def elsewhere?(host)
+ return false unless named_host?(host)
+
+ canonical_host(host) != canonical_host(Current.request_host.to_s)
+ end
+
+ # A preview names a page on the public internet, so its host is a domain
+ # name, written plainly. A bare address is not one, and a browser rewrites
+ # the many spellings of an address ("2130706433", "0x7f.0.0.1") into a
+ # single one before it fetches, which is a race a comparison here loses.
+ def named_host?(host)
+ host.present? && host.exclude?("%") && host.include?(".") && domain_ending?(host.split(".").last)
+ end
+
+ # What keeps a name from reading as an address is its last label, which is
+ # a word: never a number, and never the hexadecimal spelling of one.
+ def domain_ending?(label)
+ label.match?(/[a-z]/i) && !label.match?(/\A0x/i)
+ end
+
+ def canonical_host(host)
+ host.downcase.delete_suffix(".")
+ 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..62edf72 100644
--- a/test/controllers/rooms_controller_test.rb
+++ b/test/controllers/rooms_controller_test.rb
@@ -20,6 +20,51 @@ 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 a link preview written by hand without its image pointed at this Campfire" do
+ room = rooms(:watercooler)
+ own_url = room_url(room, host: "www.example.com")
+ post room_messages_url(room, format: :turbo_stream), params: { message: {
+ body: link_preview_body(href: own_url, url: own_url),
+ client_message_id: "same-host-preview" } }
+ assert_response :success
+
+ get room_url(room)
+
+ assert_response :success
+ assert_no_match %r{
})
{ Room.count }, -1 do
@@ -42,4 +87,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..6949f79
--- /dev/null
+++ b/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb
@@ -0,0 +1,102 @@
+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/ ",
+ "https:/rooms/1", "https:rooms/1", "http:/rooms/1", "https://", "http://:80/rooms/1" ].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 "drops a link and an image on this Campfire's own host, however it is spelled" do
+ Current.set request: ActionDispatch::TestRequest.create("HTTP_HOST" => "once.campfire.test") do
+ [ "https://once.campfire.test/rooms/1", "http://once.campfire.test/rooms/1",
+ "https://ONCE.Campfire.Test/rooms/1", "https://once.campfire.test./rooms/1",
+ "https://%6fnce.campfire.test/rooms/1", "https://%77ww.example.com/x.png" ].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
+
+ embed = embed_from href: "https://example.com/page", url: "https://example.com/image.png"
+ assert_equal "https://example.com/page", embed.href
+ assert_equal "https://example.com/image.png", embed.url
+ end
+ end
+
+ test "drops a link and an image on a bare address rather than a domain name" do
+ [ "http://127.0.0.1/rooms/1", "http://2130706433/rooms/1", "http://0177.0.0.1/rooms/1",
+ "http://0x7f.0.0.1/rooms/1", "http://1.2.3.0xff/rooms/1", "http://[::1]/rooms/1",
+ "http://localhost/rooms/1", "https://203.0.113.10/image.png" ].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 "keeps an internationalized domain written in punycode" do
+ embed = embed_from href: "https://xn--80aswg.xn--p1ai/page", url: "https://xn--80aswg.xn--p1ai/image.png"
+
+ assert_equal "https://xn--80aswg.xn--p1ai/page", embed.href
+ assert_equal "https://xn--80aswg.xn--p1ai/image.png", embed.url
+ 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