WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 6 months ago

#12400 reopened feature request

Add a wp_loaded hook, an ob_start hook, and an front end ajax hook

Reported by: Denis-de-Bernardy Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: Bootstrap/Load Keywords: has-patch dev-feedback 2nd-opinion westi-likes 3.6-early
Focuses: Cc:

Description (last modified by Denis-de-Bernardy)

Requests for some kind of wp_loaded hook have crept up here and there in trac over the years.

Typically, the requester is looking into doing front-end ajax requests and the like. There are other use cases, such as wanting to catch specific URIs -- e.g. a trailing /print/ to the url, which the permalink API is incapable of catching.

They all got rejected on grounds that there is the init hook that can be used just as well for ajax. Or the template_redirect hook in place of an ob_start hook. The list goes on.

When you want WP and plugins to be loaded and fully initialized, instantiated and ready to go, setting a priority to obscene levels on the init hook works (I typically use 1000000)... but it always feels like you're working around a crippled API.

Starting output buffers on template_redirect with a priority -1000000 feels equally clunky.

Then, there is the front-end ajax. Yes, admin-ajax.php can be used unauthenticated... But the fact of the matter is, you can end up with SSL turned on when it's not useful, and the lack of an wp-ajax.php file makes many a plugin dev wonder where in the bloody hell he should catch his own requests.

It would be sweet if this all got fixed in WP 3.0.

The argument that goes "a hook already exists" seems extremely invalid to me. There are many places in WP where two hooks (and oftentimes many more) can be used to achieve the same result. Think wp_headers and send_headers, for instance. What they have in common is some kind of before/after flow, which init and template_redirect are currently lacking.

One could argue that parse_request is nearby init, and that wp is nearby template_redirect, so they'd be good enough. But the first of these parses expensive regular expressions before firing, and both are only known to WP junkies.

Suggested hooks for WP 3.0:

  • a wp-ajax.php file built similarly to admin-ajax.php.
  • an wp_loaded hook at the very end of wp-settings.php, with a commentary that tells plugin authors that init should be used to instantiate, wp_loaded should be used to act once everything is instantiated, and that wp-ajax.php has hooks that are specific to ajax requests.
  • an ob_start (or pre_load_template, or whatever...) hook at the very beginning of template-loader.php, with a commentary that tells plugin authors that the new hook should be used to instantiate such as output buffering once WP is fully loaded, while the second is traditionally used to pick an arbitrary template.

Attachments (8)

wp-ajax.diff (944 bytes) - added by Denis-de-Bernardy 4 years ago.
wp-ajax.php file
wp-loaded.diff (946 bytes) - added by Denis-de-Bernardy 4 years ago.
wp_loaded hook after init
wp-ajax.php (541 bytes) - added by dd32 4 years ago.
Replacing old file with new without debug + corrected comments
12400.diff (767 bytes) - added by Denis-de-Bernardy 4 years ago.
12400.2.diff (2.7 KB) - added by Denis-de-Bernardy 4 years ago.
12400-cleanup.patch (1.2 KB) - added by TobiasBg 4 years ago.
Patch removes obsolete wp_ajaxurl() function and fixes a line of doc.
12400.3.diff (17.3 KB) - added by sivel 3 years ago.
New try at frontend ajax handler
12400.4.diff (17.7 KB) - added by sivel 3 years ago.
Refreshed patch

Download all attachments as: .zip

Change History (53)

comment:1 Denis-de-Bernardy4 years ago

  • Description modified (diff)

Denis-de-Bernardy4 years ago

wp-ajax.php file

Denis-de-Bernardy4 years ago

wp_loaded hook after init

comment:2 Denis-de-Bernardy4 years ago

  • Keywords needs-testing added

the new template_include hook in WP 3.0 makes a pre_template_redirect borderline useless.

the wp-ajax.php hook is completely untested.

comment:3 dd324 years ago

wp_loaded

How does that differentiate from add_action('init', '_my_func', 1000); ?

As for wp-ajax, I'm +1 for that, But i dont think we need to do the admin includes. Plugins would have to change to use this, they can include the specific admin functions if they require it.

comment:4 follow-up: dd324 years ago

But i dont think we need to do the admin includes

Ah, I see you've used the same Ajax actions for admin & front end.

I'm not too sure thats a good idea myself. A front end ajax handler should be just that, a front end ajax hander, theres no real point having multiple entry points into the same actions.

