This project does not seem to handle request data directly as such no vulnerable execution paths were found.
include
, or for example
via PHP's auto-loading mechanism.
These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more
1 | <?php |
||
2 | |||
3 | |||
4 | class CheckForMysqlPaginationIssuesBuildTask extends BuildTask |
||
5 | { |
||
6 | private static $skip_tables = []; |
||
7 | |||
8 | /** |
||
9 | * standard SS variable |
||
10 | * @var String |
||
11 | */ |
||
12 | protected $title = 'Goes through all tables and checks for bad pagination'; |
||
13 | |||
14 | /** |
||
15 | * standard SS variable |
||
16 | * @var String |
||
17 | */ |
||
18 | protected $description = "Goes through all DataObjects to check if pagination can cause data-errors."; |
||
19 | |||
20 | protected $limit = 100; |
||
21 | |||
22 | protected $step = 15; |
||
23 | |||
24 | protected $quickAndDirty = false; |
||
25 | |||
26 | protected $debug = false; |
||
27 | |||
28 | protected $testTableCustom = ''; |
||
29 | |||
30 | protected $timePerClass = []; |
||
31 | |||
32 | public function run($request) |
||
33 | { |
||
34 | |||
35 | // give us some time to run this |
||
36 | ini_set('max_execution_time', 3000); |
||
37 | $classes = ClassInfo::subclassesFor('DataObject'); |
||
38 | $array = [ |
||
39 | 'l' => 'limit', |
||
40 | 's' => 'step', |
||
41 | 'd' => 'debug', |
||
42 | 'q' => 'quickAndDirty', |
||
43 | 't' => 'testTableCustom' |
||
44 | ]; |
||
45 | foreach ($array as $getParam => $field) { |
||
46 | if (isset($_GET[$getParam])) { |
||
47 | $v = $_GET[$getParam]; |
||
48 | switch ($getParam) { |
||
49 | case 't': |
||
50 | if (in_array($v, $classes)) { |
||
51 | $this->$field = $v; |
||
52 | } |
||
53 | break; |
||
54 | default: |
||
55 | $this->$field = intval($v); |
||
56 | |||
57 | } |
||
58 | } |
||
59 | } |
||
60 | $this->flushNowQuick('<style>li {list-style: none!important;}h2.group{text-align: center;}</style>'); |
||
61 | $this->flushNow('<h3>Scroll down to bottom to see results. Output ends with <i>END</i></h3>', 'notice'); |
||
62 | $this->flushNowQuick( |
||
63 | ' |
||
64 | We run through all the summary fields for all dataobjects and select <i>limits</i> (segments) of the datalist. |
||
65 | After that we check if the same ID shows up on different segments. |
||
66 | If there are duplicates then Pagination is broken. |
||
67 | ', |
||
68 | 'notice' |
||
0 ignored issues
–
show
|
|||
69 | ); |
||
70 | $this->flushNow('<hr /><hr /><hr /><hr /><h2 class="group">SETTINGS </h2><hr /><hr /><hr /><hr />'); |
||
71 | $this->flushNow(' |
||
72 | <form method="get" action="/dev/tasks/CheckForMysqlPaginationIssuesBuildTask"> |
||
73 | <br /><br />test table:<br /><input name="t" placeholder="e.g. SiteTree" value="'.$this->testTableCustom.'" /> |
||
74 | <br /><br />limit:<br /><input name="l" placeholder="limit" value="'.$this->limit.'" /> |
||
75 | <br /><br />step:<br /><input name="s" placeholder="step" value="'.$this->step.'" /> |
||
76 | <br /><br />debug:<br /><select name="d" placeholder="debug" /><option value="0">false</option><option value="1" '.($this->debug ? 'selected="selected"' : '').'>true</option></select> |
||
77 | <br /><br />quick:<br /><select name="q" placeholder="quick" /><option value="0">false</option><option value="1" '.($this->quickAndDirty ? ' selected="selected"' : '').'>true</option></select> |
||
78 | <br /><br /><input type="submit" value="run again with variables above" /> |
||
79 | </form> |
||
80 | '); |
||
81 | $this->flushNow('<hr /><hr /><hr /><hr /><h2 class="group">CALCULATIONS </h2><hr /><hr /><hr /><hr />'); |
||
82 | // array of errors |
||
83 | $errors = []; |
||
84 | $largestTable = ''; |
||
85 | $largestTableCount = 0; |
||
86 | $skipTables = $this->Config()->get('skip_tables'); |
||
87 | // get all DataObjects and loop through them |
||
88 | |||
89 | foreach ($classes as $class) { |
||
0 ignored issues
–
show
The expression
$classes of type null|array is not guaranteed to be traversable. How about adding an additional type check?
There are different options of fixing this problem.
![]() |
|||
90 | if (in_array($class, $skipTables)) { |
||
91 | continue; |
||
92 | } |
||
93 | // skip irrelevant ones |
||
94 | if ($class !== 'DataObject') { |
||
95 | //skip test ones |
||
96 | $obj = Injector::inst()->get($class); |
||
97 | if ($obj instanceof FunctionalTest || $obj instanceof TestOnly) { |
||
98 | $this->flushNowDebug('<h2>SKIPPING: '.$class.'</h2>'); |
||
99 | continue; |
||
100 | } |
||
101 | //start the process ... |
||
102 | $this->flushNowDebug('<h2>Testing '.$class.'</h2>'); |
||
103 | |||
104 | // must exist is its own table to avoid doubling-up on tests |
||
105 | // e.g. test SiteTree and Page where Page is not its own table ... |
||
106 | if ($this->tableExists($class)) { |
||
107 | $this->timePerClass[$class] = []; |
||
108 | $this->timePerClass[$class]['start'] = microtime(true); |
||
109 | // check table size |
||
110 | $count = $class::get()->count(); |
||
111 | $checkCount = DB::query('SELECT COUNT("ID") FROM "'.$class.'"')->value(); |
||
112 | if (intval($checkCount) !== intval($count)) { |
||
113 | $this->flushNow(' |
||
114 | COUNT error! |
||
115 | '.$class.' ::get: '.$count.' rows BUT |
||
116 | DB::query(...): '.$checkCount.' rows | |
||
117 | DIFFERENCE: '.abs($count - $checkCount).'', 'deleted'); |
||
118 | } |
||
119 | if ($count > $this->step) { |
||
120 | if ($count > $largestTableCount) { |
||
121 | $largestTableCount = $count; |
||
122 | $largestTable = $class; |
||
123 | } |
||
124 | $this->flushNowQuick('<br />'.$class.': '); |
||
125 | if (! isset($errors[$class])) { |
||
126 | $errors[$class] = []; |
||
127 | } |
||
128 | // get fields ... |
||
129 | $dbFields = $obj->db(); |
||
130 | if (! is_array($dbFields)) { |
||
131 | $dbFields = []; |
||
132 | } |
||
133 | // adding base fields. |
||
134 | // we do not add ID as this should work! |
||
135 | $dbFields['ClassName'] = 'ClassName'; |
||
136 | $dbFields['Created'] = 'Created'; |
||
137 | $dbFields['LastEdited'] = 'LastEdited'; |
||
138 | |||
139 | $hasOneFields = $obj->hasOne(); |
||
140 | if (! is_array($hasOneFields)) { |
||
141 | $hasOneFields = []; |
||
142 | } |
||
143 | |||
144 | //start looping through summary fields ... |
||
145 | $summaryFields = $obj->summaryFields(); |
||
146 | foreach ($summaryFields as $field => $value) { |
||
147 | if (isset($dbFields[$field]) || isset($hasOneFields[$field])) { |
||
148 | $this->flushNowQuick(' / '.$field.': '); |
||
149 | // reset comparisonArray - this is important ... |
||
150 | $comparisonArray = []; |
||
151 | //fix has one field |
||
152 | if (isset($hasOneFields[$field])) { |
||
153 | $field .= 'ID'; |
||
154 | } |
||
155 | if (! isset($errors[$class][$field])) { |
||
156 | $errors[$class][$field] = []; |
||
157 | } |
||
158 | // start loop of limits ... |
||
159 | $this->flushNowDebug('- Sorting by '.$field); |
||
160 | for ($i = 0; $i < $this->limit && $i < ($count - $this->step); $i += $this->step) { |
||
161 | |||
162 | // OPTION 1 |
||
163 | if ($this->quickAndDirty) { |
||
164 | if (DataObject::has_own_table_database_field($class, $field)) { |
||
165 | $tempRows = DB::query('SELECT "ID" FROM "'.$class.'" ORDER BY "'.$field.'" ASC LIMIT '.$i.', '.$this->step.';'); |
||
166 | foreach ($tempRows as $row) { |
||
167 | $id = $row['ID']; |
||
168 | View Code Duplication | if (isset($comparisonArray[$id])) { |
|
0 ignored issues
–
show
This code seems to be duplicated across your project.
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. ![]() |
|||
169 | if (! isset($errors[$class][$field][$id])) { |
||
170 | $errors[$class][$field][$id] = 1; |
||
171 | } |
||
172 | $errors[$class][$field][$id]++; |
||
173 | } else { |
||
174 | $this->flushNowQuick('.'); |
||
175 | } |
||
176 | $comparisonArray[$id] = $id; |
||
177 | } |
||
178 | } else { |
||
179 | $this->flushNowDebug('<strong>SKIP: '.$class.'.'.$field.'</strong> does not exist'); |
||
180 | break; |
||
181 | } |
||
182 | |||
183 | // OPTION 2 |
||
184 | } else { |
||
185 | $tempObjects = $class::get()->sort($field)->limit($this->step, $i); |
||
186 | foreach ($tempObjects as $tempObject) { |
||
187 | $id = $tempObject->ID; |
||
188 | View Code Duplication | if (isset($comparisonArray[$id])) { |
|
0 ignored issues
–
show
This code seems to be duplicated across your project.
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. ![]() |
|||
189 | if (! isset($errors[$class][$field][$id])) { |
||
190 | $errors[$class][$field][$id] = 1; |
||
191 | } |
||
192 | $errors[$class][$field][$id]++; |
||
193 | } else { |
||
194 | $this->flushNowQuick('.'); |
||
195 | } |
||
196 | $comparisonArray[$tempObject->ID] = $tempObject->ID; |
||
197 | } |
||
198 | } |
||
199 | } |
||
200 | if (count($errors[$class][$field])) { |
||
201 | $error = '<br /><strong>Found double entries in <u>'.$class.'</u> table,'. |
||
202 | ' sorting by <u>'.$field.'</u></strong> ...'; |
||
203 | foreach ($errors[$class][$field] as $tempID => $tempCount) { |
||
204 | $error .= ' ID: '.$tempID.' occurred '.$tempCount.' times /'; |
||
205 | } |
||
206 | $this->flushNowDebug($error, 'deleted'); |
||
207 | $errors[$class][$field] = $error; |
||
208 | } |
||
209 | } else { |
||
210 | $this->flushNowDebug('<strong>SKIP: '.$class.'.'.$field.' field</strong> because it is not a DB field.'); |
||
211 | } |
||
212 | } |
||
213 | } else { |
||
214 | $this->flushNowDebug('<strong>SKIP: table '.$class.'</strong> because it does not have enough records. '); |
||
215 | } |
||
216 | $this->timePerClass[$class]['end'] = microtime(true); |
||
217 | } else { |
||
218 | $this->flushNowDebug('SKIP: '.$class.' because table does not exist. '); |
||
219 | } |
||
220 | } |
||
221 | } |
||
222 | $this->flushNow('<hr /><hr /><hr /><hr /><h2 class="group">RESULTS </h2><hr /><hr /><hr /><hr />'); |
||
223 | //print out errors again ... |
||
224 | foreach ($errors as $class => $fieldValues) { |
||
225 | $this->flushNow('<h4>'.$class.'</h4>'); |
||
226 | $time = round(($this->timePerClass[$class]['end'] - $this->timePerClass[$class]['start']) * 1000); |
||
227 | $this->flushNow('Time taken: '.$time.'μs'); |
||
228 | $errorCount = 0; |
||
229 | foreach ($fieldValues as $field => $errorMessage) { |
||
230 | if (is_string($errorMessage) && $errorMessage) { |
||
231 | $errorCount++; |
||
232 | $this->flushNow($errorMessage, 'deleted'); |
||
233 | } |
||
234 | } |
||
235 | if ($errorCount === 0) { |
||
236 | $this->flushNow('No errors', 'created'); |
||
237 | } |
||
238 | } |
||
239 | if ($this->testTableCustom) { |
||
240 | $largestTable = $this->testTableCustom; |
||
241 | } |
||
242 | $this->speedComparison($largestTable); |
||
243 | $this->flushNow('<hr /><hr /><hr /><hr /><h2 class="group">END </h2><hr /><hr /><hr /><hr />'); |
||
244 | } |
||
245 | |||
246 | protected function flushNowDebug($error, $style = '') |
||
247 | { |
||
248 | if ($this->debug) { |
||
249 | $this->flushNow($error, $style); |
||
250 | } |
||
251 | } |
||
252 | |||
253 | protected function flushNow($error, $style = '') |
||
254 | { |
||
255 | DB::alteration_message($error, $style); |
||
256 | $this->flushToBrowser(); |
||
257 | } |
||
258 | |||
259 | protected function flushNowQuick($msg) |
||
260 | { |
||
261 | echo $msg; |
||
262 | $this->flushToBrowser(); |
||
263 | } |
||
264 | |||
265 | protected function flushToBrowser() |
||
266 | { |
||
267 | // check that buffer is actually set before flushing |
||
268 | if (ob_get_length()) { |
||
269 | @ob_flush(); |
||
0 ignored issues
–
show
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.
If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of
@mkdir($dir);
// Better use
if (@mkdir($dir) === false) {
throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
![]() |
|||
270 | @flush(); |
||
0 ignored issues
–
show
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.
If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of
@mkdir($dir);
// Better use
if (@mkdir($dir) === false) {
throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
![]() |
|||
271 | @ob_end_flush(); |
||
0 ignored issues
–
show
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.
If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of
@mkdir($dir);
// Better use
if (@mkdir($dir) === false) {
throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
![]() |
|||
272 | } |
||
273 | @ob_start(); |
||
0 ignored issues
–
show
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.
If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of
@mkdir($dir);
// Better use
if (@mkdir($dir) === false) {
throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
![]() |
|||
274 | } |
||
275 | |||
276 | protected function tableExists($table) |
||
0 ignored issues
–
show
The return type could not be reliably inferred; please add a
@return annotation.
Our type inference engine in quite powerful, but sometimes the code does not
provide enough clues to go by. In these cases we request you to add a ![]() |
|||
277 | { |
||
278 | $db = DB::getConn(); |
||
0 ignored issues
–
show
The method
DB::getConn() has been deprecated with message: since version 4.0 Use DB::get_conn instead
This method has been deprecated. The supplier of the class has supplied an explanatory message. The explanatory message should give you some clue as to whether and when the method will be removed from the class and what other method or class to use instead. ![]() |
|||
279 | return $db->hasTable($table); |
||
0 ignored issues
–
show
The method
SS_Database::hasTable() has been deprecated with message: since version 4.0 Use DB::get_schema()->hasTable() instead
This method has been deprecated. The supplier of the class has supplied an explanatory message. The explanatory message should give you some clue as to whether and when the method will be removed from the class and what other method or class to use instead. ![]() |
|||
280 | } |
||
281 | |||
282 | protected function speedComparison($className) |
||
283 | { |
||
284 | $this->flushNow('<hr /><hr /><hr /><hr /><h2 class="group">SPEED COMPARISON FOR '.$className.' with '.$className::get()->count().' records</h2><hr /><hr /><hr /><hr />'); |
||
285 | $testSeq = ['A', 'B', 'C', 'C', 'B', 'A']; |
||
286 | shuffle($testSeq); |
||
287 | $this->flushNow('Test sequence: '.print_r(implode(', ', $testSeq))); |
||
288 | $testAResult = 0; |
||
289 | $testBResult = 0; |
||
290 | $testCResult = 0; |
||
291 | $isFirstRound = false; |
||
292 | foreach ($testSeq as $testIndex => $testLetter) { |
||
293 | if ($testIndex > 2) { |
||
294 | $isFirstRound = true; |
||
295 | } |
||
296 | if ($testLetter === 'A') { |
||
297 | $objects = $className::get(); |
||
298 | $testAResult += $this->runObjects($objects, $className, $isFirstRound); |
||
299 | } |
||
300 | |||
301 | if ($testLetter === 'B') { |
||
302 | $objects = $className::get()->sort(['ID' => 'ASC']); |
||
303 | $testBResult += $this->runObjects($objects, $className, $isFirstRound); |
||
304 | } |
||
305 | |||
306 | if ($testLetter === 'C') { |
||
307 | $defaultSortField = Config::inst()->get($className, 'default_sort'); |
||
308 | Config::inst()->update($className, 'default_sort', null); |
||
309 | $objects = $className::get(); |
||
310 | $testCResult += $this->runObjects($objects, $className, $isFirstRound); |
||
311 | Config::inst()->update($className, 'default_sort', $defaultSortField); |
||
312 | } |
||
313 | } |
||
314 | $testAResult = round($testAResult * 1000); |
||
315 | $testBResult = round($testBResult * 1000); |
||
316 | $testCResult = round($testCResult * 1000); |
||
317 | |||
318 | $this->flushNow('Default sort ('.print_r($defaultSortField, 1).'): '.$testAResult.'μs'); |
||
0 ignored issues
–
show
The variable
$defaultSortField 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
![]() |
|||
319 | $this->flushNow('ID sort '.$testBResult.'μs, '.(100-(round($testBResult / $testAResult, 2)*100)).'% faster than the default sort'); |
||
320 | $this->flushNow('No sort '.$testCResult.'μs, '.(100-(round($testCResult / $testAResult, 2)*100)).'% faster than the default sort'); |
||
321 | } |
||
322 | |||
323 | /** |
||
324 | * |
||
325 | * @param DataList $dataList |
||
0 ignored issues
–
show
There is no parameter named
$dataList . Was it maybe removed?
This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function. Consider the following example. The parameter /**
* @param array $germany
* @param array $island
* @param array $italy
*/
function finale($germany, $island) {
return "2:1";
}
The most likely cause is that the parameter was removed, but the annotation was not. ![]() |
|||
326 | * @param string $className |
||
327 | * @param boolean $isFirstRound |
||
328 | * |
||
329 | * @return float |
||
0 ignored issues
–
show
|
|||
330 | */ |
||
331 | protected function runObjects($objects, $className, $isFirstRound) |
||
332 | { |
||
333 | $time = 0; |
||
334 | $objects = $objects->limit(10, 20); |
||
335 | for ($i = 0; $i < 50; $i++) { |
||
336 | if ($i === 0 && $isFirstRound) { |
||
337 | $this->flushNowDebug($objects->sql()); |
||
338 | } |
||
339 | $start = microtime(true); |
||
340 | foreach ($objects as $object) { |
||
341 | $this->flushNowDebug($className.' with ID = '.$object->ID.' (not sorted)'); |
||
342 | } |
||
343 | $end = microtime(true); |
||
344 | $time += $end - $start; |
||
345 | } |
||
346 | return $time; |
||
347 | } |
||
348 | } |
||
349 |
This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue.
If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress.
In this case you can add the
@ignore
PhpDoc annotation to the duplicate definition and it will be ignored.