Nonce_Handler::is_table_locked() - Code Metrics - Inspection of "API Nonces: Removing the "is locked" check." - Automattic/jetpack - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — fix/nonce-cleanup-locked ( 3c5ab0 )

unknown

created 2020-10-06 15:30 UTC

Nonce_Handler::is_table_locked() A

↳ Parent: Project

Complexity

Conditions	2
Paths	2

Size

Total Lines

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
cc	2
nc	2
nop	0
dl	0
loc	7
rs	10
c	0
b	0
f	0

<?php
/**
 * The nonce handler.
 *
 * @package automattic/jetpack-connection
 */

namespace Automattic\Jetpack\Connection;

/**
 * The nonce handler.
 */
class Nonce_Handler {

	/**
	 * How many nonces should be removed during each run of the runtime cleanup.
	 * Can be modified using the filter `jetpack_connection_nonce_cleanup_runtime_limit`.
	 */
	const CLEANUP_RUNTIME_LIMIT = 10;

	/**
	 * How many nonces should be removed per batch during the `clean_all()` run.
	 */
	const CLEAN_ALL_LIMIT_PER_BATCH = 1000;

	/**
	 * Nonce lifetime in seconds.
	 */
	const LIFETIME = HOUR_IN_SECONDS;

	/**
	 * The nonces used during the request are stored here to keep them valid.
	 *
	 * @var array
	 */
	private static $nonces_used_this_request = array();

	/**
	 * Adds a used nonce to a list of known nonces.
	 *
	 * @param int    $timestamp the current request timestamp.
	 * @param string $nonce the nonce value.
	 * @param bool   $run_cleanup Whether to run the `cleanup_runtime()`.
	 *
	 * @return bool whether the nonce is unique or not.
	 */
	public static function add( $timestamp, $nonce, $run_cleanup = true ) {
		global $wpdb;

		if ( isset( static::$nonces_used_this_request[ "$timestamp:$nonce" ] ) ) {
class YourClass
{
    private static $someVariable;

    public static function getSomeVariable()
    {
        return static::$someVariable;
    }
}
			return static::$nonces_used_this_request[ "$timestamp:$nonce" ];
class YourClass
{
    private static $someVariable;

    public static function getSomeVariable()
    {
        return static::$someVariable;
    }
}
		}

		// This should always have gone through Jetpack_Signature::sign_request() first to check $timestamp and $nonce.
		$timestamp = (int) $timestamp;
		$nonce     = esc_sql( $nonce );

		// Raw query so we can avoid races: add_option will also update.
		$show_errors = $wpdb->hide_errors();

		// Running try...finally to make sure.
		try {
			$old_nonce = $wpdb->get_row(
				$wpdb->prepare( "SELECT 1 FROM `$wpdb->options` WHERE option_name = %s", "jetpack_nonce_{$timestamp}_{$nonce}" )
			);

			if ( is_null( $old_nonce ) ) {
				$return = (bool) $wpdb->query(
					$wpdb->prepare(
						"INSERT INTO `$wpdb->options` (`option_name`, `option_value`, `autoload`) VALUES (%s, %s, %s)",
						"jetpack_nonce_{$timestamp}_{$nonce}",
						time(),
						'no'
					)
				);

				/**
				 * Use the filter to disable the nonce cleanup that happens at shutdown after adding a new nonce.
				 *
				 * @since 9.0.0
				 *
				 * @param int $limit How many old nonces to remove at shutdown.
				 */
				if ( apply_filters( 'jetpack_connection_add_nonce_cleanup', $run_cleanup ) ) {
					add_action( 'shutdown', array( __CLASS__, 'clean_runtime' ) );
				}
			} else {
				$return = false;
			}
		} finally {
			$wpdb->show_errors( $show_errors );
		}

		static::$nonces_used_this_request[ "$timestamp:$nonce" ] = $return;
class YourClass
{
    private static $someVariable;

    public static function getSomeVariable()
    {
        return static::$someVariable;
    }
}

		return $return;
	}

	/**
	 * Removing [almost] all the nonces.
	 * Capped at 20 seconds to avoid breaking the site.
	 *
	 * @param int $cutoff_timestamp All nonces added before this timestamp will be removed.
	 *
	 * @return true
	 */
	public static function clean_all( $cutoff_timestamp = PHP_INT_MAX ) {
		// phpcs:ignore Generic.CodeAnalysis.ForLoopWithTestFunctionCall.NotAllowed
		for ( $end_time = time() + 20; time() < $end_time; ) {
			$result = static::delete( static::CLEAN_ALL_LIMIT_PER_BATCH, $cutoff_timestamp );

			if ( ! $result ) {
0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
				break;
			}
		}

		return true;
	}

