Issues in RecordController.php - All Issues - Inspection of "Fix current col def. Refs #52" - tabulate/tabulate - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 6cab57...287bd7 )

by Sam

created 2017-05-22 05:21 UTC

src/Controllers/RecordController.php (5 issues)

Labels

Severity

Major 5

Sam Wilson 5

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
/**
 * This file contains only a single class.
 *
 * @package Tabulate
 * @file
 */

namespace WordPress\Tabulate\Controllers;

use WordPress\Tabulate\DB\Grants;

/**
 * This controller handles viewing, saving, and deleting of individual Records.
 */
class RecordController extends ControllerBase {

	/**
	 * Get the record-editing template for the given table.
	 *
	 * @param \WordPress\Tabulate\DB\Table $table The table.
	 * @return \WordPress\Tabulate\Template
	 */
	private function get_template( $table ) {
		$template = new \WordPress\Tabulate\Template( 'record/admin.html' );
		$template->table = $table;
		$template->controller = 'record';
		return $template;
	}

	/**
	 * Show the record-editing form.
	 *
	 * @param string $args The request arguments.
	 * @return string
	 */
	public function index( $args ) {
		// Get database and table.
		$db = new \WordPress\Tabulate\DB\Database( $this->wpdb );
		$table = $db->get_table( $args['table'] );

		// Give it all to the template.
		$template = $this->get_template( $table );
<?php

function getDate($date)
{
    if ($date !== null) {
        return new DateTime($date);
    }

    return false;
}
		if ( isset( $args['ident'] ) ) {
			$template->record = $table->get_record( $args['ident'] );
			// Check permission.
			if ( ! Grants::current_user_can( Grants::UPDATE, $table->get_name() ) ) {
				$template->add_notice( 'error', 'You do not have permission to update data in this table.' );
			}
		}
		if ( ! isset( $template->record ) || false === $template->record ) {
			$template->record = $table->get_default_record();
			// Check permission.
			if ( ! Grants::current_user_can( Grants::CREATE, $table->get_name() ) ) {
				$template->add_notice( 'error', 'You do not have permission to create records in this table.' );
			}
			// Add query-string values.
			if ( isset( $args['defaults'] ) ) {
				$template->record->set_multiple( $args['defaults'] );
			}
		}
		// Don't save to non-updatable views.
		if ( ! $table->is_updatable() ) {
			$template->add_notice( 'error', "This table can not be updated." );
		}

		// Enable postboxes (for the history and related tables' boxen).
		wp_enqueue_script( 'dashboard' );

		// Return to URL.
		if ( isset( $args['return_to'] ) ) {
			$template->return_to = $args['return_to'];
		}

		return $template->render();
	}

	/**
	 * Save a record.
	 *
	 * @param string[] $args The request arguments.
	 * @return boolean
	 */
	public function save( $args ) {
		$db = new \WordPress\Tabulate\DB\Database( $this->wpdb );
		$table = $db->get_table( $args['table'] );
		if ( ! $table ) {
			// It shouldn't be possible to get here via the UI, so no message.
			return false;
		}

		// Guard against non-post requests. c.f. wp-comments-post.php.
		if ( ! isset( $_SERVER['REQUEST_METHOD'] ) || 'POST' !== $_SERVER['REQUEST_METHOD'] ) {
			header( 'Allow: POST' );
			header( 'HTTP/1.1 405 Method Not Allowed' );
			header( 'Content-Type: text/plain' );
			return false;
		}

		$record_ident = isset( $args['ident'] ) ? $args['ident'] : false;
		$this->verify_nonce( 'tabulate-record-' . $record_ident );
		$template = $this->get_template( $table );

		// Make sure we're not saving over an already-existing record.
		$pk_name = $table->get_pk_column()->get_name();
		$pk = $_POST[ $pk_name ];
		$existing = $table->get_record( $pk );
		if ( ! $record_ident && $existing ) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
			$template->add_notice( 'updated', "The record identified by '$pk' already exists." );
			$_REQUEST['return_to'] = $existing->get_url();
		} else {
			// Otherwise, create a new one.
			try {
				$data = wp_unslash( $_POST );
				$this->wpdb->query( 'BEGIN' );
				$template->record = $table->save_record( $data, $record_ident );
<?php

function getDate($date)
{
    if ($date !== null) {
        return new DateTime($date);
    }

    return false;
}
				$this->wpdb->query( 'COMMIT' );
				$template->add_notice( 'updated', 'Record saved.' );
			} catch ( \Exception $e ) {
				$template->add_notice( 'error', $e->getMessage() );
				$template->record = new \WordPress\Tabulate\DB\Record( $table, $data );
			}
		}
		// Redirect back to the edit form.
		$return_to = ( ! empty( $_REQUEST['return_to'] ) ) ? $_REQUEST['return_to'] : $template->record->get_url();
		wp_safe_redirect( $return_to );
		exit;
	}

	/**
	 * Delete (or ask for confirmation of deleting) a single record.
	 *
	 * @param string[] $args The request arguments.
	 * @return type
	 */
	public function delete( $args ) {
		$db = new \WordPress\Tabulate\DB\Database( $this->wpdb );
		$table = $db->get_table( $args['table'] );
		$record_ident = isset( $args['ident'] ) ? $args['ident'] : false;
		if ( ! $record_ident ) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
			wp_safe_redirect( $table->get_url() );
			exit;
		}

		// Ask for confirmation.
		if ( ! isset( $_POST['confirm_deletion'] ) ) {
			$template = new \WordPress\Tabulate\Template( 'record/delete.html' );
			$template->table = $table;
			$template->record = $table->get_record( $record_ident );
			return $template->render();
		}

		// Delete the record.
		check_admin_referer( 'tabulate-record-delete-' . $record_ident );
		try {
			$this->wpdb->query( 'BEGIN' );
			$table->delete_record( $record_ident );
			$this->wpdb->query( 'COMMIT' );
		} catch ( \Exception $e ) {
			$template = $this->get_template( $table );
<?php

function getDate($date)
{
    if ($date !== null) {
        return new DateTime($date);
    }

    return false;
}
			$template->record = $table->get_record( $record_ident );
			$template->add_notice( 'error', $e->getMessage() );
			return $template->render();
		}

		wp_safe_redirect( $table->get_url() );
		exit;
	}
}