comment:5 scribu4 years ago

  • Keywords dev-feedback has-patch added; needs-testing removed

comment:6 in reply to: ↑ 4 Denis-de-Bernardy4 years ago

Replying to dd32:

But i dont think we need to do the admin includes

Ah, I see you've used the same Ajax actions for admin & front end.

I'm not too sure thats a good idea myself. A front end ajax handler should be just that, a front end ajax hander, theres no real point having multiple entry points into the same actions.

I'm fine with the idea of making new hooks, personally. As long as *something* gets checked into WP 3.0 that makes it clear to plugin authors where they should be hooking ajax handlers.

The difference between init @ 1000 and wp_loaded, would be the same as the difference between, say, wp_headers and send_headers; or between request, and parse_request. It goes down to semantics: on the one hand side, you're "doing". On the other you're "done" - with no chance of seeing escalation races.

A functionality on the template_redirect hook might make the example more clear. Suppose you've a plugin that starts an output buffer on template_redirect. You'll make the hook start very early (e.g. -1000). The last thing you want is users who complain that your plugin isn't compatible with XYZ, only to note that XYZ is using the same hook as you are, and happens to doing its stuff, then die, before the template_redirect hook is fully executed.

The same goes here on the init hook. There are cases where you want to do something once WP is loaded, but the lack of a second hook is so that you'll occasionally see a plugin finish initializing itself *after* another plugin has decided that "it's my go, time to do something and die". Having a second hook around would make the first hook about instantiation, without action; the second would be about "at this point, everything that needs to be initialized is initialized; do stuff and die at will".

comment:7 dd324 years ago

Denis: It'd be a HUGE help if you can create patches off an up to date Trunk checkout. I'll merge this one manually tho.

comment:8 dd324 years ago

For Ajax hander, See also: #11062

comment:9 dd324 years ago

(In [13481]) add a wp_loaded action that fires once WordPress init has finished. Props Denis-de-Bernardy. See #12400

comment:10 dd324 years ago

attachment wp-ajax.php added

Thats my version of a ajax handler, Things to note:

  • 404 on empty action, or non-handled action
  • Passes unslashed data as the $args
    • This is different from the rest of the user-data functions that pass slashed data.
  • uses the ajax_<action name> style for the hook name
  • Does not return a 0 or -1 as the admin handler does (Thats for the wp-ajax classes, which i doubt will be used by 99% of those who use this file)
  • Sets the default content-type/charset, but plugins should be able to override that if they're printing XML or JSON or otherwise.

dd324 years ago

Replacing old file with new without debug + corrected comments

comment:11 Denis-de-Bernardy4 years ago

looks good. I'd give your patch one tiny tweak: try $_POST first, fallback to $_GET. Because $_REQUEST can contain a bit of both. or better yet, don't pass anything and let authors decide, to avoid inconsistencies.

Denis-de-Bernardy4 years ago

comment:12 Denis-de-Bernardy4 years ago

  • Keywords dev-feedback removed

the refreshed patch also sends a 400 (invalid request) instead of a 404.

comment:13 Denis-de-Bernardy4 years ago

another potential addition could be to automatically set the wp-ajax.php url on the wp_head hook, much like we're doing in the backend. that way, plugins won't have to worry about inserting it, and will be able to use it directly.

Denis-de-Bernardy4 years ago

comment:14 Denis-de-Bernardy4 years ago

updated patch adds and documents a function so that plugin devs get an ajaxurl variable by simply calling a function.

comment:15 nacin4 years ago

Strongly against passing $_REQUEST to an action, which appears in at least one patch above. This has been shot down previously in #11015 as well.

comment:16 nacin4 years ago

(In [13507]) Remove reference to file that doesn't exist. see #12400

comment:17 Denis-de-Bernardy4 years ago

Re:

rboren
And how about those who want to use a custom blogs.php. Do they see the message forever?

how about we add an option that allows to dismiss the message?

comment:18 Denis-de-Bernardy4 years ago

woops, wrong ticket. :P

comment:19 Denis-de-Bernardy4 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Fixed (in [13527]): Add a front end Ajax handler.

comment:20 nacin4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Per IRC discussion with rboren and sivel, we may axe wp-ajax.php.

