-
Notifications
You must be signed in to change notification settings - Fork 83
Actors: Don't convert non-numeric strings to Blog user id #1554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (4)
- .github/changelog/1554-from-description: Language not supported
- includes/collection/class-actors.php: Language not supported
- includes/functions.php: Language not supported
- tests/includes/collection/class-test-actors.php: Language not supported
pfefferle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice find!
|
What do you suggest shall we do about those erroneously created entries? Can we identify the duplicate entries in a migration step and delete them? |
|
Oh, and thanks so much for identifying and fixing this! It was baffling to me what was causing this. |
|
I wouldn't do anything about them. They'll be deleted automatically with the Outbox purge job. |
|
These yes, but as a side effect I've see lots of duplicate "default fields" being generated, specifically custom post types |
|
Oh, I didn't see that on my test site. Yes, those we should take care of. I can take a stab at that |
* Add failing test * Add changelog * Only convert to int when it's numeric * Use strict checks in user_can_activitypub * Revert "Use strict checks in user_can_activitypub" This reverts commit 8de3516. * Bail when user_id is not numeric
* tag '5.7.0': (29 commits) Release 5.7.0 (Automattic#1574) Fix: Show error if site uses "Almost Pretty Permalink" structure (Automattic#1570) Admin: Add padding to extra fields nav links (Automattic#1569) Signature: Add compat for more key encodings (Automattic#1557) Reply block: Improve fallback embed (Automattic#1560) Add label for "Health Check" and "REST API" (Automattic#1571) change changelog items (Automattic#1572) Transformers: Allow setting properties to false. (Automattic#1567) Upgrades: Delete orphaned extra fields (Automattic#1566) Fix: Missing Actor in Outbox-Activities (Automattic#1564) Outbox: Properly handle username requests (Automattic#1559) Import: Load on admin_init (Automattic#1561) Move: support same-server domain migrations (Automattic#1530) Follow Me: add a button-only mode (Automattic#1133) Tests: Convert timestamp to int for comparison (Automattic#1556) Reply: Make Mastodon embeds work (Automattic#1555) Add: `Vary` header settings (Automattic#1552) Actors: Don't convert non-numeric strings to Blog user id (Automattic#1554) Add: Shared Inbox setting (Automattic#1553) Add: New Health checks (Automattic#1524) ...
Fixes a bug in our Enable Mastodon Apps integration that can create hundreds of Outbox items.
The
mastodon_api_accountfilter passes a user_id or author_url as the second parameter, which we currently don't account for inEnable_Mastodon_Apps::api_account_internal().Requests like the following end up creating two extra fields post for every comment from Activitypub commenters, which in turn create an Outbox item each:
Stack trace:
The
author_urlwe receive inEnable_Mastodon_Apps::api_account_internal()gets passed toActors::get_by_id(), where this check converts it to0, clearing theuser_can_activitypub()check on blogs with the blog user enabled.Further down in
Enable_Mastodon_Apps::api_account_internal()it gets the extra fields for the user, which in this case is still the author_url and not an internal user_id. This non-ID id gets passed down all the way toExtra_Fields::default_actor_extra_fields(), where it can't find cached extra fields for it and fails to save the default extra fields it just created, since it doesn't have a valid user_id.Every time an Extra Field post gets created, it creates an Update activity for the blog user.
Proposed changes:
is_string()check that on its own was enough to continue with the process.Other information:
Testing instructions:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Sites with comments from the Fediverse no longer create uncached extra fields posts that flood the Outbox.