	/**
	 * Clean up the expired nonces on shutdown.
	 *
	 * @return bool True if the cleanup query has been run, false if the table is locked.
	 */
	public static function clean_runtime() {
		/**
		 * Adjust the number of old nonces that are cleaned up at shutdown.
		 *
		 * @since 9.0.0
		 *
		 * @param int $limit How many old nonces to remove at shutdown.
		 */
		$limit = apply_filters( 'jetpack_connection_nonce_cleanup_runtime_limit', static::CLEANUP_RUNTIME_LIMIT );

		static::delete( $limit, time() - static::LIFETIME );

		return true;
	}


	/**
	 * Delete the nonces.
	 *
	 * @param int      $limit How many nonces to delete.
	 * @param null|int $cutoff_timestamp All nonces added before this timestamp will be removed.
	 *
	 * @return int|false Number of removed nonces, or `false` if nothing to remove (or in case of a database error).
	 */
	public static function delete( $limit = 10, $cutoff_timestamp = null ) {
		global $wpdb;

		$ids = $wpdb->get_col(
			$wpdb->prepare(
				"SELECT option_id FROM `{$wpdb->options}`"
				. " WHERE `option_name` >= 'jetpack_nonce_' AND `option_name` < %s"
				. ' LIMIT %d',
				'jetpack_nonce_' . $cutoff_timestamp,
				$limit
			)
		);

		if ( ! is_array( $ids ) || ! count( $ids ) ) {
			// There's either nothing to remove, or there's an error and we can't proceed.
			return false;
		}

		$ids_fill = implode( ', ', array_fill( 0, count( $ids ), '%d' ) );

		// The Code Sniffer is unable to understand what's going on...
		// phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared,WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare
		return $wpdb->query( $wpdb->prepare( "DELETE FROM `{$wpdb->options}` WHERE `option_id` IN ( {$ids_fill} )", $ids ) );
	}

	/**
	 * Clean the cached nonces valid during the current request, therefore making them invalid.
	 *
	 * @return bool
	 */
	public static function invalidate_request_nonces() {
		static::$nonces_used_this_request = array();
class YourClass
{
    private static $someVariable;

    public static function getSomeVariable()
    {
        return static::$someVariable;
    }
}

		return true;
	}

}


