Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like Jetpack_Signature often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use Jetpack_Signature, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 10 | class Jetpack_Signature { |
||
| 11 | public $token; |
||
| 12 | public $secret; |
||
| 13 | |||
| 14 | function __construct( $access_token, $time_diff = 0 ) { |
||
| 23 | |||
| 24 | function sign_current_request( $override = array() ) { |
||
| 25 | if ( isset( $override['scheme'] ) ) { |
||
| 26 | $scheme = $override['scheme']; |
||
| 27 | if ( !in_array( $scheme, array( 'http', 'https' ) ) ) { |
||
| 28 | return new Jetpack_Error( 'invalid_sheme', 'Invalid URL scheme' ); |
||
| 29 | } |
||
| 30 | } else { |
||
| 31 | if ( is_ssl() ) { |
||
| 32 | $scheme = 'https'; |
||
| 33 | } else { |
||
| 34 | $scheme = 'http'; |
||
| 35 | } |
||
| 36 | } |
||
| 37 | |||
| 38 | $host_port = isset( $_SERVER['HTTP_X_FORWARDED_PORT'] ) ? $_SERVER['HTTP_X_FORWARDED_PORT'] : $_SERVER['SERVER_PORT']; |
||
| 39 | |||
| 40 | if ( is_ssl() ) { |
||
| 41 | // 443: Standard Port |
||
| 42 | // 80: Assume we're behind a proxy without X-Forwarded-Port. Hardcoding "80" here means most sites |
||
| 43 | // with SSL termination proxies (self-served, Cloudflare, etc.) don't need to fiddle with |
||
| 44 | // the JETPACK_SIGNATURE__HTTPS_PORT constant. The code also implies we can't talk to a |
||
| 45 | // site at https://example.com:80/ (which would be a strange configuration). |
||
| 46 | // JETPACK_SIGNATURE__HTTPS_PORT: Set this constant in wp-config.php to the back end webserver's port |
||
| 47 | // if the site is behind a proxy running on port 443 without |
||
| 48 | // X-Forwarded-Port and the back end's port is *not* 80. It's better, |
||
| 49 | // though, to configure the proxy to send X-Forwarded-Port. |
||
| 50 | $port = in_array( $host_port, array( 443, 80, JETPACK_SIGNATURE__HTTPS_PORT ) ) ? '' : $host_port; |
||
| 51 | } else { |
||
| 52 | // 80: Standard Port |
||
| 53 | // JETPACK_SIGNATURE__HTTPS_PORT: Set this constant in wp-config.php to the back end webserver's port |
||
| 54 | // if the site is behind a proxy running on port 80 without |
||
| 55 | // X-Forwarded-Port. It's better, though, to configure the proxy to |
||
| 56 | // send X-Forwarded-Port. |
||
| 57 | $port = in_array( $host_port, array( 80, JETPACK_SIGNATURE__HTTP_PORT ) ) ? '' : $host_port; |
||
| 58 | } |
||
| 59 | |||
| 60 | $url = "{$scheme}://{$_SERVER['HTTP_HOST']}:{$port}" . stripslashes( $_SERVER['REQUEST_URI'] ); |
||
| 61 | |||
| 62 | if ( array_key_exists( 'body', $override ) && ! empty( $override['body'] ) ) { |
||
| 63 | $body = $override['body']; |
||
| 64 | } else if ( 'POST' == strtoupper( $_SERVER['REQUEST_METHOD'] ) ) { |
||
| 65 | $body = isset( $GLOBALS['HTTP_RAW_POST_DATA'] ) ? $GLOBALS['HTTP_RAW_POST_DATA'] : null; |
||
| 66 | |||
| 67 | // Convert the $_POST to the body, if the body was empty. This is how arrays are hashed |
||
| 68 | // and encoded on the Jetpack side. |
||
| 69 | if ( defined( 'IS_WPCOM' ) && IS_WPCOM ) { |
||
| 70 | if ( empty( $body ) && is_array( $_POST ) && count( $_POST ) > 0 ) { |
||
| 71 | $body = $_POST; |
||
| 72 | } |
||
| 73 | } |
||
| 74 | |||
| 75 | } else { |
||
| 76 | $body = null; |
||
| 77 | } |
||
| 78 | |||
| 79 | if ( empty( $body ) ) { |
||
| 80 | $body = null; |
||
| 81 | } |
||
| 82 | |||
| 83 | $a = array(); |
||
| 84 | foreach ( array( 'token', 'timestamp', 'nonce', 'body-hash' ) as $parameter ) { |
||
| 85 | if ( isset( $override[$parameter] ) ) { |
||
| 86 | $a[$parameter] = $override[$parameter]; |
||
| 87 | } else { |
||
| 88 | $a[$parameter] = isset( $_GET[$parameter] ) ? stripslashes( $_GET[$parameter] ) : ''; |
||
| 89 | } |
||
| 90 | } |
||
| 91 | |||
| 92 | $method = isset( $override['method'] ) ? $override['method'] : $_SERVER['REQUEST_METHOD']; |
||
| 93 | return $this->sign_request( $a['token'], $a['timestamp'], $a['nonce'], $a['body-hash'], $method, $url, $body, true ); |
||
| 94 | } |
||
| 95 | |||
| 96 | // body_hash v. body-hash is annoying. Refactor to accept an array? |
||
| 97 | function sign_request( $token = '', $timestamp = 0, $nonce = '', $body_hash = '', $method = '', $url = '', $body = null, $verify_body_hash = true ) { |
||
| 98 | if ( !$this->secret ) { |
||
| 99 | return new Jetpack_Error( 'invalid_secret', 'Invalid secret' ); |
||
| 100 | } |
||
| 101 | |||
| 102 | if ( !$this->token ) { |
||
| 103 | return new Jetpack_Error( 'invalid_token', 'Invalid token' ); |
||
| 104 | } |
||
| 105 | |||
| 106 | list( $token ) = explode( '.', $token ); |
||
| 107 | |||
| 108 | if ( 0 !== strpos( $token, "$this->token:" ) ) { |
||
| 109 | return new Jetpack_Error( 'token_mismatch', 'Incorrect token' ); |
||
| 110 | } |
||
| 111 | |||
| 112 | // If we got an array at this point, let's encode it, so we can see what it looks like as a string. |
||
| 113 | if ( is_array( $body ) ) { |
||
| 114 | if ( count( $body ) > 0 ) { |
||
| 115 | $body = json_encode( $body ); |
||
| 116 | |||
| 117 | } else { |
||
| 118 | $body = ''; |
||
| 119 | } |
||
| 120 | } |
||
| 121 | |||
| 122 | $required_parameters = array( 'token', 'timestamp', 'nonce', 'method', 'url' ); |
||
| 123 | if ( !is_null( $body ) ) { |
||
| 124 | $required_parameters[] = 'body_hash'; |
||
| 125 | if ( !is_string( $body ) ) { |
||
| 126 | return new Jetpack_Error( 'invalid_body', 'Body is malformed.' ); |
||
| 127 | } |
||
| 128 | } |
||
| 129 | |||
| 130 | foreach ( $required_parameters as $required ) { |
||
| 131 | View Code Duplication | if ( !is_scalar( $$required ) ) { |
|
| 132 | return new Jetpack_Error( 'invalid_signature', sprintf( 'The required "%s" parameter is malformed.', str_replace( '_', '-', $required ) ) ); |
||
| 133 | } |
||
| 134 | |||
| 135 | View Code Duplication | if ( !strlen( $$required ) ) { |
|
| 136 | return new Jetpack_Error( 'invalid_signature', sprintf( 'The required "%s" parameter is missing.', str_replace( '_', '-', $required ) ) ); |
||
| 137 | } |
||
| 138 | } |
||
| 139 | |||
| 140 | if ( empty( $body ) ) { |
||
| 141 | if ( $body_hash ) { |
||
| 142 | return new Jetpack_Error( 'invalid_body_hash', 'The body hash does not match.' ); |
||
| 143 | } |
||
| 144 | } else { |
||
| 145 | if ( $verify_body_hash && jetpack_sha1_base64( $body ) !== $body_hash ) { |
||
| 146 | return new Jetpack_Error( 'invalid_body_hash', 'The body hash does not match.' ); |
||
| 147 | } |
||
| 148 | } |
||
| 149 | |||
| 150 | $parsed = parse_url( $url ); |
||
| 151 | if ( !isset( $parsed['host'] ) ) { |
||
| 152 | return new Jetpack_Error( 'invalid_signature', sprintf( 'The required "%s" parameter is malformed.', 'url' ) ); |
||
| 153 | } |
||
| 154 | |||
| 155 | if ( $parsed['host'] === JETPACK__WPCOM_JSON_API_HOST ) { |
||
| 156 | $parsed['host'] = 'public-api.wordpress.com'; |
||
| 157 | } |
||
| 158 | |||
| 159 | if ( !empty( $parsed['port'] ) ) { |
||
| 160 | $port = $parsed['port']; |
||
| 161 | } else { |
||
| 162 | if ( 'http' == $parsed['scheme'] ) { |
||
| 163 | $port = 80; |
||
| 164 | } else if ( 'https' == $parsed['scheme'] ) { |
||
| 165 | $port = 443; |
||
| 166 | } else { |
||
| 167 | return new Jetpack_Error( 'unknown_scheme_port', "The scheme's port is unknown" ); |
||
| 168 | } |
||
| 169 | } |
||
| 170 | |||
| 171 | if ( !ctype_digit( "$timestamp" ) || 10 < strlen( $timestamp ) ) { // If Jetpack is around in 275 years, you can blame mdawaffe for the bug. |
||
| 172 | return new Jetpack_Error( 'invalid_signature', sprintf( 'The required "%s" parameter is malformed.', 'timestamp' ) ); |
||
| 173 | } |
||
| 174 | |||
| 175 | $local_time = $timestamp - $this->time_diff; |
||
| 176 | if ( $local_time < time() - 600 || $local_time > time() + 300 ) { |
||
| 177 | return new Jetpack_Error( 'invalid_signature', 'The timestamp is too old.' ); |
||
| 178 | } |
||
| 179 | |||
| 180 | if ( 12 < strlen( $nonce ) || preg_match( '/[^a-zA-Z0-9]/', $nonce ) ) { |
||
| 181 | return new Jetpack_Error( 'invalid_signature', sprintf( 'The required "%s" parameter is malformed.', 'nonce' ) ); |
||
| 182 | } |
||
| 183 | |||
| 184 | $normalized_request_pieces = array( |
||
| 185 | $token, |
||
| 186 | $timestamp, |
||
| 187 | $nonce, |
||
| 188 | $body_hash, |
||
| 189 | strtoupper( $method ), |
||
| 190 | strtolower( $parsed['host'] ), |
||
| 191 | $port, |
||
| 192 | $parsed['path'], |
||
| 193 | // Normalized Query String |
||
| 194 | ); |
||
| 195 | |||
| 196 | $normalized_request_pieces = array_merge( $normalized_request_pieces, $this->normalized_query_parameters( isset( $parsed['query'] ) ? $parsed['query'] : '' ) ); |
||
| 197 | |||
| 198 | $normalized_request_string = join( "\n", $normalized_request_pieces ) . "\n"; |
||
| 199 | |||
| 200 | return base64_encode( hash_hmac( 'sha1', $normalized_request_string, $this->secret, true ) ); |
||
| 201 | } |
||
| 202 | |||
| 203 | function normalized_query_parameters( $query_string ) { |
||
| 222 | |||
| 223 | function encode_3986( $string ) { |
||
| 227 | |||
| 228 | function join_with_equal_sign( $name, $value ) { |
||
| 231 | } |
||
| 232 | |||
| 233 | function jetpack_sha1_base64( $text ) { |
||
| 236 |
PHP has two types of connecting operators (logical operators, and boolean operators):
and&&or||The difference between these is the order in which they are executed. In most cases, you would want to use a boolean operator like
&&, or||.Let’s take a look at a few examples:
Logical Operators are used for Control-Flow
One case where you explicitly want to use logical operators is for control-flow such as this:
Since
dieintroduces problems of its own, f.e. it makes our code hardly testable, and prevents any kind of more sophisticated error handling; you probably do not want to use this in real-world code. Unfortunately, logical operators cannot be combined withthrowat this point:These limitations lead to logical operators rarely being of use in current PHP code.