4
\$\begingroup\$

I am in the process of setting up WordPress with WooCommerce, one of the requirements is a bit of integration that takes a person from another website directly to a specific product. I have set the external product references in tags on the products.

An example incoming query would be:

http://localhost/TestSite/search?q=9404

I have written a short plugin that does this.

I am an average programmer, but very new to PHP, WordPress and WooCommerce. Can you see any potential errors, inefficencies or, security issues?

Any insight would be gladly recieved.

    //Tag on Test Product: 98614
    //example URL:
    //http://localhost/TestSite/search?q=98614

    function TagLinker(){

        if (isset($_GET['q']))
        {
            $TagNumber = $_GET['q'];
            $params=array(
            'post_type' => 'product',
            'post_status' => 'publish',
            'tax_query' => array( array(
                                'taxonomy' => 'product_tag',
                                'field' => 'slug',
                                'terms' => $TagNumber
                            )
            ));
            $wc_query = new WP_Query($params);

            if ($wc_query -> have_posts() ) {
                $wc_query -> the_post();

                $product_id = get_the_ID($wc_query->ID);

                $url = get_permalink( $product_id );

                wp_reset_postdata();
                if ( wp_redirect( $url ) ) {exit;}
            };
        }
    }

add_action( 'init', 'TagLinker' );
?>
\$\endgroup\$
1
\$\begingroup\$

It's a good idea with frameworks to follow the method calls to see what you're actually doing, Wordpress is one of the most exploited frameworks out there and thus has fairly good security improvements. Here is the Wordpress WP_Query class you're invoking (assuming you have the latest code pulled).

It looks like the __construct calls $this->query on your $params, which calls wp_parse_args(). (If you already have the code pulled, Grep can help finding where in the code the class lives). https://github.com/WordPress/WordPress/blob/master/wp-includes/class-wp-query.php

As far as security is concerned, your primary concern would be with the user supplied data $_GET['q']. If you do not feel that the WP_Query class is doing enough to sanitize the user data you can sanitize it yourself.

In your case, it looks like you only want the user to supply a integer? If that is the case you can filter. Adjust as necessary:

if (filter_var($_GET['q'], FILTER_VALIDATE_INT)) {
   // is int
}

Additional tips: There is no reason to set $_GET['q'] to $TagNumber in your example, since you're not sanitizing it. Doing so only increases memory usage and complicates code review.

Be consistent about whether or not to clear between an if and "{". Most of the reason for clearing has to do with refactoring and applies mostly to class definitions.

There is no need to ?> close the script, it can add unintended white space.

Don't indent functions unless they are inside classes (looks to be intended for no reason).

Pick a style for naming variables, e.g. snake_case; not $TagName and $params, but $tag_name and $params

I hope this is helpful :-)

\$\endgroup\$
  • \$\begingroup\$ That is very helpful! Thank you for your comments :) \$\endgroup\$ – EvalKeneval Dec 9 '19 at 9:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.