Merge pull request 'Improve RSS compliance' (#7932) from fix-rss into develop

Reviewed-on: https://git.pleroma.social/pleroma/pleroma/pulls/7932

changelog.d/rss.fix

@@ -0,0 +1 @@
+Fix RSS feeds to be W3C compliant and render better in RSS readers

lib/pleroma/web/feed/feed_view.ex

@@ -38,6 +38,20 @@ defmodule Pleroma.Web.Feed.FeedView do
     end
   end
 
+  def most_recent_update([], :rss), do: nil
+
+  def most_recent_update(activities, :rss) do
+    activities
+    |> Enum.map(fn activity ->
+      case activity.object do
+        %Object{updated_at: updated_at} -> updated_at
+        _ -> activity.updated_at
+      end
+    end)
+    |> Enum.max_by(&NaiveDateTime.to_erl/1)
+    |> to_rfc2822()
+  end
+
   def most_recent_update(activities, user, :atom) do
     (List.first(activities) || user).updated_at
     |> to_rfc3339()
@@ -97,6 +111,67 @@ defmodule Pleroma.Web.Feed.FeedView do
     end
   end
 
+  def rss_content_encoded(data) do
+    data
+    |> feed_content_encoded()
+    |> String.replace("]]>", "]]>")
+  end
+
+  def atom_content_encoded(data), do: feed_content_encoded(data)
+
+  defp feed_content_encoded(data) do
+    base_content =
+      case activity_content(data) do
+        "" ->
+          (data["summary"] || data["type"] || "")
+          |> escape()
+
+        content ->
+          content
+      end
+
+    attachments_html =
+      if sensitive?(data) do
+        ""
+      else
+        (data["attachment"] || [])
+        |> Enum.map(&rss_attachment_preview/1)
+        |> Enum.reject(&(&1 == ""))
+        |> Enum.join("<br/><br/>")
+      end
+
+    [base_content, attachments_html]
+    |> Enum.reject(&(&1 == ""))
+    |> Enum.join("<br/><br/>")
+  end
+
+  defp rss_attachment_preview(attachment) do
+    href = attachment_href(attachment)
+
+    if is_binary(href) and href != "" do
+      media_type = attachment_type(attachment) || "application/octet-stream"
+      escaped_href = escape(href)
+      name = attachment["name"] || "Attachment"
+      escaped_name = escape(name)
+
+      cond do
+        String.starts_with?(media_type, "image/") ->
+          ~s(<p><img src="#{escaped_href}" alt="#{escaped_name}" loading="lazy"/></p>)
+
+        String.starts_with?(media_type, "video/") ->
+          ~s(<p><video controls preload="metadata" src="#{escaped_href}"></video></p><p><a href="#{escaped_href}">#{escaped_name}</a></p>)
+
+        String.starts_with?(media_type, "audio/") ->
+          ~s(<p><audio controls preload="metadata" src="#{escaped_href}"></audio></p><p><a href="#{escaped_href}">#{escaped_name}</a></p>)
+
+        true ->
+          ~s(<p><a href="#{escaped_href}">#{escaped_name}</a></p>)
+      end
+    else
+      ""
+    end
+  end
+
   def activity_content(%{"content" => content}) do
     content
     |> String.replace(~r/[\n\r]/, "")
@@ -106,16 +181,104 @@ defmodule Pleroma.Web.Feed.FeedView do
 
   def activity_context(activity), do: escape(activity.data["context"])
 
+  def sensitive?(data), do: data["sensitive"] in [true, "true"]
+
+  def feed_self_url(conn) do
+    conn
+    |> Phoenix.Controller.current_url()
+    |> escape()
+  end
+
+  def activity_link(activity, data) do
+    cond do
+      activity.local -> data["id"] || data["url"]
+      true -> data["external_url"] || data["id"] || data["url"]
+    end
+  end
+
   def attachment_href(attachment) do
-    attachment["url"]
+    attachment
-    |> hd()
+    |> attachment_url_data()
     |> Map.get("href")
   end
 
   def attachment_type(attachment) do
-    attachment["url"]
+    attachment
-    |> hd()
+    |> attachment_url_data()
-    |> Map.get("mediaType")
+    |> Map.get("mediaType") || attachment["mediaType"]
+  end
+
+  def attachment_size(attachment) do
+    attachment
+    |> attachment_url_data()
+    |> Map.get("size", 0)
+  end
+
+  def attachment_size_positive(attachment) do
+    case attachment_size(attachment) do
+      size when is_integer(size) and size > 0 ->
+        size
+
+      size when is_binary(size) ->
+        case Integer.parse(size) do
+          {parsed, ""} when parsed > 0 -> parsed
+          _ -> nil
+        end
+
+      _ ->
+        nil
+    end
+  end
+
+  def attachment_medium(attachment) do
+    case attachment_type(attachment) do
+      "image/" <> _rest -> "image"
+      "video/" <> _rest -> "video"
+      "audio/" <> _rest -> "audio"
+      _ -> "document"
+    end
+  end
+
+  def rss_enclosure_attachment(attachments) do
+    Enum.find(attachments || [], &(not is_nil(attachment_size_positive(&1))))
+  end
+
+  def attachment_description(attachment) do
+    attachment["name"] || ""
+  end
+
+  def media_content_xml(_attachment, sensitive) when sensitive in [true, "true"], do: ""
+
+  def media_content_xml(attachment, _sensitive) do
+    href = escape(attachment_href(attachment) || "")
+    type = escape(attachment_type(attachment) || "application/octet-stream")
+    medium = escape(attachment_medium(attachment))
+
+    file_size_attr =
+      case attachment_size_positive(attachment) do
+        size when is_integer(size) -> ~s( fileSize="#{size}")
+        _ -> ""
+      end
+
+    case attachment_description(attachment) do
+      "" ->
+        ~s(<media:content url="#{href}" type="#{type}"#{file_size_attr} medium="#{medium}"/>\n)
+
+      description ->
+        """
+        <media:content url="#{href}" type="#{type}"#{file_size_attr} medium="#{medium}">
+          <media:description type="plain">#{escape(description)}</media:description>
+        </media:content>
+        """
+    end
+  end
+
+  defp attachment_url_data(attachment) do
+    case attachment["url"] do
+      [first | _] when is_map(first) -> first
+      %{} = map -> map
+      _ -> %{}
+    end
   end
 
   def get_href(id) do

lib/pleroma/web/templates/feed/feed/_activity.atom.eex

@@ -1,9 +1,9 @@
 <entry>
   <activity:object-type>http://activitystrea.ms/schema/1.0/note</activity:object-type>
   <activity:verb>http://activitystrea.ms/schema/1.0/post</activity:verb>
-  <id><%= @data["id"] %></id>
+  <id><%= escape(@data["id"] || "") %></id>
   <title><%= activity_title(@data, Keyword.get(@feed_config, :post_title, %{})) %></title>
-  <content type="html"><%= activity_description(@data) %></content>
+  <content type="html"><%= escape(atom_content_encoded(@data)) %></content>
   <published><%= to_rfc3339(@data["published"]) %></published>
   <updated><%= to_rfc3339(@data["published"]) %></updated>
   <ostatus:conversation ref="<%= activity_context(@activity) %>">
@@ -15,35 +15,26 @@
     <summary><%= escape(@data["summary"]) %></summary>
   <% end %>
 
-  <%= if @activity.local do %>
+  <link type="text/html" href='<%= escape(activity_link(@activity, @data) || "") %>' rel="alternate"/>
-    <link type="application/atom+xml" href='<%= @data["id"] %>' rel="self"/>
-    <link type="text/html" href='<%= @data["id"] %>' rel="alternate"/>
-  <% else %>
-    <link type="text/html" href='<%= @data["external_url"] %>' rel="alternate"/>
-  <% end %>
 
   <%= for tag <- Pleroma.Object.hashtags(@object) do %>
     <category term="<%= tag %>"></category>
   <% end %>
 
-  <%= for attachment <- @data["attachment"] || [] do %>
+  <%= unless sensitive?(@data) do %>
-    <link rel="enclosure" href="<%= attachment_href(attachment) %>" type="<%= attachment_type(attachment) %>"/>
+    <%= for attachment <- @data["attachment"] || [] do %>
+      <%= if size = attachment_size_positive(attachment) do %>
+        <link rel="enclosure" href="<%= escape(attachment_href(attachment) || "") %>" type="<%= escape(attachment_type(attachment) || "application/octet-stream") %>" length="<%= size %>"/>
+      <% else %>
+        <link rel="enclosure" href="<%= escape(attachment_href(attachment) || "") %>" type="<%= escape(attachment_type(attachment) || "application/octet-stream") %>"/>
+      <% end %>
+    <% end %>
   <% end %>
 
   <%= if @data["inReplyTo"] do %>
     <thr:in-reply-to ref='<%= @data["inReplyTo"] %>' href='<%= get_href(@data["inReplyTo"]) %>'/>
   <% end %>
 
-  <%= for id <- @activity.recipients do %>
-    <%= if id == Pleroma.Constants.as_public() do %>
-      <link rel="mentioned" ostatus:object-type="http://activitystrea.ms/schema/1.0/collection" href="http://activityschema.org/collection/public"/>
-    <% else %>
-      <%= unless Regex.match?(~r/^#{Pleroma.Web.Endpoint.url()}.+followers$/, id) do %>
-        <link rel="mentioned" ostatus:object-type="http://activitystrea.ms/schema/1.0/person" href="<%= id %>"/>
-      <% end %>
-    <% end %>
-  <% end %>
-
   <%= for {emoji, file} <- @data["emoji"] || %{} do %>
     <link name="<%= emoji %>" rel="emoji" href="<%= file %>"/>
   <% end %>

lib/pleroma/web/templates/feed/feed/_activity.rss.eex

@@ -1,45 +1,34 @@
 <item>
   <activity:object-type>http://activitystrea.ms/schema/1.0/note</activity:object-type>
   <activity:verb>http://activitystrea.ms/schema/1.0/post</activity:verb>
-  <guid><%= @data["id"] %></guid>
+  <guid isPermaLink="true"><%= escape(@data["id"] || "") %></guid>
   <title><%= activity_title(@data, Keyword.get(@feed_config, :post_title, %{})) %></title>
   <description><%= activity_description(@data) %></description>
+  <content:encoded><![CDATA[<%= rss_content_encoded(@data) %>]]></content:encoded>
   <pubDate><%= to_rfc2822(@data["published"]) %></pubDate>
   <ostatus:conversation ref="<%= activity_context(@activity) %>">
     <%= activity_context(@activity) %>
   </ostatus:conversation>
 
-  <%= if @activity.local do %>
+  <link><%= escape(activity_link(@activity, @data) || "") %></link>
-    <link><%= @data["id"] %></link>
-  <% else %>
-    <link><%= @data["external_url"] %></link>
-  <% end %>
-
-  <link rel="ostatus:conversation"><%= activity_context(@activity) %></link>
 
   <%= for tag <- Pleroma.Object.hashtags(@object) do %>
-    <category term="<%= tag %>"></category>
+    <%= if tag && tag != "" do %>
+      <category><%= tag %></category>
+    <% end %>
+  <% end %>
+
+  <%= unless sensitive?(@data) do %>
+    <%= with attachment when not is_nil(attachment) <- rss_enclosure_attachment(@data["attachment"]) do %>
+      <enclosure url="<%= escape(attachment_href(attachment) || "") %>" type="<%= escape(attachment_type(attachment) || "application/octet-stream") %>" length="<%= attachment_size_positive(attachment) %>" />
+    <% end %>
   <% end %>
 
   <%= for attachment <- @data["attachment"] || [] do %>
-    <enclosure url="<%= attachment_href(attachment) %>" type="<%= attachment_type(attachment) %>" />
+    <%= media_content_xml(attachment, sensitive?(@data)) %>
   <% end %>
 
   <%= if @data["inReplyTo"] do %>
     <thr:in-reply-to ref='<%= @data["inReplyTo"] %>' href='<%= get_href(@data["inReplyTo"]) %>'/>
   <% end %>
-
-  <%= for id <- @activity.recipients do %>
-    <%= if id == Pleroma.Constants.as_public() do %>
-      <link rel="mentioned" ostatus:object-type="http://activitystrea.ms/schema/1.0/collection">http://activityschema.org/collection/public</link>
-    <% else %>
-      <%= unless Regex.match?(~r/^#{Pleroma.Web.Endpoint.url()}.+followers$/, id) do %>
-        <link rel="mentioned" ostatus:object-type="http://activitystrea.ms/schema/1.0/person"><%= id %></link>
-      <% end %>
-    <% end %>
-  <% end %>
-
-  <%= for {emoji, file} <- @data["emoji"] || %{} do %>
-    <link name="<%= emoji %>" rel="emoji"><%= file %></link>
-  <% end %>
 </item>

lib/pleroma/web/templates/feed/feed/_tag_activity.atom.eex

@@ -4,9 +4,9 @@
 
     <%= render Phoenix.Controller.view_module(@conn), "_tag_author.atom", assigns %>
 
-  <id><%= @data["id"] %></id>
+  <id><%= escape(@data["id"] || "") %></id>
   <title><%= activity_title(@data, Keyword.get(@feed_config, :post_title, %{})) %></title>
-  <content type="html"><%= activity_description(@data) %></content>
+  <content type="html"><%= escape(atom_content_encoded(@data)) %></content>
   <published><%= to_rfc3339(@data["published"]) %></published>
   <updated><%= to_rfc3339(@data["published"]) %></updated>
   <ostatus:conversation ref="<%= activity_context(@activity) %>">
@@ -15,34 +15,25 @@
   <link href="<%= activity_context(@activity) %>" rel="ostatus:conversation"/>
 
   <%= if @data["summary"] != "" do %>
-    <summary><%= @data["summary"] %></summary>
+    <summary><%= escape(@data["summary"]) %></summary>
   <% end %>
 
-  <%= if @activity.local do %>
+  <link type="text/html" href='<%= escape(activity_link(@activity, @data) || "") %>' rel="alternate"/>
-    <link type="application/atom+xml" href='<%= @data["id"] %>' rel="self"/>
-    <link type="text/html" href='<%= @data["id"] %>' rel="alternate"/>
-  <% else %>
-    <link type="text/html" href='<%= @data["external_url"] %>' rel="alternate"/>
-  <% end %>
-
-  <%= for id <- @activity.recipients do %>
-    <%= if id == Pleroma.Constants.as_public() do %>
-      <link rel="mentioned"
-            ostatus:object-type="http://activitystrea.ms/schema/1.0/collection"
-            href="http://activityschema.org/collection/public"/>
-    <% else %>
-      <%= unless Regex.match?(~r/^#{Pleroma.Web.Endpoint.url()}.+followers$/, id) do %>
-        <link rel="mentioned"
-              ostatus:object-type="http://activitystrea.ms/schema/1.0/person"
-              href="<%= id %>" />
-      <% end %>
-    <% end %>
-  <% end %>
 
   <%= for tag <- Pleroma.Object.hashtags(@object) do %>
     <category term="<%= tag %>"></category>
   <% end %>
 
+  <%= unless sensitive?(@data) do %>
+    <%= for attachment <- @data["attachment"] || [] do %>
+      <%= if size = attachment_size_positive(attachment) do %>
+        <link rel="enclosure" href="<%= escape(attachment_href(attachment) || "") %>" type="<%= escape(attachment_type(attachment) || "application/octet-stream") %>" length="<%= size %>"/>
+      <% else %>
+        <link rel="enclosure" href="<%= escape(attachment_href(attachment) || "") %>" type="<%= escape(attachment_type(attachment) || "application/octet-stream") %>"/>
+      <% end %>
+    <% end %>
+  <% end %>
+
   <%= for {emoji, file} <- @data["emoji"] || %{} do %>
     <link name="<%= emoji %>" rel="emoji" href="<%= file %>"/>
   <% end %>

lib/pleroma/web/templates/feed/feed/_tag_activity.xml.eex

@@ -1,14 +1,24 @@
 <item>
-  <title><%= activity_title(@data, Keyword.get(@feed_config, :post_title, %{})) %></title>
+  <guid isPermaLink="true"><%= escape(activity_link(@activity, @data) || "") %></guid>
-
+  <link><%= escape(activity_link(@activity, @data) || "") %></link>
-
-  <guid isPermalink="true"><%= activity_context(@activity) %></guid>
-  <link><%= activity_context(@activity) %></link>
   <pubDate><%= to_rfc2822(@data["published"]) %></pubDate>
-
+  <title><%= activity_title(@data, Keyword.get(@feed_config, :post_title, %{})) %></title>
   <description><%= activity_description(@data) %></description>
-  <%= for attachment <- @data["attachment"] || [] do %>
+  <content:encoded><![CDATA[<%= rss_content_encoded(@data) %>]]></content:encoded>
-    <enclosure url="<%= attachment_href(attachment) %>" type="<%= attachment_type(attachment) %>"/>
+
+  <%= unless sensitive?(@data) do %>
+    <%= with attachment when not is_nil(attachment) <- rss_enclosure_attachment(@data["attachment"]) do %>
+      <enclosure url="<%= escape(attachment_href(attachment) || "") %>" type="<%= escape(attachment_type(attachment) || "application/octet-stream") %>" length="<%= attachment_size_positive(attachment) %>"/>
+    <% end %>
   <% end %>
 
+  <%= for attachment <- @data["attachment"] || [] do %>
+    <%= media_content_xml(attachment, sensitive?(@data)) %>
+  <% end %>
+
+  <%= for tag <- Pleroma.Object.hashtags(@object) do %>
+    <%= if tag && tag != "" do %>
+      <category><%= tag %></category>
+    <% end %>
+  <% end %>
 </item>

lib/pleroma/web/templates/feed/feed/tag.atom.eex

@@ -9,10 +9,12 @@
 
   <id><%= Routes.tag_feed_url(@conn, :feed, @tag) <> ".atom" %></id>
   <title>#<%= @tag %></title>
-  <subtitle><%= Gettext.dpgettext("static_pages", "tag feed description", "These are public toots tagged with #%{tag}. You can interact with them if you have an account anywhere in the fediverse.", tag: @tag) %></subtitle>
+  <%= with subtitle when subtitle not in [nil, ""] <- Gettext.dpgettext("static_pages", "tag feed description", "These are public toots tagged with #%{tag}. You can interact with them if you have an account anywhere in the fediverse.", tag: @tag) do %>
+    <subtitle><%= subtitle %></subtitle>
+  <% end %>
   <logo><%= feed_logo() %></logo>
-  <updated><%= most_recent_update(@activities) %></updated>
+  <updated><%= most_recent_update(@activities) || to_rfc3339(NaiveDateTime.utc_now()) %></updated>
-  <link rel="self" href="<%= "#{Routes.tag_feed_url(@conn, :feed, @tag)}.atom"  %>" type="application/atom+xml"/>
+  <link rel="self" href="<%= feed_self_url(@conn) %>" type="application/atom+xml"/>
 
   <%= for activity <- @activities do %>
   <%= render Phoenix.Controller.view_module(@conn), "_tag_activity.atom", Map.merge(assigns, prepare_activity(activity, actor: true)) %>

lib/pleroma/web/templates/feed/feed/tag.rss.eex

@@ -1,6 +1,8 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <rss version="2.0"
      xmlns:webfeeds="http://webfeeds.org/rss/1.0"
+     xmlns:content="http://purl.org/rss/1.0/modules/content/"
+     xmlns:media="http://search.yahoo.com/mrss/"
      xmlns:thr="http://purl.org/syndication/thread/1.0">
   <channel>
 
@@ -8,7 +10,12 @@
     <description><%= Gettext.dpgettext("static_pages", "tag feed description", "These are public toots tagged with #%{tag}. You can interact with them if you have an account anywhere in the fediverse.", tag: @tag) %></description>
     <link><%= "#{Routes.tag_feed_url(@conn, :feed, @tag)}.rss" %></link>
     <webfeeds:logo><%= feed_logo() %></webfeeds:logo>
+    <webfeeds:icon><%= feed_logo() %></webfeeds:icon>
     <webfeeds:accentColor>2b90d9</webfeeds:accentColor>
+    <%= with last_build_date when not is_nil(last_build_date) <- most_recent_update(@activities, :rss) do %>
+      <lastBuildDate><%= last_build_date %></lastBuildDate>
+    <% end %>
+    <generator><%= Pleroma.Application.named_version() %></generator>
     <%= for activity <- @activities do %>
     <%= render Phoenix.Controller.view_module(@conn), "_tag_activity.xml", Map.merge(assigns, prepare_activity(activity)) %>
     <% end %>

lib/pleroma/web/templates/feed/feed/user.atom.eex

@@ -8,10 +8,12 @@
 
   <id><%= Routes.user_feed_url(@conn, :feed, @user.nickname) <> ".atom" %></id>
   <title><%= @user.nickname <> "'s timeline" %></title>
-  <subtitle><%= escape(@user.bio) %></subtitle>
+  <%= if @user.bio && @user.bio != "" do %>
+    <subtitle><%= escape(@user.bio) %></subtitle>
+  <% end %>
   <updated><%= most_recent_update(@activities, @user, :atom) %></updated>
   <logo><%= logo(@user) %></logo>
-  <link rel="self" href="<%= "#{Routes.user_feed_url(@conn, :feed, @user.nickname)}.atom" %>" type="application/atom+xml"/>
+  <link rel="self" href="<%= feed_self_url(@conn) %>" type="application/atom+xml"/>
 
   <%= render Phoenix.Controller.view_module(@conn), "_author.atom", assigns %>
 

lib/pleroma/web/templates/feed/feed/user.rss.eex

@@ -1,26 +1,32 @@
 <?xml version="1.0" encoding="UTF-8" ?>
 <rss version="2.0"
      xmlns:atom="http://www.w3.org/2005/Atom"
+     xmlns:content="http://purl.org/rss/1.0/modules/content/"
+     xmlns:webfeeds="http://webfeeds.org/rss/1.0"
+     xmlns:media="http://search.yahoo.com/mrss/"
      xmlns:thr="http://purl.org/syndication/thread/1.0"
      xmlns:activity="http://activitystrea.ms/spec/1.0/"
      xmlns:ostatus="http://ostatus.org/schema/1.0"
      xmlns:poco="http://portablecontacts.net/spec/1.0">
   <channel>
     <title><%= @user.nickname <> "'s timeline" %></title>
-    <link><%= "#{Routes.user_feed_url(@conn, :feed, @user.nickname)}.rss" %></link>
+    <atom:link href="<%= feed_self_url(@conn) %>"
-    <atom:link href="<%= Routes.user_feed_url(@conn, :feed, @user.nickname) <> ".atom" %>"
                rel="self" type="application/rss+xml" />
+    <link><%= "#{Routes.user_feed_url(@conn, :feed, @user.nickname)}.rss" %></link>
     <description><%= escape(@user.bio) %></description>
     <image>
       <url><%= logo(@user) %></url>
       <title><%= @user.nickname <> "'s timeline" %></title>
       <link><%= "#{Routes.user_feed_url(@conn, :feed, @user.nickname)}.rss" %></link>
     </image>
+    <lastBuildDate><%= most_recent_update(@activities, @user, :rss) %></lastBuildDate>
+    <webfeeds:icon><%= logo(@user) %></webfeeds:icon>
+    <generator><%= Pleroma.Application.named_version() %></generator>
 
     <%= render Phoenix.Controller.view_module(@conn), "_author.rss", assigns %>
 
     <%= if last_activity(@activities) do %>
-      <link rel="next"><%= "#{Routes.user_feed_url(@conn, :feed, @user.nickname)}.rss?max_id=#{last_activity(@activities).id}" %></link>
+      <atom:link rel="next" href="<%= "#{Routes.user_feed_url(@conn, :feed, @user.nickname)}.rss?max_id=#{last_activity(@activities).id}" %>" />
     <% end %>
 
     <%= for activity <- @activities do %>

test/pleroma/web/feed/feed_view_test.exs

@@ -0,0 +1,59 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.Feed.FeedViewTest do
+  use Pleroma.DataCase, async: true
+
+  alias Pleroma.Web.Feed.FeedView
+
+  test "escapes fallback content" do
+    data = %{
+      "attachment" => [],
+      "content" => "",
+      "summary" => "<script>alert('feed')</script>"
+    }
+
+    expected = "&lt;script&gt;alert(&#39;feed&#39;)&lt;/script&gt;"
+
+    assert FeedView.atom_content_encoded(data) == expected
+    assert FeedView.rss_content_encoded(data) == expected
+  end
+
+  test "does not preview or classify sensitive attachments" do
+    attachment = %{
+      "name" => "spoiler",
+      "url" => [%{"href" => "https://example.com/spoiler.png", "mediaType" => "image/png"}]
+    }
+
+    data = %{
+      "attachment" => [attachment],
+      "content" => "content warning",
+      "sensitive" => true
+    }
+
+    assert FeedView.atom_content_encoded(data) == "content warning"
+    assert FeedView.rss_content_encoded(data) == "content warning"
+    assert FeedView.media_content_xml(attachment, true) == ""
+    assert FeedView.media_content_xml(attachment, "true") == ""
+
+    media_content = FeedView.media_content_xml(attachment, false)
+    assert media_content =~ "<media:description type=\"plain\">spoiler</media:description>"
+    refute media_content =~ "media:rating"
+
+    assert FeedView.media_content_xml(Map.delete(attachment, "name"), false) ==
+             ~s(<media:content url="https://example.com/spoiler.png" type="image/png" medium="image"/>\n)
+  end
+
+  test "does not treat a legacy false string as sensitive" do
+    data = %{
+      "attachment" => [
+        %{"url" => [%{"href" => "https://example.com/image.png", "mediaType" => "image/png"}]}
+      ],
+      "content" => "visible",
+      "sensitive" => "false"
+    }
+
+    assert FeedView.atom_content_encoded(data) =~ "<img"
+  end
+end

test/pleroma/web/feed/tag_controller_test.exs

@@ -30,9 +30,18 @@ defmodule Pleroma.Web.Feed.TagControllerTest do
         %{
           "url" => [
             %{
-              "href" =>
+              "href" => "https://peertube.moe/video.mp4?first=one&second=two",
-                "https://peertube.moe/static/webseed/df5f464b-be8d-46fb-ad81-2d4c2d1630e3-480.mp4",
               "mediaType" => "video/mp4",
+              "size" => "123",
+              "type" => "Link"
+            }
+          ]
+        },
+        %{
+          "url" => [
+            %{
+              "href" => "https://peertube.moe/notes.pdf",
+              "mediaType" => "application/pdf",
               "type" => "Link"
             }
           ]
@@ -63,6 +72,15 @@ defmodule Pleroma.Web.Feed.TagControllerTest do
            ]
 
     assert xpath(xml, ~x"//feed/entry/author/name/text()"ls) == [user.nickname, user.nickname]
+    assert xpath(xml, ~x"//feed/entry/link[@rel='self']/@href"sl) == []
+    assert length(xpath(xml, ~x"//feed/entry/link[@rel='alternate']/@href"sl)) == 2
+
+    assert xpath(xml, ~x"//feed/entry/link[@rel='enclosure']/@href"sl) == [
+             "https://peertube.moe/video.mp4?first=one&second=two",
+             "https://peertube.moe/notes.pdf"
+           ]
+
+    assert xpath(xml, ~x"//feed/entry/link[@rel='enclosure']/@length"sl) == ["123"]
 
     conn =
       conn
@@ -96,17 +114,28 @@ defmodule Pleroma.Web.Feed.TagControllerTest do
         %{
           "url" => [
             %{
-              "href" =>
+              "href" => "https://peertube.moe/video.mp4?first=one&second=two",
-                "https://peertube.moe/static/webseed/df5f464b-be8d-46fb-ad81-2d4c2d1630e3-480.mp4",
               "mediaType" => "video/mp4",
+              "size" => "123",
+              "type" => "Link"
+            }
+          ]
+        },
+        %{
+          "url" => [
+            %{
+              "href" => "https://peertube.moe/notes.pdf",
+              "mediaType" => "application/pdf",
               "type" => "Link"
             }
           ]
         }
       ])
 
+    edited_at = NaiveDateTime.add(object.updated_at, 60)
+
     object
-    |> Ecto.Changeset.change(data: object_data)
+    |> Ecto.Changeset.change(data: object_data, updated_at: edited_at)
     |> Pleroma.Repo.update()
 
     {:ok, activity2} = CommonAPI.post(user, %{status: "42 This is :moominmamma #PleromaArt"})
@@ -136,15 +165,27 @@ defmodule Pleroma.Web.Feed.TagControllerTest do
              ~c"yeah #PleromaArt"
            ]
 
+    assert xpath(xml, ~x"//channel/item/guid/@isPermaLink"sl) == ["true", "true"]
+
     assert xpath(xml, ~x"//channel/item/pubDate/text()"sl) == [
              FeedView.to_rfc2822(activity2.data["published"]),
              FeedView.to_rfc2822(activity1.data["published"])
            ]
 
-    assert xpath(xml, ~x"//channel/item/enclosure/@url"sl) == [
+    assert xpath(xml, ~x"//channel/lastBuildDate/text()"s) ==
-             "https://peertube.moe/static/webseed/df5f464b-be8d-46fb-ad81-2d4c2d1630e3-480.mp4"
+             FeedView.to_rfc2822(edited_at)
+
+    assert xpath(xml, ~x"//channel/item/media:content/@url"sl) == [
+             "https://peertube.moe/video.mp4?first=one&second=two",
+             "https://peertube.moe/notes.pdf"
            ]
 
+    assert xpath(xml, ~x"//channel/item/enclosure/@url"sl) == [
+             "https://peertube.moe/video.mp4?first=one&second=two"
+           ]
+
+    assert xpath(xml, ~x"//channel/item/enclosure/@length"sl) == ["123"]
+
     obj1 = Object.normalize(activity1, fetch: false)
     obj2 = Object.normalize(activity2, fetch: false)
 
@@ -181,6 +222,90 @@ defmodule Pleroma.Web.Feed.TagControllerTest do
            ]
   end
 
+  test "does not fabricate lastBuildDate for an empty RSS feed", %{conn: conn} do
+    response =
+      conn
+      |> put_req_header("accept", "application/rss+xml")
+      |> get(tag_feed_path(conn, :feed, "empty.rss"))
+      |> response(200)
+
+    refute response =~ "<lastBuildDate>"
+  end
+
+  test "provides a valid updated timestamp for an empty Atom feed", %{conn: conn} do
+    response =
+      conn
+      |> put_req_header("accept", "application/atom+xml")
+      |> get(tag_feed_path(conn, :feed, "empty.atom"))
+      |> response(200)
+
+    updated = response |> parse() |> xpath(~x"//feed/updated/text()"s)
+    assert {:ok, _datetime, 0} = DateTime.from_iso8601(updated)
+  end
+
+  test "escapes Atom IDs and tag feed self links", %{conn: conn} do
+    user = insert(:user, local: false)
+    object_id = "https://remote.example/object?first=one&second=two"
+
+    note =
+      insert(:note,
+        user: user,
+        object_local: false,
+        data: %{
+          "content" => "remote #atomlink",
+          "external_url" => object_id,
+          "id" => object_id,
+          "summary" => "",
+          "tag" => ["atomlink"]
+        }
+      )
+
+    insert(:note_activity, user: user, note: note, local: false, object_local: false)
+
+    response =
+      conn
+      |> put_req_header("accept", "application/atom+xml")
+      |> get("/tags/atomlink.atom?third=three&fourth=four")
+      |> response(200)
+
+    assert response =~ "https://remote.example/object?first=one&amp;second=two"
+    assert response =~ "third=three&amp;fourth=four"
+
+    xml = parse(response)
+    assert xpath(xml, ~x"//feed/entry/id/text()"s) == object_id
+  end
+
+  test "escapes and round-trips RSS item permalinks", %{conn: conn} do
+    user = insert(:user, local: false)
+    permalink = "https://remote.example/post?first=one&second=two"
+
+    note =
+      insert(:note,
+        user: user,
+        object_local: false,
+        data: %{
+          "content" => "remote #rsslink",
+          "external_url" => permalink,
+          "summary" => "",
+          "tag" => ["rsslink"]
+        }
+      )
+
+    insert(:note_activity, user: user, note: note, local: false, object_local: false)
+
+    response =
+      conn
+      |> put_req_header("accept", "application/rss+xml")
+      |> get(tag_feed_path(conn, :feed, "rsslink.rss"))
+      |> response(200)
+
+    assert response =~ "https://remote.example/post?first=one&amp;second=two"
+
+    xml = parse(response)
+    assert xpath(xml, ~x"//channel/item/guid/text()"s) == permalink
+    assert xpath(xml, ~x"//channel/item/link/text()"s) == permalink
+  end
+
   describe "private instance" do
     setup do: clear_config([:instance, :public], false)
 

test/pleroma/web/feed/user_controller_test.exs

@@ -83,12 +83,12 @@ defmodule Pleroma.Web.Feed.UserControllerTest do
         |> get(user_feed_path(conn, :feed, user.nickname))
         |> response(200)
 
-      activity_titles =
+      xml = SweetXml.parse(resp)
-        resp
+      activity_titles = SweetXml.xpath(xml, ~x"//entry/title/text()"l)
-        |> SweetXml.parse()
-        |> SweetXml.xpath(~x"//entry/title/text()"l)
 
       assert activity_titles == [~c"Won't, didn'...", ~c"2hu", ~c"2hu & as"]
+      assert SweetXml.xpath(xml, ~x"//entry/link[@rel='self']/@href"sl) == []
+      assert length(SweetXml.xpath(xml, ~x"//entry/link[@rel='alternate']/@href"sl)) == 3
       assert resp =~ FeedView.escape(object.data["content"])
       assert resp =~ FeedView.escape(object.data["summary"])
       assert resp =~ FeedView.escape(object.data["context"])
@@ -138,6 +138,107 @@ defmodule Pleroma.Web.Feed.UserControllerTest do
       assert activity_titles == [~c"2hu & as"]
     end
 
+    test "escapes query parameters in feed self links", %{conn: conn, user: user} do
+      for format <- ~w[atom rss] do
+        response =
+          conn
+          |> get("/users/#{user.nickname}/feed.#{format}?first=one&second=two")
+          |> response(200)
+
+        assert response =~ "first=one&amp;second=two"
+        refute response =~ "first=one&second=two"
+      end
+    end
+
+    test "keeps fallback summary markup inert in rendered Atom content", %{
+      conn: conn,
+      object: object,
+      user: user
+    } do
+      data =
+        object.data
+        |> Map.put("content", "")
+        |> Map.put("summary", "<script>alert('feed')</script>")
+
+      object
+      |> Ecto.Changeset.change(data: data)
+      |> Pleroma.Repo.update!()
+
+      response =
+        conn
+        |> get("/users/#{user.nickname}/feed.atom")
+        |> response(200)
+
+      assert response =~
+               "&amp;lt;script&amp;gt;alert(&amp;#39;feed&amp;#39;)&amp;lt;/script&amp;gt;"
+
+      content_values = response |> parse() |> xpath(~x"//entry/content/text()"sl)
+
+      assert Enum.any?(content_values, fn content ->
+               String.starts_with?(content, "&lt;script&gt;alert(&#39;feed&#39;)&lt;/script&gt;")
+             end)
+    end
+
+    test "does not expose sensitive attachments as enclosures", %{
+      conn: conn,
+      object: object,
+      user: user
+    } do
+      object
+      |> Ecto.Changeset.change(data: Map.put(object.data, "sensitive", true))
+      |> Pleroma.Repo.update!()
+
+      atom_response =
+        conn
+        |> get("/users/#{user.nickname}/feed.atom")
+        |> response(200)
+
+      rss_response =
+        conn
+        |> get("/users/#{user.nickname}/feed.rss")
+        |> response(200)
+
+      refute atom_response =~ "rel=\"enclosure\""
+      refute rss_response =~ "<enclosure "
+    end
+
+    test "uses enclosure fallbacks for user feeds", %{conn: conn, object: object, user: user} do
+      attachment = %{"url" => [%{"href" => "https://example.com/file.bin"}]}
+
+      object
+      |> Ecto.Changeset.change(data: Map.put(object.data, "attachment", [attachment]))
+      |> Pleroma.Repo.update!()
+
+      atom_xml =
+        conn
+        |> get("/users/#{user.nickname}/feed.atom")
+        |> response(200)
+        |> parse()
+
+      assert xpath(atom_xml, ~x"//entry/link[@rel='enclosure']/@href"sl) == [
+               "https://example.com/file.bin"
+             ]
+
+      assert xpath(atom_xml, ~x"//entry/link[@rel='enclosure']/@type"sl) == [
+               "application/octet-stream"
+             ]
+
+      assert xpath(atom_xml, ~x"//entry/link[@rel='enclosure']/@length"sl) == []
+
+      rss_xml =
+        conn
+        |> get("/users/#{user.nickname}/feed.rss")
+        |> response(200)
+        |> parse()
+
+      assert xpath(rss_xml, ~x"//item/enclosure/@url"sl) == []
+      assert xpath(rss_xml, ~x"//item/media:content/@url"sl) == ["https://example.com/file.bin"]
+
+      assert xpath(rss_xml, ~x"//item/media:content/@type"sl) == [
+               "application/octet-stream"
+             ]
+    end
+
     test "returns 404 for a missing feed", %{conn: conn} do
       conn =
         conn