From caa261f4bfff8f8fdedaccb463e71fb554106a2f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 16 Apr 2021 15:12:38 -0700 Subject: [PATCH 1/2] Gracefully handle expect, unsupported activities --- bookwyrm/activitypub/base_activity.py | 6 +++- bookwyrm/activitypub/verbs.py | 6 +++- .../tests/views/inbox/test_inbox_create.py | 29 ++++++++++++++++--- 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 3d922b47..dd2795bb 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -52,10 +52,14 @@ def naive_parse(activity_objects, activity_json, serializer=None): if activity_json.get("publicKeyPem"): # ugh activity_json["type"] = "PublicKey" + + activity_type = activity_json.get("type") try: - activity_type = activity_json["type"] serializer = activity_objects[activity_type] except KeyError as e: + # we know this exists and that we can't handle it + if activity_type in ["Question"]: + return None raise ActivitySerializerError(e) return serializer(activity_objects=activity_objects, **activity_json) diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index 23032a87..400939d0 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -16,7 +16,11 @@ class Verb(ActivityObject): def action(self): """ usually we just want to update and save """ - self.object.to_model() + obj = self.object + # it may return None if the object is invalid in an expected way + # ie, Question type + if obj: + obj.to_model() @dataclass(init=False) diff --git a/bookwyrm/tests/views/inbox/test_inbox_create.py b/bookwyrm/tests/views/inbox/test_inbox_create.py index f8ed6a84..16c674e0 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_create.py +++ b/bookwyrm/tests/views/inbox/test_inbox_create.py @@ -6,6 +6,7 @@ from unittest.mock import patch from django.test import TestCase from bookwyrm import models, views +from bookwyrm.activitypub import ActivitySerializerError # pylint: disable=too-many-public-methods @@ -51,7 +52,7 @@ class InboxCreate(TestCase): } models.SiteSettings.objects.create() - def test_handle_create_status(self): + def test_create_status(self): """ the "it justs works" mode """ self.assertEqual(models.Status.objects.count(), 1) @@ -82,7 +83,7 @@ class InboxCreate(TestCase): views.inbox.activity_task(activity) self.assertEqual(models.Status.objects.count(), 2) - def test_handle_create_status_remote_note_with_mention(self): + def test_create_status_remote_note_with_mention(self): """ should only create it under the right circumstances """ self.assertEqual(models.Status.objects.count(), 1) self.assertFalse( @@ -105,7 +106,7 @@ class InboxCreate(TestCase): ) self.assertEqual(models.Notification.objects.get().notification_type, "MENTION") - def test_handle_create_status_remote_note_with_reply(self): + def test_create_status_remote_note_with_reply(self): """ should only create it under the right circumstances """ self.assertEqual(models.Status.objects.count(), 1) self.assertFalse(models.Notification.objects.filter(user=self.local_user)) @@ -126,7 +127,7 @@ class InboxCreate(TestCase): self.assertTrue(models.Notification.objects.filter(user=self.local_user)) self.assertEqual(models.Notification.objects.get().notification_type, "REPLY") - def test_handle_create_list(self): + def test_create_list(self): """ a new list """ activity = self.create_json activity["object"] = { @@ -149,3 +150,23 @@ class InboxCreate(TestCase): self.assertEqual(book_list.curation, "curated") self.assertEqual(book_list.description, "summary text") self.assertEqual(book_list.remote_id, "https://example.com/list/22") + + def test_create_unsupported_type(self): + """ ignore activities we know we can't handle """ + activity = self.create_json + activity["object"] = { + "id": "https://example.com/status/887", + "type": "Question", + } + # just observer how it doesn't throw an error + views.inbox.activity_task(activity) + + def test_create_unknown_type(self): + """ ignore activities we know we've never heard of """ + activity = self.create_json + activity["object"] = { + "id": "https://example.com/status/887", + "type": "Threnody", + } + with self.assertRaises(ActivitySerializerError): + views.inbox.activity_task(activity) From bd294cce837c9611aa727e838851daa4bab76c1a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 16 Apr 2021 15:17:55 -0700 Subject: [PATCH 2/2] Check if obj exists for updates --- bookwyrm/activitypub/verbs.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index 400939d0..442e3858 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -58,7 +58,9 @@ class Update(Verb): def action(self): """ update a model instance from the dataclass """ - self.object.to_model(allow_create=False) + obj = self.object + if obj: + self.object.to_model(allow_create=False) @dataclass(init=False)