Unchecked Indexes in admin-ajax.php

[miqrogroove]

Notices produced by URL guessing.

Notice: Undefined index: action in /wp-admin/admin-ajax.php on line 25

Notice: Undefined index: action in /wp-admin/admin-ajax.php on line 204

Notice: Undefined index: action in /wp-admin/admin-ajax.php on line 1428

[sivel]

Since admin-ajax.php requires $_REQUESTaction? just die early on if it's not set

[miqrogroove]

umm.. if wp_die() isn't appropriate for AJAX, could we at least put an HTTP status in there?

[azaozz]

If the server was reached and responded the HTTP status should stay at 200.

I like the proposed patch no need to proceed if action is not set.

[miqrogroove]

I don't like web spiders seeing status 200 on random URLs.

[sivel]

If we sent any status at all the preferred status would be 400. However to properly send a 400 we would need to move the check after the call to wp-load.php so that we could use status_header() or we end up duplicating code.

Honestly I still prefer to just die with a -1 since that is what we already do when the action is not hooked. Otherwise we start sending inconsistent results back.

[nacin]

With this we can probably remove an isset() check or two later in the file.

Also, it looks like we check for !empty() for wp_ajax_nopriv_ but only isset() for wp_ajax_. Standardizing this on !empty() up top would be good. I don't see a reason for firing the literal hook "wp_ajax_" if admin-ajax.php?action is called.

[nacin]

Scratch that, we'll still need the isset() check around $_GETaction?, as it may be $_POST.

That also means this won't fix everything. Checking for $_REQUESTaction? at the top may still generate a notice on line 25 as $_POSTautosave? may not be set, because it may be GET.

I like the strategy though.

[sivel]

Same as before but adds an additional isset

[nacin]

(In [13175]) Fix notices, props sivel. Fixes #12122