8000 Fixes PHP notices (#383) and improve detects of backend request by stodorovic · Pull Request #384 · Automattic/wp-super-cache · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fixes PHP notices (#383) and improve detects of backend request #384

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Fixes PHP notices (#383) and improve detects of backend request #384

wants to merge 5 commits into from

Conversation

stodorovic
Copy link
@stodorovic stodorovic commented Sep 13, 2017 •
edited
Loading
  • Use wp_json_encode if this function exists (WP Core >= 4.1.0). It fixes #349
  • Introduce wpsc_is_backend
  • Improve get_current_url_supercache_dir by using regular expressions
  • Additional checking for $WPSC_HTTP_HOST

@stodorovic
Copy link
Author

I've created function wpsc_is_backend. It's replacement for is_admin and it caches the result. Function is_admin covers only WP dashboard and ajax request ( /wp-admin/admin-ajax.php ). Other PHP scripts as wp-login.php, wp-cron.php, xmlrpc.php, wp-cli and various custom php-cli scripts aren't covered. It doesn't make sense to create output buffer for them. I had troubles in the past. This PR uses wpsc_is_backend to prevent creating output buffer for all these requests.

Second issue which is connected with this topic is executing wp-cron.php using php-cli. There aren't apache environment variables (more info on #383 ) and it triggers some PHP warnings/notices. I made custom shell script which emulate those variables and I'll try to make it public. For now, based on my experience, I tried to add emulate $WPSC_HTTP_HOST on same way. Also, based on RFC3986, WP core regular expressions and my experience, I improved get_current_url_supercache_dir by using regexp which is more efficient (performance, code readability). In addition, it reports "usage of absolute path". (if someone removed hostname).

@donnchawp donnchawp added this to the 1.5.7 milestone Oct 2, 2017
@donnchawp
Copy link
Contributor

Looks great apart from using get_option() in wp-cache-base.php as I think that will produce a fatal error, but I haven't tested that yet.

There's a lot here though, and I want to get 1.5.6 out soon with some minor fixes to the REST API so I'll hold off on this until the release after. Thanks!

@stodorovic
Copy link
Author

Thanks for review!
I've also dilemma about get_option. I'll try to do more tests, it seems that's possible to get fatal error in some cases. I had busy with other tasks, so I'll review it again in next days.
Maybe we can move this part of the code later, but it's good to set $WPSC_HTTP_HOST for people which runs cron via php -q wp-cron.php. I prefer to make shell script which will set proper environment before executing php-cli.

@donnchawp
Copy link
Contributor

Thanks for commenting out the get_option() part. It might be simpler to disable caching if HTTP_HOST isn't set, because it's not clear where the cache files will go, or come from.

That should probably be in a new PR, just for that.

@stodorovic
Copy link
Author
stodorovic commented Oct 25, 2017 •
edited
Loading

Some people run cron job via shell script and my primary concern is preload process - some functions use $WPSC_HTTP_HOST. I'll try to debug all functions and see what's the best approach. I hope that I can do it in next couple days.

Also, all warnings/errors (which I've analyzed) are coming from wp-cron.php which is already excluded from caching.

@stodorovic
Copy link
Author

I've done more checking related to $WPSC_HTTP_HOST. I've added following changes:

  • function wp_cache_verify_config_file - additional condition $WPSC_HTTP_HOST != ''.
  • function get_current_url_supercache_dir - use hostname from get_option( 'home' ) if $WPSC_HTTP_HOST == ''. This function will return correct directory when is called from PHP cli environment.
  • function wp_cache_shutdown_callback - $WPSC_HTTP_HOST is part of meta data and admin/cron processes aren't affected.

I think that's acceptable for now.

@alexsanford alexsanford modified the milestones: 1.5.7, 1.5.9 Oct 26, 2017
@stodorovic stodorovic mentioned this pull request Oct 27, 2017
Open
@donnchawp
Copy link
Contributor

Damn, I left this too long and moving all the functions out of wp-cache-phase1.php has broken this PR.

@stodorovic
Copy link
Author

I just tried to fix conflicts, but it seems that's easier to create totally new PR. I'll try to do it in next couple hours.

@stodorovic stodorovic mentioned this pull request Dec 6, 2017
Merged
@stodorovic stodorovic closed this Dec 6, 2017
@stodorovic stodorovic deleted the fix/383_few_PHP_notices branch December 18, 2017 07:27
@stodorovic stodorovic mentioned this pull request Aug 12, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.5.9
Development

Successfully merging this pull request may close these issues.

json_encode(): Invalid UTF-8 sequence in argument in /wp-cache-phase2.php on line 53
3 participants
@stodorovic @donnchawp @alexsanford
0