Repository navigation
Conversation
jeherve
left a comment
There was a problem hiding this comment.
I tested this one together with #3877, since I think the two need to ship together.
With both branches merged locally, the object stored in the outbox matched what a logged-out visitor sees, for the Create and for an Update after an edit.
I have a few remarks on both PRs. I'm putting it all here so it's in one place.
This PR
wp_set_current_user() resets the kses filters. It fires set_current_user, and core hooks kses_init() on it. So once transform_object_properties() has run, the kses filters are back to the default for that user, whatever the caller had set up before:
- as an author, filters removed with
kses_remove_filters()are back on; - as an admin, filters added with
kses_init_filters()are gone.
I couldn't find a caller in the plugin that's affected today (Outbox::add() transforms before it touches kses), so it's more of a trap for later. I'm wondering if we should remember the state and put it back?
There's no finally. If a getter throws (a third-party callback on activitypub_the_content, for example), the rest of the request runs as user 0. Something like this would cover both remarks; I haven't tested it:
$current_user_id = \get_current_user_id();
$has_kses = false !== \has_filter( 'content_save_pre', 'wp_filter_post_kses' );
\wp_set_current_user( 0 );
try {
// ...
} finally {
\wp_set_current_user( $current_user_id );
$has_kses ? \kses_init_filters() : \kses_remove_filters();
}#3877
The global $post isn't put back. generate_post_summary() sets $GLOBALS['post'], then calls wp_reset_postdata(). That restores the main query's post, not the one that was there before. I tried a template starting with [ap_excerpt], and transformed post B while post A was the queried post: I got B's excerpt, followed by A's title, permalink and content. It needs a post published during a front-end page view, so it's narrow, but trunk gets it right. Keeping the previous global and restoring it should be enough.
The summary picks up more than it used to. It's now rendered, so whatever the_content outputs ends up in there. I got "SearchSearch" from a search block, and markup appended by a filter came through too. When a plugin shows a prompt in place of hidden content, the prompt is in the summary, along with long runs of whitespace-only lines; the \R+ cleanup doesn't catch lines that only contain spaces. the_content also runs twice per object now. I don't have a great answer for the noise, but the whitespace should be easy to fix :)
Not covered by either PR
Access decided from the request. Switching the user covers plugins that check the current user. Some also grant access from a cookie or a token on the request, and that still applies when the author publishes from their own browser. I don't think we can solve that here; it has to be handled by the plugin doing the gating (i.e. Jetpack). is_activitypub_request() is already true during the transform, so they have something to check. I have a patch here for an example: Automattic/jetpack#53490
Attachments are built from the raw post_content (get_block_attachments()), so an image that a plugin hides from the_content is still attached to the object, with its alt text. Mentions and the reply / quote targets are read the same way. Should that be part of this fix, or a follow-up?
What do you think?
|
I added the kses part: the transformer now checks if I didn't add the #3877: the global Access decided from the request: I agree, that has to be handled by the plugin that gates the content. Thanks for the Jetpack patch! Attachments, mentions and reply/quote targets: I would do this in a follow-up. It changes how they are collected, not who renders them. |
jeherve
left a comment
There was a problem hiding this comment.
This should be good to go now! 🚢
Proposed changes:
Base::transform_object_properties(), so it covers every place an object is built: publishing and updates, the preview and the content negotiation.the_contentper viewer then put the author's version into the federated object. Posts published by cron already rendered as a visitor.Other information:
The summary is still built from the raw post content. I will look at that in a follow-up, together with a way to remove single
the_contentfilters.Testing instructions:
the_contentfilter that returns something different for logged-in users, e.g.is_user_logged_in() ? 'members' : 'teaser'.Accept: application/activity+json: the content shows the logged-out version.Changelog entry
Changelog Entry Details
Significance
Type
Message
Federated posts and comments now always show what a logged-out visitor sees, not the version shown to the logged-in author.