Skip to content
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

YOAST + Elementor + ACF Pro: Infinite Loop with wp_insert_post() - [Edge Case] #14643

Open
6 of 9 tasks
acf-extended opened this issue Mar 28, 2020 · 3 comments
Open
6 of 9 tasks

Comments

@acf-extended
Copy link

acf-extended commented Mar 28, 2020

  • I've read and understood the contribution guidelines.
  • I've searched for any related issues and avoided creating a duplicate issue.

Please give us a description of what happened.

I found a compatibility problem between Elementor, YOAST and ACF Pro. While this is an edge case, I think it should be looked at because it could cause some problems with others plugins.

Note: This is a cross-bugreport on both Elementor & YOAST github repo.
Elementor bugreport is available here.

How can we reproduce this behavior?

  • Add the following code in the theme functions.php file:
add_action('wp', function(){
	
	if(isset($_POST['test']) && !empty($_POST['test'])){
		
		if(have_rows('repeater', 32)):
			while(have_rows('repeater', 32)): the_row();
			
				// Log
				error_log('wp_insert_post');
				
				// wp_insert_post
				wp_insert_post(array(
					'post_title' => 'Page',
					'post_type' => 'page',
				));
				
				// Inifinite loop...
			
			endwhile;
		endif;
		
	}
	
	
});

add_shortcode('test', function(){
	
	if(have_rows('repeater', 32)):
		while(have_rows('repeater', 32)): the_row();
	
			// Do something with the repeater ...
		
		endwhile;
	endif;
	
	ob_start();
	
	?>
	<form action="" method="post">
		
		<input type="hidden" name="test" value="1" />
		<button>Submit</button>
		
	</form>
	<?php
	
	return ob_get_clean();

});
  • In ACF: Create a new field group Test with the conditional logic: Post Type = Page
  • In the field group add a repeater/flexible content field called repeater
  • Add a row with a single sub field called Test
  • Create a new page & add a new row in the ACF repeater field
  • Save the page and edit with Elementor
  • Add a shortcode widget, with the following code: [test]
  • Save the page
  • Retrieve the post_id and replace it in the code above (in all have_rows('repeater', 32) parts)
  • Display the page and click on the "Submit" button
  • The form is processed, and an infinite loop of post creation is triggered

The fundamental problem:

On each post creation, YOAST check the new post content for their "Link Checker" feature using the save_post hook (which is triggered inside wp_insert_post()).

When YOAST do that check, it retrieve the post content using apply_filters('the_content'). But Elementor add specific filters to the_content, and load all components and shortcodes. In our case, applying the shortcodes will print the form which trigger an infinite loop.

Potential bugfix

I found a workaround by disabling the YOAST "Link Checker" using the following hook, right before the wp_insert_post():

add_filter('wpseo_should_index_links', '__return_false');

However, this can also be fixed if we remove the filter applied on the_content in the file: /elementor/includes/frontend.php:273. The way it is implemented right now means that Elementor load all components & shortcodes when using wp_insert_post() in conjunction with YOAST, which seems problematic.

Technical info

  • If relevant, which editor is affected (or editors):
  • Classic Editor
  • Gutenberg
  • Classic Editor plugin
  • Which browser is affected (or browsers):
  • Chrome
  • Firefox
  • Safari
  • Other

Used versions

  • WordPress version: 5.3.2
  • Yoast SEO version: 13.3
  • Relevant plugins in case of a bug:
  • ACF Pro 5.8.9 (latest)
  • Elementor 2.9.7 (latest)
  • Tested with theme: Twenty Twenty
@ite-klass
Copy link

ite-klass commented Jul 6, 2021

I think we have run into this issue with a slightly different setup. Took me a while to find the issue of this intransparent infinite loop.

As we verified this issue exists both on older versions and current versions I will list both:

  • Wordpress 5.6.4 and 5.7.2
  • Elementor 3.1.3 and 3.2.5 (as this seems relevant according to OP, although I did not see that myself)
  • WP Job Openings 2.1.1 and 2.2.0
  • Pro Pack for WP Job Openings 1.3.1 and 2.0.0

The jobs plugin adds a form which allows for job applications to be sent in. These are created as posts with sub-type awsm_job_application (post_type in DB, object_sub_type on $indexable in the indexable link builder).

The form may be sent in via ajax, which is not a problem, or POST to the page, which causes the infinite loop.

I don’t see why the behavior should be different if YOAST SEO wants to update link metadata for new posts?

My understanding is that build_indexable is hooked into/for wp_insert_post. In https://github.com/Yoast/wordpress-seo/blob/trunk/src/integrations/watchers/indexable-post-watcher.php#L205 it identifies and changes content, and saves it at https://github.com/Yoast/wordpress-seo/blob/trunk/src/integrations/watchers/indexable-post-watcher.php#L207.
This causes another wp_insert_post which triggers the hook yet again. This loop is triggered indefinitely.

So it seems to me that this hook is missing an adequate repetition/handled check.

Possibly it should only index posts of known and accepted post subtypes?

@ite-klass
Copy link

ite-klass commented Jul 6, 2021

The result of this is thousands of junk/duplicate entries/posts btw. Cleaning those up is infeasible for an admin user (due to paging and bin) and has to be cleaned up by a DB admin every time it happens. So the impact of this hole is quiet high and bothersome.

The hook not only loops, but creates a new post every loop. It’s a busy loop too, which has potential for DoS attacks.

For that reason I would not agree with severity minor.

@Djennez
Copy link
Member

Djennez commented Aug 9, 2021

If I read the original issue correctly, the problem here is the fact that potential looping code is executed when the_content filter is called. We call the_content to check the post for specific content that would be rendered when the post is live, which, I believe, is a normal usage of this filter. With WordPress being the pluggable and modular system that it is, I am having a hard time imagining no other plugins that would / could be doing the same thing and potentially triggering this loop as well. I feel it's the responsibility of the developer of the looping code to account for a possible use of the_content filter and make sure it does not cause loops.

Please provide additional thoughts on the matter. Since this is an edge-case that may not be caused by our usage of code, it currently does not have priority on our side to be picked up. But if any information is provided that would indicate that we would have to change something (or my assumption from before is incorrect), we can definitely look into this more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants