Issues in Manager.php - Fixed Issues - Inspection of "Improved code quality & security" - amarkal/amarkal-metabox - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 322b62...9c0738 )

by Askupa

created 2017-03-27 20:44 UTC

Manager.php (2 issues)

Labels

Coding Style 2

Severity

Minor 2

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php

namespace Amarkal\Metabox;

/**
 * Metabox Manager adds metaboxes to WordPress posts
 */
class Manager
{
    /**
     * @var Singleton The reference to *Singleton* instance of this class
     */
    private static $instance;
    
    /**
     * @var Array Stores all the registered metaboxes
     */
    private $metaboxes = array();
    
    /**
     * Security nonce action
     */
    const NONCE_ACTION = 'amarkal_metabox';
    
    /**
     * Returns the *Singleton* instance of this class.
     *
     * @return Singleton The *Singleton* instance.
     */
    public static function get_instance()
    {
        if( null === static::$instance ) 
        {
            static::$instance = new static();
            static::$instance->init();
        }
        return static::$instance;
    }
    
    /**
     * Add a metabox.
     * 
     * @param string $id
     * @param array $args
     * @throws \RuntimeException if the given metabox id has already been registered
     */
    public function add( $id, array $args )
    {
        if( !in_array($id, $this->metaboxes) )
        {
            $this->metaboxes[$id] = array_merge($this->default_args(), $args);
        }
        else throw new \RuntimeException("A metabox with id '$id' has already been registered.");
    }
    
    /**
     * Render a metabox.
     * 
     * @param WP_Post $post
     * @param array $args
     */
    public function render( $post, $args )
    {
        $metabox = $this->metaboxes[$args['id']];
        wp_nonce_field(self::NONCE_ACTION, $args['id'].'_nonce');
        foreach( $metabox['fields'] as $field )
        {
            $field['post_id'] = $post->ID;
            $field_template = new Field($field);
            echo $field_template->render();
        }
    }
    
    /**
     * Internally used to register metaboxes.
     */
    public function add_meta_boxes()
    {
        foreach( $this->metaboxes as $id => $args )
        {
            \add_meta_box(
                $id,
                $args['title'],
                array($this, 'render'),
                $args['screen'],
                $args['context'],
                $args['priority']
            );
        }
    }
    
    /**
     * Save metaboxes data for a given page.
     * 
     * @param number $post_id
     */
    public function save_meta_boxes( $post_id )
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        /**
         * A note on security:
         * 
         * We need to verify this came from the our screen and with proper authorization,
         * because save_post can be triggered at other times. since metaboxes can 
         * be removed - by having a nonce field in only one metabox there is no 
         * guarantee the nonce will be there. By placing a nonce field in each 
         * metabox you can check if data from that metabox has been sent 
         * (and is actually from where you think it is) prior to processing any data.
         * @see http://wordpress.stackexchange.com/a/49460/25959
         */
 
        /*
         * If this is an autosave, our form has not been submitted,
         * so we don't want to do anything.
         */
        if( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) 
        {
            return $post_id;
        }
 
        // Check the user's permissions.
        if( array_key_exists('post_type', $_POST) && 'page' == filter_input(INPUT_POST, 'post_type') ) 
        {
            if( !current_user_can('edit_page', $post_id) ) return $post_id;
        } 
        else 
        {
            if( !current_user_can('edit_post', $post_id) ) return $post_id;
        }

        // Update the meta fields.
        foreach( $this->metaboxes as $id => $metabox )
        {
            $this->save_meta_box( $post_id, $id, $metabox );
        }
    }
    
    /**
     * Save the data of a single metabox.
     * 
     * @param number $post_id
     * @param string $id
     * @param array $metabox
     */
    public function save_meta_box( $post_id, $id, $metabox )
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $nonce_name = $id.'_nonce';
        
        // Check if our nonce is set.
        if( !array_key_exists($nonce_name, $_POST) ) 
        {
            return $post_id;
        }

        $nonce = filter_input(INPUT_POST, $nonce_name);

        // Verify that the nonce is valid.
        if ( !wp_verify_nonce($nonce, self::NONCE_ACTION) ) 
        {
            return $post_id;
        }
        
        foreach( $metabox['fields'] as $field )
        {
            $data = filter_input( INPUT_POST, $field['name'] );
            \update_post_meta( $post_id, $field['name'], $data );
        }
    }
    
    /**
     * Print custom metabox style.
     */
    public function print_style() 
    {
        $cs = get_current_screen();
        
        foreach( $this->metaboxes as $metabox )
        {
            if( $metabox['screen'] === $cs->id )
            {
                echo '<style>';
                include 'metabox.css';
                echo '</style>';
                return;
            }
        }
    }
    
    /**
     * Initiate the metaboxes by adding action hooks for printing and saving.
     */
    private function init()
    {
        \add_action( 'add_meta_boxes', array( $this, 'add_meta_boxes' ) );
        \add_action( 'save_post', array( $this, 'save_meta_boxes' ) );
        \add_action( 'admin_footer', array( $this, 'print_style' ) );
    }
    
    /**
     * Default arguments for the add() method.
     * 
     * @return array
     */
    private function default_args()
    {
        return array(
            'title'    => null,
            'screen'   => null,
            'context'  => 'advanced',
            'priority' => 'default',
            'fields'   => array()
        );
    }
}