1			<?php
2			/**
3			* The nonce handler.
4			*
5			* @package automattic/jetpack-connection
6			*/
7
8			namespace Automattic\Jetpack\Connection;
9
10			/**
11			* The nonce handler.
12			*/
13			class Nonce_Handler {
14
15			/**
16			* How many nonces should be removed during each run of the runtime cleanup.
17			* Can be modified using the filter `jetpack_connection_nonce_cleanup_runtime_limit`.
18			*/
19			const CLEANUP_RUNTIME_LIMIT = 10;
20
21			/**
22			* How many nonces should be removed per batch during the `clean_all()` run.
23			*/
24			const CLEAN_ALL_LIMIT_PER_BATCH = 1000;
25
26			/**
27			* Nonce lifetime in seconds.
28			*/
29			const LIFETIME = HOUR_IN_SECONDS;
30
31			/**
32			* The nonces used during the request are stored here to keep them valid.
33			*
34			* @var array
35			*/
36			private static $nonces_used_this_request = array();
37
38			/**
39			* Adds a used nonce to a list of known nonces.
40			*
41			* @param int $timestamp the current request timestamp.
42			* @param string $nonce the nonce value.
43			* @param bool $run_cleanup Whether to run the `cleanup_runtime()`.
44			*
45			* @return bool whether the nonce is unique or not.
46			*/
47			public static function add( $timestamp, $nonce, $run_cleanup = true ) {
48			global $wpdb;
49
50			if ( isset( static::$nonces_used_this_request[ "$timestamp:$nonce" ] ) ) {
			0 ignored issues – show Bug introduced 2020-08-31 14:00 UTC by Report Bug Copy Issue Report Since `$nonces_used_this_request` is declared private, accessing it with `static` will lead to errors in possible sub-classes; consider using `self`, or increasing the visibility of `$nonces_used_this_request` to at least protected. Let’s assume you have a class which uses late-static binding: class YourClass { private static $someVariable; public static function getSomeVariable() { return static::$someVariable; } } The code above will run fine in your PHP runtime. However, if you now create a sub-class and call the `getSomeVariable()` on that sub-class, you will receive a runtime error: class YourSubClass extends YourClass { } YourSubClass::getSomeVariable(); // Will cause an access error. In the case above, it makes sense to update `SomeClass` to use `self` instead: class SomeClass { private static $someVariable; public static function getSomeVariable() { return self::$someVariable; // self works fine with private. } } Loading history...
51			return static::$nonces_used_this_request[ "$timestamp:$nonce" ];
			0 ignored issues – show Bug introduced 2020-08-31 14:00 UTC by Report Bug Copy Issue Report Since `$nonces_used_this_request` is declared private, accessing it with `static` will lead to errors in possible sub-classes; consider using `self`, or increasing the visibility of `$nonces_used_this_request` to at least protected. Let’s assume you have a class which uses late-static binding: class YourClass { private static $someVariable; public static function getSomeVariable() { return static::$someVariable; } } The code above will run fine in your PHP runtime. However, if you now create a sub-class and call the `getSomeVariable()` on that sub-class, you will receive a runtime error: class YourSubClass extends YourClass { } YourSubClass::getSomeVariable(); // Will cause an access error. In the case above, it makes sense to update `SomeClass` to use `self` instead: class SomeClass { private static $someVariable; public static function getSomeVariable() { return self::$someVariable; // self works fine with private. } } Loading history...
52			}
53
54			// This should always have gone through Jetpack_Signature::sign_request() first to check $timestamp and $nonce.
55			$timestamp = (int) $timestamp;
56			$nonce = esc_sql( $nonce );
57
58			// Raw query so we can avoid races: add_option will also update.
59			$show_errors = $wpdb->hide_errors();
60
61			// Running try...finally to make sure.
62			try {
63			$old_nonce = $wpdb->get_row(
64			$wpdb->prepare( "SELECT 1 FROM `$wpdb->options` WHERE option_name = %s", "jetpack_nonce_{$timestamp}_{$nonce}" )
65			);
66
67			if ( is_null( $old_nonce ) ) {
68			$return = (bool) $wpdb->query(
69			$wpdb->prepare(
70			"INSERT INTO `$wpdb->options` (`option_name`, `option_value`, `autoload`) VALUES (%s, %s, %s)",
71			"jetpack_nonce_{$timestamp}_{$nonce}",
72			time(),
73			'no'
74			)
75			);
76
77			/**
78			* Use the filter to disable the nonce cleanup that happens at shutdown after adding a new nonce.
79			*
80			* @since 9.0.0
81			*
82			* @param int $limit How many old nonces to remove at shutdown.
83			*/
84			if ( apply_filters( 'jetpack_connection_add_nonce_cleanup', $run_cleanup ) ) {
85			add_action( 'shutdown', array( __CLASS__, 'clean_runtime' ) );
86			}
87			} else {
88			$return = false;
89			}
90			} finally {
91			$wpdb->show_errors( $show_errors );
92			}
93
94			static::$nonces_used_this_request[ "$timestamp:$nonce" ] = $return;
			0 ignored issues – show Bug introduced 2020-08-31 14:00 UTC by Report Bug Copy Issue Report Since `$nonces_used_this_request` is declared private, accessing it with `static` will lead to errors in possible sub-classes; consider using `self`, or increasing the visibility of `$nonces_used_this_request` to at least protected. Let’s assume you have a class which uses late-static binding: class YourClass { private static $someVariable; public static function getSomeVariable() { return static::$someVariable; } } The code above will run fine in your PHP runtime. However, if you now create a sub-class and call the `getSomeVariable()` on that sub-class, you will receive a runtime error: class YourSubClass extends YourClass { } YourSubClass::getSomeVariable(); // Will cause an access error. In the case above, it makes sense to update `SomeClass` to use `self` instead: class SomeClass { private static $someVariable; public static function getSomeVariable() { return self::$someVariable; // self works fine with private. } } Loading history... Bug introduced 2020-09-18 18:34 UTC by Report Bug Copy Issue Report The variable `$return` does not seem to be defined for all execution paths leading up to this point. If you define a variable conditionally, it can happen that it is not defined for all execution paths. Let’s take a look at an example: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } // $x is potentially undefined here. echo $x; } In the above example, the variable `$x` is defined if you pass “foo” or “bar” as argument for `$a`. However, since the `switch` statement has no default case statement, if you pass any other value, the variable `$x` would be undefined. Available Fixes Check for existence of the variable explicitly: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } if (isset($x)) { // Make sure it's always set. echo $x; } } Define a default value for the variable: function myFunction($a) { $x = ''; // Set a default which gets overridden for certain paths. switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } echo $x; } Add a value for the missing path: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; // We add support for the missing case. default: $x = ''; break; } echo $x; } Loading history...
95
96			return $return;
97			}
98
99			/**
100			* Removing [almost] all the nonces.
101			* Capped at 20 seconds to avoid breaking the site.
102			*
103			* @param int $cutoff_timestamp All nonces added before this timestamp will be removed.
104			*
105			* @return true
106			*/
107			public static function clean_all( $cutoff_timestamp = PHP_INT_MAX ) {
108			// phpcs:ignore Generic.CodeAnalysis.ForLoopWithTestFunctionCall.NotAllowed
109			for ( $end_time = time() + 20; time() < $end_time; ) {
110			$result = static::delete( static::CLEAN_ALL_LIMIT_PER_BATCH, $cutoff_timestamp );
111
112			if ( ! $result ) {
			0 ignored issues – show Bug Best Practice introduced 2020-09-07 20:19 UTC by Report Bug Copy Issue Report The expression `$result` of type `integer\|false` is loosely compared to `false`; this is ambiguous if the integer can be zero. You might want to explicitly use `=== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `integer` values, zero is a special case, in particular the following results might be unexpected: 0 == false // true 0 == null // true 123 == false // false 123 == null // false // It is often better to use strict comparison 0 === false // false 0 === null // false Loading history...
113			break;
114			}
115			}
116
117			return true;
118			}
119
120			/**
121			* Clean up the expired nonces on shutdown.
122			*
123			* @return bool True if the cleanup query has been run, false if the table is locked.
124			*/
125			public static function clean_runtime() {
126			/**
127			* Adjust the number of old nonces that are cleaned up at shutdown.
128			*
129			* @since 9.0.0
130			*
131			* @param int $limit How many old nonces to remove at shutdown.
132			*/
133			$limit = apply_filters( 'jetpack_connection_nonce_cleanup_runtime_limit', static::CLEANUP_RUNTIME_LIMIT );
134
135			static::delete( $limit, time() - static::LIFETIME );
136
137			return true;
138			}
139
140
141			/**
142			* Delete the nonces.
143			*
144			* @param int $limit How many nonces to delete.
145			* @param null\|int $cutoff_timestamp All nonces added before this timestamp will be removed.
146			*
147			* @return int\|false Number of removed nonces, or `false` if nothing to remove (or in case of a database error).
148			*/
149			public static function delete( $limit = 10, $cutoff_timestamp = null ) {
150			global $wpdb;
151
152			$ids = $wpdb->get_col(
153			$wpdb->prepare(
154			"SELECT option_id FROM `{$wpdb->options}`"
155			. " WHERE `option_name` >= 'jetpack_nonce_' AND `option_name` < %s"
156			. ' LIMIT %d',
157			'jetpack_nonce_' . $cutoff_timestamp,
158			$limit
159			)
160			);
161
162			if ( ! is_array( $ids ) \|\| ! count( $ids ) ) {
163			// There's either nothing to remove, or there's an error and we can't proceed.
164			return false;
165			}
166
167			$ids_fill = implode( ', ', array_fill( 0, count( $ids ), '%d' ) );
168
169			// The Code Sniffer is unable to understand what's going on...
170			// phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared,WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare
171			return $wpdb->query( $wpdb->prepare( "DELETE FROM `{$wpdb->options}` WHERE `option_id` IN ( {$ids_fill} )", $ids ) );
172			}
173
174			/**
175			* Clean the cached nonces valid during the current request, therefore making them invalid.
176			*
177			* @return bool
178			*/
179			public static function invalidate_request_nonces() {
180			static::$nonces_used_this_request = array();
			0 ignored issues – show Bug introduced 2020-08-31 17:07 UTC by Report Bug Copy Issue Report Since `$nonces_used_this_request` is declared private, accessing it with `static` will lead to errors in possible sub-classes; consider using `self`, or increasing the visibility of `$nonces_used_this_request` to at least protected. Let’s assume you have a class which uses late-static binding: class YourClass { private static $someVariable; public static function getSomeVariable() { return static::$someVariable; } } The code above will run fine in your PHP runtime. However, if you now create a sub-class and call the `getSomeVariable()` on that sub-class, you will receive a runtime error: class YourSubClass extends YourClass { } YourSubClass::getSomeVariable(); // Will cause an access error. In the case above, it makes sense to update `SomeClass` to use `self` instead: class SomeClass { private static $someVariable; public static function getSomeVariable() { return self::$someVariable; // self works fine with private. } } Loading history...
181
182			return true;
183			}
184
185			}
186

Automattic / jetpack

Push — fix/nonce-cleanup-locked ( 3c5ab0 )

Nonce_Handler::is_table_locked() A

Complexity

Size

Duplication

Importance

Available Fixes

Duplication Side-by-Side

Filter issues like