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.
This commit is contained in:
Tusooa Zhu 2022-07-07 15:11:29 -04:00 committed by FloatingGhost
parent 9b6cc76be0
commit 9e88fbcdb1
3 changed files with 145 additions and 18 deletions

View file

@ -10,7 +10,10 @@ def update_content_fields(orig_object_data, updated_object) 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 @@ def make_update_object_data(original_data, new_data, date) 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
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: updated
} =
if update_is_reasonable == :update_everything do
%{data: updated_data, updated: updated} =
original_data
|> update_content_fields(new_data)
%{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
)
|> Map.put(:updated, updated)
else
%{
updated_object: original_data,
used_history_in_new_object?: false,
updated: false
}
end
updated_data =
if update_is_reasonable != false do
updated_data
|> maybe_update_poll(new_data)
else
updated_data
end
%{
updated_data: updated_data,

View file

@ -147,14 +147,19 @@ test "it uses a given changeset to update", %{user: user, update: update} 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)
@ -175,8 +180,69 @@ test "it updates the note", %{
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"} = new_note.data
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 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", %{
@ -220,6 +286,14 @@ test "it puts the original note at the front of formerRepresentations", %{
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)
@ -243,7 +317,18 @@ test "it does not prepend to formerRepresentations if no actual changes are made
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)
@ -275,7 +360,10 @@ test "allows updating choice count without generating edit history", %{
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)

View file

@ -1368,7 +1368,7 @@ test "updates a post with emoji and federate properly" 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}:"