Oliver Campion

Sorry but sanitization is needed for data entered by user not the data coming from code itself.

This statement is incorrect and very worrying that you have said this not only because $_REQUEST data can come from any source, not just user or “code”.

The request to our server with an array delivered to $_REQUEST['action'] instead of a string came from a third party not from our site (plugin, theme or otherwise).

It is imperative that you sanitise your data input before it is used no-matter where its source and you should not assume that the data isn’t from a third party just because that’s where you expect it to come from.

Untrusted data comes from many sources (users, third party sites, even your own database!) and all of it needs to be checked before it’s used.

Common APIs Handbook

To explain as to “what is the point of sending this action when response is 400 bad request” … it is not good practice to allow PHP errors to appear in your error.log and a simple is_string() will resolve that.

That may be true but you are not checking that the input is a string or sanitising the input from $_REQUEST['action'] which at best leaves your preserved_ajax_actions() able to be sent the wrong kind of data input (not just from a third party theme or plugin but also by any constructed request to /?action=).

You should check and sanitise the data by changing line 556 from …

$current_action = ! empty( $_REQUEST['action'] ) ? $_REQUEST['action'] : false;

… to …

$current_action = is_string( $_REQUEST['action'] ) && ! empty( $_REQUEST['action'] ) ? sanitize_text_field( $_REQUEST['action'] ) : false;

OK, understood. If you change your mind and would like me to look into it further let me know.

Thank you for adjusting your review, that would be most gratefully appreciated.

Oliver

Interesting! Can you please take a look at this image on your iPad? The linked image is to the actual header image used in the theme which is 2,000 x 1,200 pixels as per the theme’s recommendations when it was developed in 2016.

This may be the issue as it looks like your iPad’s resolution is 2,732 x 2,048 pixels.

As I recall, the bug that our plugin fixed was caused by the way that Safari used to change the way cover images were shown on iPads back then whereby the images were badly distorted to fill the entire length of a page rather than just the viewport. We’ve not heard of this happening recently so I think it’s likely that your image issue is related to your iPad having a greater resolution than the theme restricts images to (uploaded header media is scaled and cropped to 2,000 x 1,200 pixels).

We may be able to create a fix that increases these dimensions … but … please bear in mind that WordPress core by default scales uploaded images to 2,560 pixels so we would also have to disable that restriction.

Let me know if you’d like us to investigate that for you.

I can totally understand your frustration.

You should find there is already a reply to your support forum topic and hopefully we can resolve your issue speedily.

Hello,

Sorry to hear you’re having issue with this.

Can you please let us know exactly what iPad you have and what iOS version you are running? Also please confirm that you are using Safari.

Also, with regards to the pixelated featured image, are you referring to the default theme header image (plant pot on a table)? We cannot see a page on your site that has an actual featured image? If you have a specific page with a featured image, please provide a link to it so we can inspect it.

Many thanks,

Oliver

Hello,

Thank you for taking the time to review our plugin.

It is a great shame that you didn’t instead take the time to use the support forum to resolve your issue. Had you done so you would have found us accommodating to try to resolve your issue!

Good news … it’s not too late ?? so feel free to head over and leave a support topic.

Kind regards,

Oliver

PS – We are not allowed to answer support requests in reviews
PPS – We cannot talk about premium plugins due to WordPress Forum Guidelines

Thanks for the additional info. Actually I can not replace $query->get( 's' ) with isset($_GET['s'])because this plugin also alter the custom WP query, Ajax requests and REST requests. All of those will stopped working if I do that.

WPES does not alter all REST requests and a simple toggle for REST support is not needed. 

Your comments above state that your plugin both alters and does not alter REST requests so you can see our confusion.

We honestly now are not concerned which of these statements of yours is true, all we know is that there is a case somewhere whereby a request without using the "s" query variable can trigger your plugin to add additional WHERE statements to the query when we don’t want them added because they cause slow MySQL queries.

We take your point about your future plugin development and, as such, have amended our fix for the issues as follows …

add_action('init', 'wp_extended_search_disable', 9);

function wp_extended_search_disable() {

    // Prevent Extended Search if not a site search
    if (!isset($_GET['s'])) {

        add_filter('wpes_enabled', '__return_false');

    }

}

Please note the priority of 9 for the init hook as any higher will not work because your plugin uses the default priority 10 to initialise itself on that hook.

Oliver

OK, thanks for your reply. I’m sorry that you have not seemed to understand the issue again but it’s not a problem for us as our previous mentioned work-around is in place and does what we require.

Oliver

Hello,

To update this thread, we believe we have found the type of URL that is being abused by bad actors …

/wp-json/wp/v2/posts?_fields=content,title,link,date_gmt,modified_gmt&search={search term here}&search_columns=post_content&per_page=100&page=1

For our application of your plugin, we do not require searches to be extended via the /wp-json/ API but only through the use of the s query variable.

So, just to confirm, do you think you would be able to add the ability to enable / disable the implementation of this plugin’s extended search functions on the different types of core searches using checkboxes in your settings page like …

Enable Extended Search for …
? Front end query variable searches
? Back end /wp-json/ API searches

Currently we have successfully done this ourselves with the following code (as previously mentioned) …

add_action('init', 'wp_extended_search_disable', 11);

function wp_extended_search_disable() {

    // Prevent Extended Search if not a site search
    if (!isset($_GET['s'])) {

        remove_all_filters('pre_get_posts', 500);
        remove_all_filters('posts_search', 500);

    }

}

Oliver

Great! Thanks for the update.

We’ve added support in Deny All Firewall so if you use that plugin and the firewall is enabled then .htaccess rules will be added.

Hello again, we have modified your .htaccess rules as follows which will prevent errors if the headers aren’t enabled …

# BEGIN Serve gzip compressed CSS and JS files
<IfModule mod_rewrite.c>
	<IfModule mod_deflate.c>
		<IfModule mod_headers.c>
#Serve gzip compressed CSS files if they exist and the client accepts gzip.
RewriteCond %{HTTP:Accept-encoding} gzip
RewriteCond %{REQUEST_FILENAME}\.gz -s
RewriteRule ^(.*)\.css $1\.css\.gz [QSA]
#Serve gzip compressed JS files if they exist and the client accepts gzip.
RewriteCond %{HTTP:Accept-encoding} gzip
RewriteCond %{REQUEST_FILENAME}\.gz -s
RewriteRule ^(.*)\.js $1\.js\.gz [QSA]
#Serve correct content types, and prevent mod_deflate double gzip.
RewriteRule \.css\.gz$ - [T=text/css,E=no-gzip:1,E=is_gzip:1]
RewriteRule \.js\.gz$ - [T=text/javascript,E=no-gzip:1,E=is_gzip:1]
Header set Content-Encoding "gzip" env=is_gzip
		</IfModule>
	</IfModule>
</IfModule>
# END Serve gzip compressed CSS and JS files

Feel free to include this in your readme.txt ??

Oliver

OK, it’s working. Thank you.

Looks like the browser automatically gunzips the .gz file.

Thank you.

Oliver

Good shout. The issue was we didn’t have a2enmod headers enabled. Might be worth updating readme.txt to ensure users know to have those three mods enabled for Gzip.

The page now loads but inspecting the sources, looks like the regular .css and .js files are being served? Tested in Chrome and Safari. Would you mind confirming? https://williamstewart.me

I’ve looked on the server and the corresponding .css.gz and .js.gz files all exist within /wp-content/mmr/