Some arguments made there:

  1. An uneducated plugin dev needs education, not a slightly more obvious, redundant API layer.
  1. Using wp-ajax.php is inherently less secure.
  1. It has the benefit of being able to require the auth cookies instead of just the logged_in cookie. The fact that it cannot make use of the proper auth cookies makes it worthy of death.

comment:21 sivel4 years ago

  • Cc matt@… added

comment:22 follow-up: TobiasBg4 years ago

I agree that it should be dropped, if it causes too much trouble. On the other hand however, it really is a great thing as it enables (nopriv) AJAX requests from the frontend, for those sites where admin-ajax.php is e.g. behind a .htaccess or running on SSL. So, I'd really like for it to be kept to have something to work with in those situations.

comment:23 in reply to: ↑ 22 sivel4 years ago

Replying to TobiasBg:

for those sites where admin-ajax.php is e.g. behind a .htaccess or running on SSL.

As for the .htaccess concern you can easily remove admin-ajax.php from your restrictions by doing something like:

<Files admin-ajax.php>
    Order allow,deny
    Allow from all
    Satisfy any
</Files>

As for SSL, as nacin, rboren and myself discussed, we agree that it would actually be a benefit if the admin was protected by SSL.

comment:24 TobiasBg4 years ago

Thanks for that .htaccess. While it would work of course, I don't think it really is an option for a plugin that wants to use frontend AJAX, to have the site admin edit his original .htaccess.

And of course I agree, that SSL for the admin is awesome! However, if a plugin just wanted to do some AJAX (to obtain some arbitrary plugin value/string for display on the frontend), it would be total overhead to do that over a SSL connection, in my opinion.

What I'm saying: Frontend AJAX should be easy. The correct end-point is not really admin-ajax.php, as there might be cases were that would not be optimal, as stated for the two possibilities of .htaccess or SSL.

comment:25 nacin4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [13828]) Remove wp-ajax.php. admin-ajax.php can be used (and is better for) for front-end/nopriv AJAX requests. fixes #12400

comment:26 TobiasBg4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

There's still some cleanup necessary, beyond the removal of the file. The attached patch removes the now obsolete wp_ajaxurl() again (as the URL would be wrong anyway) and fixes a doc line.

(To come back to the dev chat: I now understand the security problems caused by wp-ajax.php, so I agree to the removal. However, I would appreciate, if a core dev could maybe write a blog post on how to do frontend AJAX the correct way (including workarounds for the mentioned problems). nacin, might be something for your site :-) )

Thanks!

TobiasBg4 years ago

Patch removes obsolete wp_ajaxurl() function and fixes a line of doc.

comment:27 nacin4 years ago

I kind of like wp_ajaxurl() pointing to admin-ajax.php, but it was suggested that is best left for a/the plugin in the dev chat. Will pull it.

comment:28 nacin4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [13831]) Remove references to wp-ajax.php. props TobiasBg, fixes #12400

comment:30 azizur4 years ago

  • Cc prodevstudio+wordpress@… added

comment:31 sivel3 years ago

  • Milestone changed from 3.0 to Future Release
  • Resolution fixed deleted
  • Status changed from closed to reopened

sivel3 years ago

New try at frontend ajax handler

comment:32 sivel3 years ago

  • Keywords dev-feedback 2nd-opinion added

Reopening this because I believe there may be a way to accomplish this in a way that doesn't duplicate a lot of code and cause security vulnerabilities and be backwards compatible, and add no new ajax hooks.

I think we can make wp-ajax.php the primary ajax handler, at least from the standpoint that the actions are fired in this file, separate out the functionality in the switch statements in admin-ajax.php into actual functions, have admin-ajax.php hook into the wp_ajax_ and wp_ajax_nopriv_ actions, and include wp-ajax.php

This is just a thought, with a proof of concept patch.

Let me know what your thoughts are, if it is deemed that this still isn't a good scenario then we can close the ticket.

I still believe we can do without this patch as long as we keep pushing education as the answer. One of the biggest points of confusion I see is surrounding the fact that admin-ajax.php lives in wp-admin, and people assume that it should not be used for frontend ajax. We also see scenarios where the security put in place on wp-admin causes issues. I offered a suggestion in a previous comment for this as well.

This patch should be backwards compatible, and would allow both admin-ajax.php and wp-ajax.php to be used as ajax endpoints. It moves a lot of code that should have probably been in separated functions to begin with, and utilizes the WP API for hooking in, rather than huge switch statements.

