Load Order: save_mod_rewrite_rules assumes wp-admin/includes/file.php is loaded

[YuriV]

Hi, I'm not completely sure if it's a bug but it definitely caused us some problems. After updating WP to 4.9.6. this morning, we received http 500 erros on our frontpage. The errorlog came up with this. The stack (below) gave us no info about a possible plugin causing this. Our host helped us out by editing misc.php

On line 206 the added:

                if ( ! function_exists( 'get_home_path' ) ) {
                        include_once ABSPATH . '/wp-admin/includes/file.php';
                }

FYI: we never edited core WP files before

Error log after the update to 4.9.6:

==> /var/log/php/error.log <==
[18-May-2018 07:26:39 UTC] PHP Fatal error:  Uncaught Error: Call to undefined function get_home_path() in /data/www/historiek.net/wp-admin/includes/misc.php:204
Stack trace:
#0 /data/www/historiek.net/wp-includes/class-wp-rewrite.php(1775): save_mod_rewrite_rules()
#1 /data/www/historiek.net/wp-includes/class-wp-hook.php(286): WP_Rewrite->flush_rules(true)
#2 /data/www/historiek.net/wp-includes/class-wp-hook.php(310): WP_Hook->apply_filters(NULL, Array)
#3 /data/www/historiek.net/wp-includes/plugin.php(453): WP_Hook->do_action(Array)
#4 /data/www/historiek.net/wp-settings.php(471): do_action('wp_loaded')
#5 /data/www/historiek.net/wp-config.php(98): require_once('/data/www/histo...')
#6 /data/www/historiek.net/wp-load.php(37): require_once('/data/www/histo...')
#7 /data/www/historiek.net/wp-blog-header.php(13): require_once('/data/www/histo...')
#8 /data/www/historiek.net/index.php(17): require('/data/www/histo...')
#9 {main}
  thrown in /data/www/historiek.net/wp-admin/includes/misc.php on line 204

[YuriV]

PS: The error was not only on the frontpage, but on all articles pages. Backend was working fine.

[iandunn]

Can you paste in a list of all the plugins that are active, and the theme too?

[YuriV]

Hi Ian,

Hereby.

Theme
Rhonda 1.5 (Inibot)

Plugins

Accelerated Mobile Pages (0.9.86.2)
Ad Inserter (2.3.8)
Akismet Anti-Spam (4.0.3)
Autoptimize (2.3.4)
Better Search Replace (1.3.2)
Bounce Handler Mailpoet (1.3.8)
Co-Authors Plus (3.3.0)
Code-snippets (2.10.1.1)
Cookie Notice (1.2.42)
Disable Comments (1.7.1)
Doneren met Mollie (2.4.11)
Easy FancyBox (1.8.3)
EWWW Image Optimizer (4.2.1)
GoodBarber Api (1.0.12)
Jetpack (6.1)
MailPoet 3 (3.7.2)
MailPoet 3 Premium (3.0.18)
Q2W3 Fixed Widget (5.1.4)
Redirection (3.2)
Rich Text Tags, Categories, and Taxonomies (1.8)
Schema (3.0.1)
Use Google Libraries (1.6.2.3)
User Rol Editor (4.42)
Varnish HTTP Purge (4.5.0)
Widget CSS Classes (1.5.2.1)
WP Subtitle (3.0)
Yoast SEO (7.5.1)

I did a Google Search and found this: ​https://wordpress.org/support/topic/undefined-function-get_home_path/ So it might have something to do with Mailpoet.

Also found this: ​https://wordpress.org/support/topic/error-500-on-all-pages-after-wordpress-4-9-6-update/

[kraftbj]

@SergeyBiryukov identified the exact path for this particular error, which is caused by wp-admin/includes/misc.php being loaded via wp_add_privacy_policy_content() when an rewrite flush is requested outside of the admin context. That combo leads to save_mod_rewrite_rules function_exists check to pass. That function assumes wp-admin/includes/file.php has been loaded, which isn't true in this case.

Patch incoming.

[kraftbj]

require_once file.php to ensure get_home_path is declared.

[iandunn]

If we can programmatically detect the problem, we could also consider adding a _doing_it_wrong() call, to notify the plugin author which hook they should use.

[kraftbj]

I don't think a doing_it_wrong is merited. Generally, the issue is more our privacy code includes a file that didn't expect itself to be loaded without the rest of the admin suite.

A good Todo is audit the rest of misc.php to see if there are other instances of similar assumptions. I can check that out this afternoon.

[kraftbj]

I made a relatively quick scan of misc.php and I didn't see other functions that I would expect to not be loaded—mainly things from wp-includes/functions.php.

[jcobrien]

Might be related, but upgrading to 4.9.6 made editing variations / attributes in woo commerce go into an infinite loading loop. I verified this on two cloned installations. The only change between working and non-working was this update.

I'd like to add that I replicated this issue using only two active plugins, woo commerce and tinymce...and on 2017 default theme.

[kraftbj]

@jcobrien Can you report that to WooCommerce support via ​https://woocommerce.com/my-account/tickets/ or ​https://wordpress.org/support/plugin/woocommerce ? I haven't personally heard of that, but I would not expect it to be related to this specific ticket.

