Skip to content

Commit

Permalink
Protocol.check_supported: block blank posts
Browse files Browse the repository at this point in the history 
fixes #1737
  • Loading branch information
@snarfed
snarfed committed Feb 1, 2025
1 parent 0f7289b commit e044dec
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 8 deletions.
17 changes: 15 additions & 2 deletions protocol.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from google.cloud import ndb
from google.cloud.ndb import OR
from google.cloud.ndb.model import _entity_to_protobuf
from granary import as1, as2
from granary import as1, as2, source
from granary.source import html_to_text
from oauth_dropins.webutil.appengine_info import DEBUG
from oauth_dropins.webutil.flask_util import cloud_tasks_only
Expand Down Expand Up @@ -1688,7 +1688,7 @@ def load(cls, id, remote=None, local=True, raise_=True, **kwargs):

@classmethod
def check_supported(cls, obj):
"""If this protocol doesn't support this object, return 204.
"""If this protocol doesn't support this object, raises HTTP 204.
Also reports an error.
Expand All @@ -1698,6 +1698,9 @@ def check_supported(cls, obj):
Args:
obj (Object)
Raises:
werkzeug.HTTPException: if this protocol doesn't support this object
"""
if not obj.type:
return
Expand All @@ -1709,6 +1712,16 @@ def check_supported(cls, obj):
and inner_type not in cls.SUPPORTED_AS1_TYPES)):
error(f"Bridgy Fed for {cls.LABEL} doesn't support {obj.type} {inner_type} yet", status=204)

# don't allow posts with blank content and no image/video/audio
crud_obj = (as1.get_object(obj.as1) if obj.type in ('post', 'update')
else obj.as1)
if (crud_obj.get('objectType') in as1.POST_TYPES
and not util.get_url(crud_obj, key='image')
and not any(util.get_urls(crud_obj, 'attachments', inner_key='stream'))
# TODO: handle articles with displayName but not content
and not source.html_to_text(crud_obj.get('content')).strip()):
error('Blank content and no image or video or audio', status=204)

# DMs are only allowed to/from protocol bot accounts
if recip := as1.recipient_if_dm(obj.as1):
protocol_user_ids = PROTOCOL_DOMAINS + common.protocol_user_copy_ids()
Expand Down
1 change: 1 addition & 0 deletions tests/test_activitypub.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@
'object': {
'type': 'Note',
'id': 'https://mas.to/note',
'content': 'foo',
},
}
WEBMENTION_DISCOVERY = requests_response(
Expand Down
62 changes: 56 additions & 6 deletions tests/test_protocol.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -873,9 +873,11 @@ def test_convert_object_adds_source_links_to_create_update(self):

def test_check_supported(self):
for obj in (
{'objectType': 'note'},
{'objectType': 'note', 'content': 'foo'},
{'objectType': 'note', 'attachments': {'stream': {'url': 'http://x/yz'}}},
{'objectType': 'note', 'image': [{'url': 'http://my/pic'}]},
{'objectType': 'activity', 'verb': 'post',
'object': {'objectType': 'note'}},
'object': {'objectType': 'note', 'content': 'foo'}},
{'objectType': 'activity', 'verb': 'follow'},
{'objectType': 'activity', 'verb': 'delete', 'object': 'x'},
{'objectType': 'activity', 'verb': 'undo', 'object': {'foo': 'bar'}},
Expand Down Expand Up @@ -920,6 +922,19 @@ def test_check_supported(self):
with self.subTest(proto=proto), self.assertRaises(NoContent):
proto.check_supported(dm)

# blank content and no video/audio/image
for obj in (
{'objectType': 'note'},
{'objectType': 'note', 'content': ' '},
{'objectType': 'note', 'content': '<p></p>'},
{'objectType': 'activity', 'verb': 'post',
'object': {'objectType': 'note'}},
{'objectType': 'activity', 'verb': 'update',
'object': {'objectType': 'note'}},
):
with self.subTest(obj=obj), self.assertRaises(NoContent):
Fake.check_supported(Object(our_as1=obj))

# from and to a copy id of a protocol bot user
self.make_user(cls=Web, id='ap.brid.gy',
copies=[Target(protocol='fake', uri='fake:ap-bot')])
Expand Down Expand Up @@ -978,6 +993,7 @@ def test_create_post(self):
post_as1 = {
'id': 'fake:post',
'objectType': 'note',
'content': 'foo',
}
create_as1 = {
'id': 'fake:create',
Expand Down Expand Up @@ -1029,6 +1045,7 @@ def test_create_post_bare_object(self):
'id': 'fake:post',
'objectType': 'note',
'author': 'fake:user',
'content': 'foo',
}
self.assertEqual(('OK', 202), Fake.receive_as1(post_as1))

Expand Down Expand Up @@ -1056,6 +1073,7 @@ def test_post_by_user_enabled_atproto_adds_pds_target(self, mock_send):
'id': 'fake:post',
'objectType': 'note',
'author': 'fake:user',
'content': 'foo',
}))

self.assertEqual(1, mock_send.call_count)
Expand Down Expand Up @@ -1098,6 +1116,7 @@ def test_reply_to_not_bridged_account_skips_atproto(self, mock_send):
'objectType': 'note',
'author': 'efake:user',
'inReplyTo': 'efake:post',
'content': 'foo',
})

self.assertEqual(0, mock_send.call_count)
Expand All @@ -1118,6 +1137,7 @@ def test_reply_to_non_bridged_post_with_mention_skips_atproto(self, mock_send):
'objectType': 'note',
'actor': 'fake:user',
'inReplyTo': 'fake:post',
'content': 'foo',
'tags': [{
'objectType': 'mention',
'url': 'other:bob'
Expand Down Expand Up @@ -1146,6 +1166,7 @@ def test_reply_to_non_bridged_post_skips_enabled_protocol_with_followers(self):
'objectType': 'note',
'actor': 'fake:user',
'inReplyTo': 'fake:post',
'content': 'foo',
})
self.assertEqual(204, code)
self.assertEqual([], ExplicitFake.sent)
Expand All @@ -1171,6 +1192,7 @@ def test_reply_from_non_bridged_post_isnt_bridged_but_gets_dm_prompt(self):
'objectType': 'note',
'actor': 'efake:eve',
'inReplyTo': 'fake:post',
'content': 'foo',
})
self.assertEqual(204, code)

Expand Down Expand Up @@ -1507,6 +1529,7 @@ def test_create_post_use_instead(self):
'id': 'fake:post',
'objectType': 'note',
'author': 'fake:user',
'content': 'foo',
}
obj = self.store_object(id='fake:post', our_as1=note_as1,
source_protocol='fake')
Expand All @@ -1532,6 +1555,7 @@ def test_update_post_wrong_actor_error(self):
'id': 'fake:post',
'objectType': 'note',
'author': 'fake:user',
'content': 'foo',
}
self.store_object(id='fake:post', our_as1=post_as1, source_protocol='fake')

Expand All @@ -1547,25 +1571,30 @@ def test_update_post_wrong_actor_error(self):
def test_update_post(self):
self.make_followers()

post_as1 = {
orig_as1 = {
'id': 'fake:post',
'objectType': 'note',
'author': 'fake:user',
'content': 'foo',
}
self.store_object(id='fake:post', our_as1=post_as1, source_protocol='fake',
self.store_object(id='fake:post', our_as1=orig_as1, source_protocol='fake',
copies=[Target(uri='other:post', protocol='other')])

new_as1 = {
**orig_as1,
'content': 'bar',
}
update_as1 = {
'id': 'fake:update',
'objectType': 'activity',
'verb': 'update',
'actor': 'fake:user',
'object': post_as1,
'object': new_as1,
}
self.assertEqual(('OK', 202), Fake.receive_as1(update_as1))

self.assert_object('fake:post',
our_as1=post_as1,
our_as1=new_as1,
type='note',
feed=[self.alice.key, self.bob.key],
users=[self.user.key],
Expand Down Expand Up @@ -1668,6 +1697,7 @@ def test_create_reply(self):
'objectType': 'note',
'inReplyTo': 'other:post',
'author': 'fake:user',
'content': 'foo',
}
create_as1 = {
'id': 'fake:create',
Expand Down Expand Up @@ -1698,6 +1728,7 @@ def test_create_reply_bare_object(self):
'objectType': 'note',
'inReplyTo': 'other:post',
'author': 'fake:user',
'content': 'foo',
}
OtherFake.fetchable['other:post'] = {
'objectType': 'note',
Expand Down Expand Up @@ -1735,6 +1766,7 @@ def test_create_reply_to_self_delivers_to_followers(self):
'objectType': 'note',
'inReplyTo': 'fake:post',
'author': 'fake:user',
'content': 'foo',
}
self.store_object(id='fake:post', source_protocol='fake',
copies=[Target(protocol='other', uri='other:post')],
Expand Down Expand Up @@ -1778,6 +1810,7 @@ def test_create_reply_to_other_protocol(self):
'objectType': 'note',
'inReplyTo': 'fake:post',
'author': 'fake:user',
'content': 'foo',
}
self.assertEqual(('OK', 202), Fake.receive_as1(reply_as1))

Expand Down Expand Up @@ -1839,6 +1872,7 @@ def test_create_self_reply_to_same_protocol_bridge_if_original_is_bridged(self):
'objectType': 'note',
'inReplyTo': 'efake:post',
'author': 'efake:user',
'content': 'foo',
}
self.assertEqual(('OK', 202), ExplicitFake.receive_as1(reply_as1))

Expand Down Expand Up @@ -1875,6 +1909,7 @@ def test_update_reply(self):
'objectType': 'note',
'inReplyTo': 'other:post',
'author': 'fake:user',
'content': 'foo',
}
self.store_object(id='fake:reply', our_as1=reply_as1, source_protocol='fake')

Expand Down Expand Up @@ -2111,6 +2146,7 @@ def test_send_error(self, mock_send):
post_as1 = {
'id': 'fake:post',
'objectType': 'note',
'content': 'foo',
}
create_as1 = {
'id': 'fake:create',
Expand Down Expand Up @@ -2731,6 +2767,7 @@ def test_resolve_ids_reply_mentions(self):
'id': 'fake:reply',
'author': 'fake:user',
'objectType': 'note',
'content': 'foo',
'inReplyTo': [
'fake:unknown-post',
'fake:post',
Expand Down Expand Up @@ -2770,6 +2807,7 @@ def test_resolve_ids_reply_mentions(self):
'id': 'fake:reply',
'objectType': 'note',
'author': 'fake:user',
'content': 'foo',
'inReplyTo': [
'fake:unknown-post',
'fake:post',
Expand Down Expand Up @@ -3077,6 +3115,7 @@ def test_receive_activity_lease(self):
'id': 'fake:note',
'objectType': 'note',
'author': 'fake:user',
'content': 'foo',
},
}

Expand Down Expand Up @@ -3193,6 +3232,7 @@ def test_receive_task_handler_properties(self):
'id': 'fake:post',
'objectType': 'note',
'author': 'fake:other',
'content': 'foo',
}
create = {
'id': 'fake:post#bridgy-fed-create',
Expand Down Expand Up @@ -3228,6 +3268,7 @@ def test_receive_task_handler_authed_as(self):
'id': 'fake:post',
'objectType': 'note',
'author': 'fake:user',
'content': 'foo',
}

got = self.post('/queue/receive', data={
Expand All @@ -3244,6 +3285,7 @@ def test_receive_task_handler_authed_as_domain_vs_homepage(self):
'id': 'https://user.com/c',
'objectType': 'note',
'author': 'https://user.com/',
'content': 'foo',
}

got = self.post('/queue/receive', data={
Expand All @@ -3263,6 +3305,7 @@ def test_receive_task_handler_authed_as_www_subdomain(self, _):
'id': 'http://www.foo.com/post',
'objectType': 'note',
'author': 'http://www.foo.com/bar',
'content': 'foo',
}

got = self.post('/queue/receive', data={
Expand All @@ -3280,6 +3323,7 @@ def test_receive_task_handler_authed_as_mixed_subdomains(self, _):
'objectType': 'note',
'id': 'http://user.com/post',
'author': 'http://m.user.com/',
'content': 'foo',
}

got = self.post('/queue/receive', data={
Expand All @@ -3296,6 +3340,7 @@ def test_receive_task_handler_authed_as_wrong_domain(self, _):
'id': 'http://bar.com/post',
'objectType': 'note',
'author': 'http://bar.com/',
'content': 'foo',
}

got = self.post('/queue/receive', data={
Expand All @@ -3311,6 +3356,7 @@ def test_receive_task_handler_not_authed_as(self):
'id': 'fake:post',
'objectType': 'note',
'author': 'fake:other',
'content': 'foo',
}

got = self.post('/queue/receive', data={
Expand Down Expand Up @@ -3392,6 +3438,7 @@ def test_post_create_send_tasks(self, mock_create_task):
'id': 'fake:post',
'objectType': 'note',
'author': 'fake:user',
'content': 'foo',
}
self.assertEqual(('OK', 202), Fake.receive_as1(note_as1))

Expand Down Expand Up @@ -3426,6 +3473,7 @@ def test_reply_send_tasks_orig_obj(self, mock_create_task):
'objectType': 'note',
'inReplyTo': 'other:post',
'author': 'fake:user',
'content': 'foo',
}
OtherFake.fetchable['other:post'] = {
'objectType': 'note',
Expand Down Expand Up @@ -3463,6 +3511,7 @@ def test_send_task_handler_obj_id(self):
note = self.store_object(id='fake:note', our_as1={
'id': 'fake:post',
'objectType': 'note',
'content': 'foo',
})
target = Target(uri='fake:shared:target', protocol='fake')
create = self.store_object(id='fake:create', our_as1={
Expand Down Expand Up @@ -3546,6 +3595,7 @@ def test_send_task_follow_user_use_instead(self, mock_send):
self.store_object(id='fake:note', our_as1={
'id': 'fake:post',
'objectType': 'note',
'content': 'foo',
})
resp = self.post('/queue/send', data={
'protocol': 'fake',
Expand Down

0 comments on commit e044dec

Please sign in to comment.