1			<?php
2			/**
3			* This file contains only a single class.
4			*
5			* @package Tabulate
6			* @file
7			*/
8
9			namespace WordPress\Tabulate\Controllers;
10
11			use WordPress\Tabulate\DB\Grants;
12
13			/**
14			* This controller handles viewing, saving, and deleting of individual Records.
15			*/
16			class RecordController extends ControllerBase {
17
18			/**
19			* Get the record-editing template for the given table.
20			*
21			* @param \WordPress\Tabulate\DB\Table $table The table.
22			* @return \WordPress\Tabulate\Template
23			*/
24			private function get_template( $table ) {
25			$template = new \WordPress\Tabulate\Template( 'record/admin.html' );
26			$template->table = $table;
27			$template->controller = 'record';
28			return $template;
29			}
30
31			/**
32			* Show the record-editing form.
33			*
34			* @param string $args The request arguments.
35			* @return string
36			*/
37			public function index( $args ) {
38			// Get database and table.
39			$db = new \WordPress\Tabulate\DB\Database( $this->wpdb );
40			$table = $db->get_table( $args['table'] );
41
42			// Give it all to the template.
43			$template = $this->get_template( $table );
			0 ignored issues – show Security Bug introduced 2016-02-15 02:07 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like `$table` defined by `$db->get_table($args['table'])` on line `40` can also be of type `false`; however, `WordPress\Tabulate\Contr...troller::get_template()` does only seem to accept `object<WordPress\Tabulate\DB\Table>`, did you maybe forget to handle an error condition? This check looks for type mismatches where the missing type is `false`. This is usually indicative of an error condtion. Consider the follow example <?php function getDate($date) { if ($date !== null) { return new DateTime($date); } return false; } This function either returns a new `DateTime` object or false, if there was an error. This is a typical pattern in PHP programming to show that an error has occurred without raising an exception. The calling code should check for this returned `false` before passing on the value to another function or method that may not be able to handle a `false`. Loading history...
44			if ( isset( $args['ident'] ) ) {
45			$template->record = $table->get_record( $args['ident'] );
46			// Check permission.
47			if ( ! Grants::current_user_can( Grants::UPDATE, $table->get_name() ) ) {
48			$template->add_notice( 'error', 'You do not have permission to update data in this table.' );
49			}
50			}
51			if ( ! isset( $template->record ) \|\| false === $template->record ) {
52			$template->record = $table->get_default_record();
53			// Check permission.
54			if ( ! Grants::current_user_can( Grants::CREATE, $table->get_name() ) ) {
55			$template->add_notice( 'error', 'You do not have permission to create records in this table.' );
56			}
57			// Add query-string values.
58			if ( isset( $args['defaults'] ) ) {
59			$template->record->set_multiple( $args['defaults'] );
60			}
61			}
62			// Don't save to non-updatable views.
63			if ( ! $table->is_updatable() ) {
64			$template->add_notice( 'error', "This table can not be updated." );
65			}
66
67			// Enable postboxes (for the history and related tables' boxen).
68			wp_enqueue_script( 'dashboard' );
69
70			// Return to URL.
71			if ( isset( $args['return_to'] ) ) {
72			$template->return_to = $args['return_to'];
73			}
74
75			return $template->render();
76			}
77
78			/**
79			* Save a record.
80			*
81			* @param string[] $args The request arguments.
82			* @return boolean
83			*/
84			public function save( $args ) {
85			$db = new \WordPress\Tabulate\DB\Database( $this->wpdb );
86			$table = $db->get_table( $args['table'] );
87			if ( ! $table ) {
88			// It shouldn't be possible to get here via the UI, so no message.
89			return false;
90			}
91
92			// Guard against non-post requests. c.f. wp-comments-post.php.
93			if ( ! isset( $_SERVER['REQUEST_METHOD'] ) \|\| 'POST' !== $_SERVER['REQUEST_METHOD'] ) {
94			header( 'Allow: POST' );
95			header( 'HTTP/1.1 405 Method Not Allowed' );
96			header( 'Content-Type: text/plain' );
97			return false;
98			}
99
100			$record_ident = isset( $args['ident'] ) ? $args['ident'] : false;
101			$this->verify_nonce( 'tabulate-record-' . $record_ident );
102			$template = $this->get_template( $table );
103
104			// Make sure we're not saving over an already-existing record.
105			$pk_name = $table->get_pk_column()->get_name();
106			$pk = $_POST[ $pk_name ];
107			$existing = $table->get_record( $pk );
108			if ( ! $record_ident && $existing ) {
			0 ignored issues – show Bug Best Practice introduced 2016-02-15 02:07 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$record_ident` of type `string\|false` is loosely compared to `false`; this is ambiguous if the string can be empty. You might want to explicitly use `=== false` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
109			$template->add_notice( 'updated', "The record identified by '$pk' already exists." );
110			$_REQUEST['return_to'] = $existing->get_url();
111			} else {
112			// Otherwise, create a new one.
113			try {
114			$data = wp_unslash( $_POST );
115			$this->wpdb->query( 'BEGIN' );
116			$template->record = $table->save_record( $data, $record_ident );
			0 ignored issues – show Security Bug introduced 2016-02-15 02:07 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like `$record_ident` defined by `isset($args['ident']) ? $args['ident'] : false` on line `100` can also be of type `false`; however, `WordPress\Tabulate\DB\Table::save_record()` does only seem to accept `string\|null`, did you maybe forget to handle an error condition? This check looks for type mismatches where the missing type is `false`. This is usually indicative of an error condtion. Consider the follow example <?php function getDate($date) { if ($date !== null) { return new DateTime($date); } return false; } This function either returns a new `DateTime` object or false, if there was an error. This is a typical pattern in PHP programming to show that an error has occurred without raising an exception. The calling code should check for this returned `false` before passing on the value to another function or method that may not be able to handle a `false`. Loading history...
117			$this->wpdb->query( 'COMMIT' );
118			$template->add_notice( 'updated', 'Record saved.' );
119			} catch ( \Exception $e ) {
120			$template->add_notice( 'error', $e->getMessage() );
121			$template->record = new \WordPress\Tabulate\DB\Record( $table, $data );
122			}
123			}
124			// Redirect back to the edit form.
125			$return_to = ( ! empty( $_REQUEST['return_to'] ) ) ? $_REQUEST['return_to'] : $template->record->get_url();
126			wp_safe_redirect( $return_to );
127			exit;
128			}
129
130			/**
131			* Delete (or ask for confirmation of deleting) a single record.
132			*
133			* @param string[] $args The request arguments.
134			* @return type
135			*/
136			public function delete( $args ) {
137			$db = new \WordPress\Tabulate\DB\Database( $this->wpdb );
138			$table = $db->get_table( $args['table'] );
139			$record_ident = isset( $args['ident'] ) ? $args['ident'] : false;
140			if ( ! $record_ident ) {
			0 ignored issues – show Bug Best Practice introduced 2016-02-15 02:07 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$record_ident` of type `string\|false` is loosely compared to `false`; this is ambiguous if the string can be empty. You might want to explicitly use `=== false` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
141			wp_safe_redirect( $table->get_url() );
142			exit;
143			}
144
145			// Ask for confirmation.
146			if ( ! isset( $_POST['confirm_deletion'] ) ) {
147			$template = new \WordPress\Tabulate\Template( 'record/delete.html' );
148			$template->table = $table;
149			$template->record = $table->get_record( $record_ident );
150			return $template->render();
151			}
152
153			// Delete the record.
154			check_admin_referer( 'tabulate-record-delete-' . $record_ident );
155			try {
156			$this->wpdb->query( 'BEGIN' );
157			$table->delete_record( $record_ident );
158			$this->wpdb->query( 'COMMIT' );
159			} catch ( \Exception $e ) {
160			$template = $this->get_template( $table );
			0 ignored issues – show Security Bug introduced 2016-02-15 02:07 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like `$table` defined by `$db->get_table($args['table'])` on line `138` can also be of type `false`; however, `WordPress\Tabulate\Contr...troller::get_template()` does only seem to accept `object<WordPress\Tabulate\DB\Table>`, did you maybe forget to handle an error condition? This check looks for type mismatches where the missing type is `false`. This is usually indicative of an error condtion. Consider the follow example <?php function getDate($date) { if ($date !== null) { return new DateTime($date); } return false; } This function either returns a new `DateTime` object or false, if there was an error. This is a typical pattern in PHP programming to show that an error has occurred without raising an exception. The calling code should check for this returned `false` before passing on the value to another function or method that may not be able to handle a `false`. Loading history...
161			$template->record = $table->get_record( $record_ident );
162			$template->add_notice( 'error', $e->getMessage() );
163			return $template->render();
164			}
165
166			wp_safe_redirect( $table->get_url() );
167			exit;
168			}
169			}
170

tabulate / tabulate

Push — master ( 6cab57...287bd7 )

src/Controllers/RecordController.php (5 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like