If you could submit that (and let me know you've done so), as soon as possible, that would be extremely helpful. I know WooCommerce is aiming for an update related to GDPR pretty soon.

[jcobrien]

I have submitted I ticket. I should add that I got a 404 error after update to 4.9.6. I also cannot view the privacy setttings *privacy.php did not exist. and I get a 404 on viewing cron events.

[azaozz]

Replying to jcobrien:

I got a 404 error after update to 4.9.6. I also cannot view the privacy settings *privacy.php did not exist.

This sounds like a partial/failed upgrade. Please reinstall WP. If the problem persists, perhaps post to the ​support forum.

[azaozz]

In 44142.2.diff:

• Add _doing_it_wrong() and return early when a wrong hook was used to call wp_add_privacy_policy_content().
• Add _doing_it_wrong() when rewrite rules are flushed outside wp-admin context.

Also wondering if we should just return early for attempts to flush the rewrite rules outside wp-admin instead of making it possible. Seems that didn't work/wasn't possible before 4.9.6.

Note that in normal circumstances wp_add_privacy_policy_content() cannot be used on the init hook. It throws a fatal error right away as wp-admin/includes/admin.php (Core Administration API) is not loaded yet :)

[kraftbj]

Also wondering if we should just return early for attempts to flush the rewrite rules outside wp-admin instead of making it possible. Seems that didn't work/wasn't possible before 4.9.6.

General flushing of the rewrite rules worked (and still does without a fatal), but hard flushes used to not work and now lead to the fatal described in this ticket under a perfect storm. https://core.trac.wordpress.org/browser/tags/4.9.6/src/wp-includes/class-wp-rewrite.php#L1774

From a performance standpoint, flushing rewrite rules on the front end is a bad idea, but I think we should split that off to a separate ticket since who knows what plugins have been doing. Since we can reasonably fix this issue without doing that, let's not conflate the two issues (loading and new code being hooked in an unintended place vs front-end rewrite flushing).

[kraftbj]

doing_it_wrong on any non-admin flush

[kraftbj]

@azaozz What about indicating that rules should only be flushed within wp-admin within the flush_rewrite_rules function that is typically called, as seen in 44142.3.diff?

[SergeyBiryukov]

Replying to kraftbj:

What about indicating that rules should only be flushed within wp-admin within the flush_rewrite_rules function that is typically called, as seen in 44142.3.diff?

44142.3.diff​ has a non-zero chance of displaying unexpected notices on front end if someone accidentally has WP_DEBUG enabled on production.

Developer Reference has an ​example of flushing rewrite rules on front end every 48 hours for theme development.

I'd be more comfortable with landing this notice in a major release with a dev note written at beta stage than in an automatically installed minor update.

I have no objections to the notice in wp_add_privacy_policy_content() though, since that's new functionality.

[azaozz]

What about indicating that rules should only be flushed within wp-admin within the flush_rewrite_rules function that is typically called, as seen in 44142.3.diff?

Yeah, makes sense.

I'd be more comfortable with landing this notice in a major release...

Makes sense too. Also perhaps we can add to the "doing it wrong" message that flushing the rules is acceptable only in development.

...I think we should split that (doing it wrong notice when flushing the rules) off to a separate ticket

Agreed. Lets make a ticket with milestone for the next major release.

[azaozz]

Created #44171 as follow-up.

[kraftbj]

Agreed with all of the above. In .4, everything from .3 except removing the notice thrown for rewrite rules. Will add that to #44171.

[kraftbj]

Focus the patch on the load order issue at hand

[azaozz]

44142.4.diff looks good.

[chetan200891]

Just a minor suggestion for DocBlock in 44142.4.diff.

Can we use Plugin Handbook instead of Plugins Handbook? For consistent wording. Because ​https://developer.wordpress.org/plugins/privacy/suggesting-text-for-the-site-privacy-policy/ page title is Plugin Handbook.

[kraftbj]

Same as .4 with "Plugin Handbook" title reference corrected.

[Shelob9]

I wanted to add a link to a conflict related to this we are having for context.

I will add more details if I can find them, but it appears this issue also happens when Caldera Forms is active with Ultimate Member active and persists after the update plugin process. It also happens during update if Divi is active in some way (I can't reproduce this well.)

​https://github.com/CalderaWP/Caldera-Forms/issues/2574#issuecomment-394825977

[desrosj]

The save_mod_rewrite_rules() function's inline documentation is missing a @return tag. We can add that when we commit this.

[SergeyBiryukov]

In 43361:

Privacy: Make sure wp_add_privacy_policy_content() does not cause a fatal error by unintentionally flushing rewrite rules outside of the admin context.

Add a _doing_it_wrong() message describing the correct usage of the function.

Props kraftbj, azaozz, SergeyBiryukov, YuriV.
Fixes #44142.

[SergeyBiryukov]

In 43362:

Docs: Add missing @return value for save_mod_rewrite_rules() and iis7_save_url_rewrite_rules().

See #44142.

[SergeyBiryukov]

In 43363:

Docs: Correct inline comment added in [43361] for consistency with other comments.

See #44142.

[SergeyBiryukov]

In 43364:

Privacy: Make sure wp_add_privacy_policy_content() does not cause a fatal error by unintentionally flushing rewrite rules outside of the admin context.

Add a _doing_it_wrong() message describing the correct usage of the function.

Props kraftbj, azaozz, SergeyBiryukov, YuriV.
Merges [43361], [43362], [43363] to the 4.9 branch.
Fixes #44142.

[desrosj]

#43677 was marked as a duplicate.