commit: 069554e9253a47f99225e12cc0ee99700fb89c6e
parent f84ed44cea1e5793dd899c74c38336a1721889e6
Author: Tusooa Zhu <tusooa@kazv.moe>
Date: Thu, 7 Jul 2022 15:11:29 -0400
Guard against outdated Updates
It is possible for an earlier Update to be received by us later.
For this, we now
(1) only allows Updates to poll counts if there is no updated field,
or the updated field is the same as the last updated date or
creation date;
(2) does not allow updating anything if the updated field
is older than the last updated date or creation date;
(3) allows updating updatable fields otherwise (normal updates);
(4) if only the updated field is changed, it does not create
a new history item on its own.
Diffstat:
3 files changed, 145 insertions(+), 18 deletions(-)
diff --git a/lib/pleroma/object/updater.ex b/lib/pleroma/object/updater.ex
@@ -10,7 +10,10 @@ defmodule Pleroma.Object.Updater do
|> Enum.reduce(
%{data: orig_object_data, updated: false},
fn field, %{data: data, updated: updated} ->
- updated = updated or Map.get(updated_object, field) != Map.get(orig_object_data, field)
+ updated =
+ updated or
+ (field != "updated" and
+ Map.get(updated_object, field) != Map.get(orig_object_data, field))
data =
if Map.has_key?(updated_object, field) do
@@ -136,21 +139,57 @@ defmodule Pleroma.Object.Updater do
# This calculates the data of the new Object from an Update.
# new_data's formerRepresentations is considered.
def make_new_object_data_from_update_object(original_data, new_data) do
- %{data: updated_data, updated: updated} =
- original_data
- |> update_content_fields(new_data)
+ update_is_reasonable =
+ with {_, updated} when not is_nil(updated) <- {:cur_updated, new_data["updated"]},
+ {_, {:ok, updated_time, _}} <- {:cur_updated, DateTime.from_iso8601(updated)},
+ {_, last_updated} when not is_nil(last_updated) <-
+ {:last_updated, original_data["updated"] || original_data["published"]},
+ {_, {:ok, last_updated_time, _}} <-
+ {:last_updated, DateTime.from_iso8601(last_updated)},
+ :gt <- DateTime.compare(updated_time, last_updated_time) do
+ :update_everything
+ else
+ # only allow poll updates
+ {:cur_updated, _} -> :no_content_update
+ :eq -> :no_content_update
+ # allow all updates
+ {:last_updated, _} -> :update_everything
+ # allow no updates
+ _ -> false
+ end
- %{updated_object: updated_data, used_history_in_new_object?: used_history_in_new_object?} =
- updated_data
- |> maybe_update_history(original_data,
- updated: updated,
- use_history_in_new_object?: true,
- new_data: new_data
- )
+ %{
+ updated_object: updated_data,
+ used_history_in_new_object?: used_history_in_new_object?,
+ updated: updated
+ } =
+ if update_is_reasonable == :update_everything do
+ %{data: updated_data, updated: updated} =
+ original_data
+ |> update_content_fields(new_data)
+
+ updated_data
+ |> maybe_update_history(original_data,
+ updated: updated,
+ use_history_in_new_object?: true,
+ new_data: new_data
+ )
+ |> Map.put(:updated, updated)
+ else
+ %{
+ updated_object: original_data,
+ used_history_in_new_object?: false,
+ updated: false
+ }
+ end
updated_data =
- updated_data
- |> maybe_update_poll(new_data)
+ if update_is_reasonable != false do
+ updated_data
+ |> maybe_update_poll(new_data)
+ else
+ updated_data
+ end
%{
updated_data: updated_data,
diff --git a/test/pleroma/web/activity_pub/side_effects_test.exs b/test/pleroma/web/activity_pub/side_effects_test.exs
@@ -142,14 +142,19 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
describe "update notes" do
setup do
+ make_time = fn ->
+ Pleroma.Web.ActivityPub.Utils.make_date()
+ end
+
user = insert(:user)
- note = insert(:note, user: user)
+ note = insert(:note, user: user, data: %{"published" => make_time.()})
_note_activity = insert(:note_activity, note: note)
updated_note =
note.data
|> Map.put("summary", "edited summary")
|> Map.put("content", "edited content")
+ |> Map.put("updated", make_time.())
{:ok, update_data, []} = Builder.update(user, updated_note)
{:ok, update, _meta} = ActivityPub.persist(update_data, local: true)
@@ -170,8 +175,69 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
updated_note: updated_note
} do
{:ok, _, _} = SideEffects.handle(update, object_data: updated_note)
+ updated_time = updated_note["updated"]
+
+ new_note = Pleroma.Object.get_by_id(object_id)
+
+ assert %{
+ "summary" => "edited summary",
+ "content" => "edited content",
+ "updated" => ^updated_time
+ } = new_note.data
+ end
+
+ test "it rejects updates with no updated attribute in object", %{
+ object_id: object_id,
+ update: update,
+ updated_note: updated_note
+ } do
+ old_note = Pleroma.Object.get_by_id(object_id)
+ updated_note = Map.drop(updated_note, ["updated"])
+ {:ok, _, _} = SideEffects.handle(update, object_data: updated_note)
+ new_note = Pleroma.Object.get_by_id(object_id)
+ assert old_note.data == new_note.data
+ end
+
+ test "it rejects updates with updated attribute older than what we have in the original object",
+ %{
+ object_id: object_id,
+ update: update,
+ updated_note: updated_note
+ } do
+ old_note = Pleroma.Object.get_by_id(object_id)
+ {:ok, creation_time, _} = DateTime.from_iso8601(old_note.data["published"])
+
+ updated_note =
+ Map.put(updated_note, "updated", DateTime.to_iso8601(DateTime.add(creation_time, -10)))
+
+ {:ok, _, _} = SideEffects.handle(update, object_data: updated_note)
new_note = Pleroma.Object.get_by_id(object_id)
- assert %{"summary" => "edited summary", "content" => "edited content"} = new_note.data
+ assert old_note.data == new_note.data
+ end
+
+ test "it rejects updates with updated attribute older than the last Update", %{
+ object_id: object_id,
+ update: update,
+ updated_note: updated_note
+ } do
+ old_note = Pleroma.Object.get_by_id(object_id)
+ {:ok, creation_time, _} = DateTime.from_iso8601(old_note.data["published"])
+
+ updated_note =
+ Map.put(updated_note, "updated", DateTime.to_iso8601(DateTime.add(creation_time, +10)))
+
+ {:ok, _, _} = SideEffects.handle(update, object_data: updated_note)
+
+ old_note = Pleroma.Object.get_by_id(object_id)
+ {:ok, update_time, _} = DateTime.from_iso8601(old_note.data["updated"])
+
+ updated_note =
+ Map.put(updated_note, "updated", DateTime.to_iso8601(DateTime.add(update_time, -5)))
+
+ {:ok, _, _} = SideEffects.handle(update, object_data: updated_note)
+
+ new_note = Pleroma.Object.get_by_id(object_id)
+ assert old_note.data == new_note.data
end
test "it updates using object_data", %{
@@ -215,6 +281,14 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
note.data
|> Map.put("summary", "edited summary 2")
|> Map.put("content", "edited content 2")
+ |> Map.put(
+ "updated",
+ first_edit["updated"]
+ |> DateTime.from_iso8601()
+ |> elem(1)
+ |> DateTime.add(10)
+ |> DateTime.to_iso8601()
+ )
{:ok, second_update_data, []} = Builder.update(user, second_updated_note)
{:ok, update, _meta} = ActivityPub.persist(second_update_data, local: true)
@@ -238,7 +312,18 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
updated_note: updated_note
} do
{:ok, _, _} = SideEffects.handle(update, object_data: updated_note)
- %{data: _first_edit} = Pleroma.Object.get_by_id(object_id)
+ %{data: first_edit} = Pleroma.Object.get_by_id(object_id)
+
+ updated_note =
+ updated_note
+ |> Map.put(
+ "updated",
+ first_edit["updated"]
+ |> DateTime.from_iso8601()
+ |> elem(1)
+ |> DateTime.add(10)
+ |> DateTime.to_iso8601()
+ )
{:ok, _, _} = SideEffects.handle(update, object_data: updated_note)
%{data: new_note} = Pleroma.Object.get_by_id(object_id)
@@ -270,7 +355,10 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
data["oneOf"]
|> Enum.map(fn choice -> put_in(choice, ["replies", "totalItems"], 5) end)
- updated_question = data |> Map.put("oneOf", new_choices)
+ updated_question =
+ data
+ |> Map.put("oneOf", new_choices)
+ |> Map.put("updated", Pleroma.Web.ActivityPub.Utils.make_date())
{:ok, update_data, []} = Builder.update(user, updated_question)
{:ok, update, _meta} = ActivityPub.persist(update_data, local: true)
diff --git a/test/pleroma/web/common_api_test.exs b/test/pleroma/web/common_api_test.exs
@@ -1596,7 +1596,7 @@ defmodule Pleroma.Web.CommonAPITest do
clear_config([:instance, :federating], true)
with_mock Pleroma.Web.Federator,
- publish: fn p -> nil end do
+ publish: fn _p -> nil end do
{:ok, updated} = CommonAPI.update(user, activity, %{status: "updated 2 :#{emoji2}:"})
assert updated.data["object"]["content"] == "updated 2 :#{emoji2}:"