In any case, I figured I had some free time, an idea and thought we could give this another go.

sivel3 years ago

Refreshed patch

comment:33 juliobox20 months ago

  • Cc juliobosk@… added

comment:34 follow-up: johnjamesjacoby20 months ago

Other reasons why there should be a dedicated theme-side AJAX handler:

  • admin-ajax.php sets WP_ADMIN to true, causing plugins that use is_admin() to load admin code for theme-side requests.
  • admin-ajax.php fires the 'admin_init' action, executing code hooked to it from inside the above mentioned included files.

Theoretically, the only way to avoid loading admin code for theme-side AJAX requests is to explicitly check for DOING_AJAX before loading anything. This pollutes plugin code with a lame constant check that is likely to go away eventually, given the move away from constant usage in recent WordPress versions.

I'd settle for a front-ajax.php inside of /wp-admin/ that didn't set WP_ADMIN and didn't fire the 'admin_init' action. I'd even go for a $_GET hack to bypass those two issues:

var ajaxurl = 'http://bbp.com/wp-admin/admin-ajax.php?admin=false';
Last edited 20 months ago by johnjamesjacoby (previous) (diff)

comment:35 ethitter20 months ago

  • Cc erick@… added

comment:36 Viper007Bond20 months ago

  • Cc Viper007Bond added

comment:37 in reply to: ↑ 34 westi20 months ago

  • Keywords westi-likes 3.6-early added

Replying to johnjamesjacoby:

Other reasons why there should be a dedicated theme-side AJAX handler:

  • admin-ajax.php sets WP_ADMIN to true, causing plugins that use is_admin() to load admin code for theme-side requests.
  • admin-ajax.php fires the 'admin_init' action, executing code hooked to it from inside the above mentioned included files.

Theoretically, the only way to avoid loading admin code for theme-side AJAX requests is to explicitly check for DOING_AJAX before loading anything. This pollutes plugin code with a lame constant check that is likely to go away eventually, given the move away from constant usage in recent WordPress versions.

I'd settle for a front-ajax.php inside of /wp-admin/ that didn't set WP_ADMIN and didn't fire the 'admin_init' action. I'd even go for a $_GET hack to bypass those two issues:

var ajaxurl = 'http://bbp.com/wp-admin/admin-ajax.php?admin=false';

I'm beginning to think that we should have the frontend-ajax.php live in wp-includes.

We can update the secure cookie setting to also set the cookie for this path / folder.

It's more obvious that it isn't an admin request.

It shouldn't accidentally trigger a bad plugin test for admin requests that looks at the url.

We should talk about this for 3.6

comment:38 nacin20 months ago

Replying to westi:

We can update the secure cookie setting to also set the cookie for this path / folder.

Not sure I like this. But I do like the idea overall. I would probably like to keep it in wp-admin though, to get that cookie benefit. I've been wanting to retire the wp-content/plugins secure cookie for a while; adding another is kind of lame. (Though could be paired nicely with an overhaul of our cookie handling, in particular to set us up for cross-domain login for multisite, but I digress.)

Yeah, it's time for a new ajax handler that does not set WP_ADMIN or fire admin_init. This was a mess when working with the customizer. I wouldn't mind having all new core handlers go through it, either. We do have some decisions to make on exactly how it should work and load things — probably worth going through tickets to see what other complaints and ideas there are.

comment:39 kovshenin20 months ago

  • Cc kovshenin added

comment:40 yoavf20 months ago

  • Cc yoavf added

comment:41 mfields19 months ago

  • Cc michael@… added

I think that a front-end Ajax would be very handy! One thing that I think would be beneficial to have is a core-supported way of handling javascript enqueues via javascript. Let's say we have a theme that loads full posts via Ajax (P2 would be a good example, also any theme with built in infinite scroll) and we also have a custom plugin installed which provides a shortcode that relies on javascript for some of it's functionality. More times than not this plugin will conditionally print its javascript in the footer (like a Jedi!). The problem is that the current page has already loaded all of it's scripts and the shortcode's script will never be loaded.

comment:42 iamtakashi19 months ago

  • Cc takashi@… added

comment:43 DJPaul18 months ago

  • Cc djpaul@… added

comment:44 leewillis7714 months ago

  • Cc junk@… added

comment:45 nacin6 months ago

  • Component changed from General to Bootstrap/Load
Note: See TracTickets for help on using tickets.