1			<?php
2
3			namespace Amarkal\Metabox;
4
5			/**
6			* Metabox Manager adds metaboxes to WordPress posts
7			*/
8			class Manager
9			{
10			/**
11			* @var Singleton The reference to Singleton instance of this class
12			*/
13			private static $instance;
14
15			/**
16			* @var Array Stores all the registered metaboxes
17			*/
18			private $metaboxes = array();
19
20			/**
21			* Security nonce action
22			*/
23			const NONCE_ACTION = 'amarkal_metabox';
24
25			/**
26			* Returns the Singleton instance of this class.
27			*
28			* @return Singleton The Singleton instance.
29			*/
30			public static function get_instance()
31			{
32			if( null === static::$instance )
33			{
34			static::$instance = new static();
35			static::$instance->init();
36			}
37			return static::$instance;
38			}
39
40			/**
41			* Add a metabox.
42			*
43			* @param string $id
44			* @param array $args
45			* @throws \RuntimeException if the given metabox id has already been registered
46			*/
47			public function add( $id, array $args )
48			{
49			if( !in_array($id, $this->metaboxes) )
50			{
51			$this->metaboxes[$id] = array_merge($this->default_args(), $args);
52			}
53			else throw new \RuntimeException("A metabox with id '$id' has already been registered.");
54			}
55
56			/**
57			* Render a metabox.
58			*
59			* @param WP_Post $post
60			* @param array $args
61			*/
62			public function render( $post, $args )
63			{
64			$metabox = $this->metaboxes[$args['id']];
65			wp_nonce_field(self::NONCE_ACTION, $args['id'].'_nonce');
66			foreach( $metabox['fields'] as $field )
67			{
68			$field['post_id'] = $post->ID;
69			$field_template = new Field($field);
70			echo $field_template->render();
71			}
72			}
73
74			/**
75			* Internally used to register metaboxes.
76			*/
77			public function add_meta_boxes()
78			{
79			foreach( $this->metaboxes as $id => $args )
80			{
81			\add_meta_box(
82			$id,
83			$args['title'],
84			array($this, 'render'),
85			$args['screen'],
86			$args['context'],
87			$args['priority']
88			);
89			}
90			}
91
92			/**
93			* Save metaboxes data for a given page.
94			*
95			* @param number $post_id
96			*/
97			public function save_meta_boxes( $post_id )
			0 ignored issues – show Coding Style introduced 2017-03-27 18:52 UTC by Report Bug Copy Issue Report Show Similar Issues like this `save_meta_boxes` uses the super-global variable `$_POST` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
98			{
99			/**
100			* A note on security:
101			*
102			* We need to verify this came from the our screen and with proper authorization,
103			* because save_post can be triggered at other times. since metaboxes can
104			* be removed - by having a nonce field in only one metabox there is no
105			* guarantee the nonce will be there. By placing a nonce field in each
106			* metabox you can check if data from that metabox has been sent
107			* (and is actually from where you think it is) prior to processing any data.
108			* @see http://wordpress.stackexchange.com/a/49460/25959
109			*/
110
111			/*
112			* If this is an autosave, our form has not been submitted,
113			* so we don't want to do anything.
114			*/
115			if( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE )
116			{
117			return $post_id;
118			}
119
120			// Check the user's permissions.
121			if( array_key_exists('post_type', $_POST) && 'page' == filter_input(INPUT_POST, 'post_type') )
122			{
123			if( !current_user_can('edit_page', $post_id) ) return $post_id;
124			}
125			else
126			{
127			if( !current_user_can('edit_post', $post_id) ) return $post_id;
128			}
129
130			// Update the meta fields.
131			foreach( $this->metaboxes as $id => $metabox )
132			{
133			$this->save_meta_box( $post_id, $id, $metabox );
134			}
135			}
136
137			/**
138			* Save the data of a single metabox.
139			*
140			* @param number $post_id
141			* @param string $id
142			* @param array $metabox
143			*/
144			public function save_meta_box( $post_id, $id, $metabox )
			0 ignored issues – show Coding Style introduced 2017-03-27 18:52 UTC by Report Bug Copy Issue Report Show Similar Issues like this `save_meta_box` uses the super-global variable `$_POST` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
145			{
146			$nonce_name = $id.'_nonce';
147
148			// Check if our nonce is set.
149			if( !array_key_exists($nonce_name, $_POST) )
150			{
151			return $post_id;
152			}
153
154			$nonce = filter_input(INPUT_POST, $nonce_name);
155
156			// Verify that the nonce is valid.
157			if ( !wp_verify_nonce($nonce, self::NONCE_ACTION) )
158			{
159			return $post_id;
160			}
161
162			foreach( $metabox['fields'] as $field )
163			{
164			$data = filter_input( INPUT_POST, $field['name'] );
165			\update_post_meta( $post_id, $field['name'], $data );
166			}
167			}
168
169			/**
170			* Print custom metabox style.
171			*/
172			public function print_style()
173			{
174			$cs = get_current_screen();
175
176			foreach( $this->metaboxes as $metabox )
177			{
178			if( $metabox['screen'] === $cs->id )
179			{
180			echo '<style>';
181			include 'metabox.css';
182			echo '</style>';
183			return;
184			}
185			}
186			}
187
188			/**
189			* Initiate the metaboxes by adding action hooks for printing and saving.
190			*/
191			private function init()
192			{
193			\add_action( 'add_meta_boxes', array( $this, 'add_meta_boxes' ) );
194			\add_action( 'save_post', array( $this, 'save_meta_boxes' ) );
195			\add_action( 'admin_footer', array( $this, 'print_style' ) );
196			}
197
198			/**
199			* Default arguments for the add() method.
200			*
201			* @return array
202			*/
203			private function default_args()
204			{
205			return array(
206			'title' => null,
207			'screen' => null,
208			'context' => 'advanced',
209			'priority' => 'default',
210			'fields' => array()
211			);
212			}
213			}

amarkal / amarkal-metabox

Push — master ( 322b62...9c0738 )

Manager.php (2 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like