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 PhpassPassword 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 PhpassPassword, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
16 | class PhpassPassword implements IPassword { |
||
17 | const HASH_PHPASS = 0x00; |
||
18 | const HASH_BLOWFISH = 0x01; |
||
19 | const HASH_EXTDES = 0x02; |
||
20 | const HASH_BEST = 0x03; |
||
21 | |||
22 | protected $itoa64; |
||
23 | protected $iteration_count_log2; |
||
24 | protected $portable; |
||
25 | protected $random_state; |
||
26 | |||
27 | protected $hashMethod; |
||
28 | |||
29 | /** |
||
30 | * Initializes an instance of the of the {@link PhpPass} class. |
||
31 | * |
||
32 | * @param int $hashMethod The hash method to use when hashing passwords. |
||
33 | * @param int $iteration_count_log2 The number of times to iterate when generating the passwords. |
||
34 | */ |
||
35 | 6 | public function __construct($hashMethod = PhpassPassword::HASH_PHPASS, $iteration_count_log2 = 8) { |
|
36 | 6 | $this->itoa64 = './0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz'; |
|
37 | |||
38 | 6 | if ($iteration_count_log2 < 4 || $iteration_count_log2 > 31) { |
|
39 | 4 | $iteration_count_log2 = 8; |
|
40 | } |
||
41 | 6 | $this->iteration_count_log2 = $iteration_count_log2; |
|
42 | 6 | $this->hashMethod = $hashMethod; |
|
43 | |||
44 | 6 | $this->random_state = microtime(); |
|
45 | 6 | if (function_exists('getmypid')) { |
|
46 | 6 | $this->random_state .= getmypid(); |
|
47 | } |
||
48 | 6 | } |
|
49 | |||
50 | /** |
||
51 | * {@inheritdoc} |
||
52 | */ |
||
53 | 1 | public function needsRehash($hash) { |
|
59 | |||
60 | /** |
||
61 | * {@inheritdoc} |
||
62 | */ |
||
63 | 8 | public function hash($password) { |
|
64 | 8 | $random = ''; |
|
65 | |||
66 | 8 | View Code Duplication | if (CRYPT_BLOWFISH == 1 && ($this->hashMethod & self::HASH_BLOWFISH)) { |
1 ignored issue
–
show
|
|||
67 | 3 | $random = $this->getRandomBytes(16); |
|
68 | $hash = |
||
69 | 3 | crypt($password, $this->gensaltBlowfish($random)); |
|
70 | 3 | if (strlen($hash) == 60) { |
|
71 | 3 | return $hash; |
|
72 | } |
||
73 | } |
||
74 | |||
75 | 6 | View Code Duplication | if (CRYPT_EXT_DES == 1 && ($this->hashMethod & self::HASH_EXTDES)) { |
1 ignored issue
–
show
|
|||
76 | 2 | if (strlen($random) < 3) { |
|
77 | 2 | $random = $this->getRandomBytes(3); |
|
78 | } |
||
79 | $hash = |
||
80 | 2 | crypt($password, $this->gensaltExtended($random)); |
|
81 | 2 | if (strlen($hash) == 20) { |
|
82 | 2 | return $hash; |
|
83 | } |
||
84 | } |
||
85 | |||
86 | 5 | if (strlen($random) < 6) { |
|
87 | 5 | $random = $this->getRandomBytes(6); |
|
88 | } |
||
89 | 5 | $hash = $this->cryptPrivate( |
|
90 | $password, |
||
91 | 5 | $this->gensaltPrivate($random) |
|
92 | ); |
||
93 | 5 | if (strlen($hash) == 34) { |
|
94 | 5 | return $hash; |
|
95 | } |
||
96 | |||
97 | # Returning '*' on error is safe here, but would _not_ be safe |
||
98 | # in a crypt(3)-like function used _both_ for generating new |
||
99 | # hashes and for validating passwords against existing hashes. |
||
100 | return '*'; |
||
101 | } |
||
102 | |||
103 | /** |
||
104 | * Get a string of random bytes. |
||
105 | * |
||
106 | * @param int $count The number of bytes to get. |
||
107 | * @return string Returns a string of the generated random bytes. |
||
108 | */ |
||
109 | 8 | protected function getRandomBytes($count) { |
|
110 | 8 | $output = ''; |
|
111 | 8 | if (is_readable('/dev/urandom') && |
|
112 | 8 | ($fh = @fopen('/dev/urandom', 'rb')) |
|
113 | ) { |
||
114 | 8 | $output = fread($fh, $count); |
|
115 | 8 | fclose($fh); |
|
116 | } |
||
117 | |||
118 | 8 | if (strlen($output) < $count) { |
|
119 | $output = ''; |
||
120 | for ($i = 0; $i < $count; $i += 16) { |
||
121 | $this->random_state = |
||
122 | md5(microtime().$this->random_state); |
||
123 | $output .= |
||
124 | pack('H*', md5($this->random_state)); |
||
125 | } |
||
126 | $output = substr($output, 0, $count); |
||
127 | } |
||
128 | |||
129 | 8 | return $output; |
|
130 | } |
||
131 | |||
132 | /** |
||
133 | * Generate a password salt appropriate for blowfish. |
||
134 | * |
||
135 | * @param string $input The random input to generate the salt from. |
||
136 | * @return string The generated salt. |
||
137 | */ |
||
138 | 3 | protected function gensaltBlowfish($input) { |
|
177 | |||
178 | /** |
||
179 | * Generate a password salt based on the input. |
||
180 | * |
||
181 | * @param string $input The string to generate the salt from. |
||
182 | * @return string The generated salt. |
||
183 | */ |
||
184 | 2 | private function gensaltExtended($input) { |
|
200 | |||
201 | /** |
||
202 | * A custom base64 encoding function. |
||
203 | * |
||
204 | * @param string $input The string to encode. |
||
205 | * @param int $count The number of characters to encode. |
||
206 | * @return string Returns the encoded string. |
||
207 | */ |
||
208 | 6 | protected function encode64($input, $count) { |
|
209 | 6 | $output = ''; |
|
210 | 6 | $i = 0; |
|
211 | do { |
||
212 | 6 | $value = ord($input[$i++]); |
|
213 | 6 | $output .= $this->itoa64[$value & 0x3f]; |
|
214 | 6 | if ($i < $count) { |
|
215 | 6 | $value |= ord($input[$i]) << 8; |
|
216 | } |
||
217 | 6 | $output .= $this->itoa64[($value >> 6) & 0x3f]; |
|
218 | 6 | if ($i++ >= $count) { |
|
219 | 5 | break; |
|
220 | } |
||
221 | 6 | if ($i < $count) { |
|
222 | 6 | $value |= ord($input[$i]) << 16; |
|
223 | } |
||
224 | 6 | $output .= $this->itoa64[($value >> 12) & 0x3f]; |
|
225 | 6 | if ($i++ >= $count) { |
|
226 | break; |
||
227 | } |
||
228 | 6 | $output .= $this->itoa64[($value >> 18) & 0x3f]; |
|
229 | 6 | } while ($i < $count); |
|
230 | |||
231 | 6 | return $output; |
|
232 | } |
||
233 | |||
234 | /** |
||
235 | * A portable version of a crypt-like algorithm. |
||
236 | * |
||
237 | * @param string $password The plaintext password to encrypt. |
||
238 | * @param string $setting The hash prefix that defines what kind of algorithm to use. |
||
239 | * @return string Returns the encrypted string. |
||
240 | */ |
||
241 | 10 | private function cryptPrivate($password, $setting) { |
|
242 | 10 | $output = '*0'; |
|
243 | 10 | if (substr($setting, 0, 2) == $output) { |
|
244 | $output = '*1'; |
||
245 | } |
||
246 | |||
247 | 10 | $id = substr($setting, 0, 3); |
|
248 | # We use "$P$", phpBB3 uses "$H$" for the same thing |
||
249 | 10 | if ($id != '$P$' && $id != '$H$') { |
|
250 | 6 | return $output; |
|
251 | } |
||
252 | |||
253 | 5 | $count_log2 = strpos($this->itoa64, $setting[3]); |
|
254 | 5 | if ($count_log2 < 7 || $count_log2 > 30) { |
|
255 | return $output; |
||
256 | } |
||
257 | |||
258 | 5 | $count = 1 << $count_log2; |
|
259 | |||
260 | 5 | $salt = substr($setting, 4, 8); |
|
261 | 5 | if (strlen($salt) != 8) { |
|
262 | return $output; |
||
263 | } |
||
264 | |||
265 | # We're kind of forced to use MD5 here since it's the only |
||
266 | # cryptographic primitive available in all versions of PHP |
||
267 | # currently in use. To implement our own low-level crypto |
||
268 | # in PHP would result in much worse performance and |
||
269 | # consequently in lower iteration counts and hashes that are |
||
270 | # quicker to crack (by non-PHP code). |
||
271 | 5 | View Code Duplication | if (PHP_VERSION >= '5') { |
1 ignored issue
–
show
|
|||
272 | 5 | $hash = md5($salt.$password, true); |
|
273 | do { |
||
274 | 5 | $hash = md5($hash.$password, false); |
|
275 | 5 | } while (--$count); |
|
276 | } else { |
||
277 | $hash = pack('H*', md5($salt.$password)); |
||
278 | do { |
||
279 | $hash = pack('H*', md5($hash.$password)); |
||
280 | } while (--$count); |
||
281 | } |
||
282 | |||
283 | 5 | $output = substr($setting, 0, 12); |
|
284 | 5 | $output .= $this->encode64($hash, 16); |
|
285 | |||
286 | 5 | return $output; |
|
287 | } |
||
288 | |||
289 | /** |
||
290 | * Generate a password salt based on the given input string. |
||
291 | * |
||
292 | * @param string $input The input string to generate the salt from. |
||
293 | * @return string Returns the password salt prefixed with `$P$`. |
||
294 | */ |
||
295 | 5 | private function gensaltPrivate($input) { |
|
302 | |||
303 | /** |
||
304 | * {@inheritdoc} |
||
305 | */ |
||
306 | 10 | public function verify($password, $hash) { |
|
307 | 10 | if (!$hash) { |
|
308 | 1 | return false; |
|
309 | } |
||
310 | 9 | $calc_hash = $this->cryptPrivate($password, $hash); |
|
311 | 9 | if ($calc_hash[0] === '*') { |
|
312 | 6 | $calc_hash = crypt($password, $hash); |
|
313 | } |
||
314 | |||
315 | 9 | return $calc_hash === $hash; |
|
316 | } |
||
317 | |||
318 | /** |
||
319 | * Get the current hash method. |
||
320 | * |
||
321 | * @return int Returns the current hash method. |
||
322 | */ |
||
323 | public function getHashMethod() { |
||
326 | |||
327 | /** |
||
328 | * Set the current hash method. |
||
329 | * |
||
330 | * @param int $hashMethod The new hash mathod. |
||
331 | * @return PhpassPassword Returns $this for fluent calls. |
||
332 | */ |
||
333 | 1 | public function setHashMethod($hashMethod) { |
|
337 | } |
||
338 |
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.