From 3e134b07fa4e382f1f4cfdbe90e74f8e73336a4e Mon Sep 17 00:00:00 2001
From: Oneric <oneric@oneric.stub>
Date: Fri, 15 Mar 2024 18:57:09 -0100
Subject: [PATCH] fetcher: return final URL after redirects from get_object
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Since we reject cross-domain redirects, this doesn’t yet
make a difference, but it’s requried for stricter checking
subsequent commits will introduce.

To make sure (and in case we ever decide to reallow
cross-domain redirects) also use the final location
for containment and reachability checks.
---
 lib/pleroma/object/fetcher.ex        | 27 +++++++++++++++++++--------
 test/pleroma/object/fetcher_test.exs | 27 ++++++++++++++++++++++++---
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex
index f256bbf77..263f9a4fa 100644
--- a/lib/pleroma/object/fetcher.ex
+++ b/lib/pleroma/object/fetcher.ex
@@ -259,12 +259,12 @@ def fetch_and_contain_remote_object_from_id(id) when is_binary(id) do
 
     with {:scheme, true} <- {:scheme, String.starts_with?(id, "http")},
          {_, :ok} <- {:local_fetch, Containment.contain_local_fetch(id)},
-         {:ok, body} <- get_object(id),
+         {:ok, final_id, body} <- get_object(id),
          {:ok, data} <- safe_json_decode(body),
-         {_, :ok} <- {:containment, Containment.contain_origin_from_id(id, data)},
-         {_, :ok} <- {:containment, Containment.contain_origin(id, data)} do
-      unless Instances.reachable?(id) do
-        Instances.set_reachable(id)
+         {_, :ok} <- {:containment, Containment.contain_origin_from_id(final_id, data)},
+         {_, :ok} <- {:containment, Containment.contain_origin(final_id, data)} do
+      unless Instances.reachable?(final_id) do
+        Instances.set_reachable(final_id)
       end
 
       {:ok, data}
@@ -305,6 +305,15 @@ defp check_crossdomain_redirect(final_host, original_url) do
     {:cross_domain_redirect, final_host != URI.parse(original_url).host}
   end
 
+  if @mix_env == :test do
+    defp get_final_id(nil, initial_url), do: initial_url
+    defp get_final_id("", initial_url), do: initial_url
+  end
+
+  defp get_final_id(final_url, _intial_url) do
+    final_url
+  end
+
   @doc "Do NOT use; only public for use in tests"
   def get_object(id) do
     date = Pleroma.Signature.signed_date()
@@ -325,16 +334,18 @@ def get_object(id) do
            {:has_content_type, List.keyfind(headers, "content-type", 0)},
          {:parse_content_type, {:ok, "application", subtype, type_params}} <-
            {:parse_content_type, Plug.Conn.Utils.media_type(content_type)} do
+      final_id = get_final_id(final_url, id)
+
       case {subtype, type_params} do
         {"activity+json", _} ->
-          {:ok, body}
+          {:ok, final_id, body}
 
         {"ld+json", %{"profile" => "https://www.w3.org/ns/activitystreams"}} ->
-          {:ok, body}
+          {:ok, final_id, body}
 
         # pixelfed sometimes (and only sometimes) responds with http instead of https
         {"ld+json", %{"profile" => "http://www.w3.org/ns/activitystreams"}} ->
-          {:ok, body}
+          {:ok, final_id, body}
 
         _ ->
           {:error, {:content_type, content_type}}
diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs
index d1f3c070e..a761578f9 100644
--- a/test/pleroma/object/fetcher_test.exs
+++ b/test/pleroma/object/fetcher_test.exs
@@ -693,12 +693,13 @@ test "should return ok if the content type is application/activity+json" do
         } ->
           %Tesla.Env{
             status: 200,
+            url: "https://mastodon.social/2",
             headers: [{"content-type", "application/activity+json"}],
             body: "{}"
           }
       end)
 
-      assert {:ok, "{}"} = Fetcher.get_object("https://mastodon.social/2")
+      assert {:ok, _, "{}"} = Fetcher.get_object("https://mastodon.social/2")
     end
 
     test "should return ok if the content type is application/ld+json with a profile" do
@@ -709,6 +710,7 @@ test "should return ok if the content type is application/ld+json with a profile
         } ->
           %Tesla.Env{
             status: 200,
+            url: "https://mastodon.social/2",
             headers: [
               {"content-type",
                "application/ld+json; profile=\"https://www.w3.org/ns/activitystreams\""}
@@ -717,7 +719,7 @@ test "should return ok if the content type is application/ld+json with a profile
           }
       end)
 
-      assert {:ok, "{}"} = Fetcher.get_object("https://mastodon.social/2")
+      assert {:ok, _, "{}"} = Fetcher.get_object("https://mastodon.social/2")
 
       Tesla.Mock.mock(fn
         %{
@@ -734,7 +736,7 @@ test "should return ok if the content type is application/ld+json with a profile
           }
       end)
 
-      assert {:ok, "{}"} = Fetcher.get_object("https://mastodon.social/2")
+      assert {:ok, _, "{}"} = Fetcher.get_object("https://mastodon.social/2")
     end
 
     test "should not return ok with other content types" do
@@ -745,6 +747,7 @@ test "should not return ok with other content types" do
         } ->
           %Tesla.Env{
             status: 200,
+            url: "https://mastodon.social/2",
             headers: [{"content-type", "application/json"}],
             body: "{}"
           }
@@ -753,5 +756,23 @@ test "should not return ok with other content types" do
       assert {:error, {:content_type, "application/json"}} =
                Fetcher.get_object("https://mastodon.social/2")
     end
+
+    test "returns the url after redirects" do
+      Tesla.Mock.mock(fn
+        %{
+          method: :get,
+          url: "https://mastodon.social/5"
+        } ->
+          %Tesla.Env{
+            status: 200,
+            url: "https://mastodon.social/7",
+            headers: [{"content-type", "application/activity+json"}],
+            body: "{}"
+          }
+      end)
+
+      assert {:ok, "https://mastodon.social/7", "{}"} =
+               Fetcher.get_object("https://mastodon.social/5")
+    end
   end
 end