| Conditions | 47 |
| Paths | 1866 |
| Total Lines | 181 |
| Code Lines | 88 |
| Lines | 0 |
| Ratio | 0 % |
| Changes | 10 | ||
| Bugs | 1 | Features | 0 |
Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.
For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.
Commonly applied refactorings include:
If many parameters/temporary variables are present:
| 1 | <?php |
||
| 405 | public function render(?\XoopsDatabase $db = null) |
||
| 406 | { |
||
| 407 | // 1) Explicit injection |
||
| 408 | // 2) Legacy global |
||
| 409 | // 3) Factory (if available) |
||
| 410 | if ($db === null && isset($GLOBALS['xoopsDB']) && $GLOBALS['xoopsDB'] instanceof \XoopsDatabase) { |
||
| 411 | $db = $GLOBALS['xoopsDB']; |
||
| 412 | } |
||
| 413 | |||
| 414 | if ($db === null && class_exists('\XoopsDatabaseFactory')) { |
||
| 415 | try { |
||
| 416 | $db = \XoopsDatabaseFactory::getDatabaseConnection(); |
||
| 417 | } catch (\Throwable $e) { |
||
| 418 | throw new \RuntimeException('Database connection required to render Criteria: ' . $e->getMessage(), 0, $e); |
||
| 419 | } |
||
| 420 | } |
||
| 421 | |||
| 422 | if (!$db) { |
||
| 423 | throw new \RuntimeException('Database connection required to render Criteria'); |
||
| 424 | } |
||
| 425 | |||
| 426 | $col = (string)($this->column ?? ''); |
||
| 427 | |||
| 428 | if ($col === '') { |
||
| 429 | return ''; |
||
| 430 | } |
||
| 431 | |||
| 432 | $backtick = (strpos($col, '.') === false && strpos($col, '(') === false) ? '`' : ''; |
||
| 433 | $clause = (empty($this->prefix) ? '' : "{$this->prefix}.") . $backtick . $col . $backtick; |
||
| 434 | |||
| 435 | if (!empty($this->function)) { |
||
| 436 | $clause = sprintf($this->function, $clause); |
||
| 437 | } |
||
| 438 | |||
| 439 | $op = strtoupper((string)$this->operator); |
||
| 440 | // REMOVE THIS LINE: $valStr = (string)$this->value; |
||
| 441 | |||
| 442 | // NULL operators |
||
| 443 | if ($op === 'IS NULL' || $op === 'IS NOT NULL') { |
||
| 444 | return $clause . ' ' . $op; |
||
| 445 | } |
||
| 446 | |||
| 447 | /** |
||
| 448 | * IN / NOT IN |
||
| 449 | */ |
||
| 450 | if ($op === 'IN' || $op === 'NOT IN') { |
||
| 451 | // Modern safe path: array input |
||
| 452 | if (is_array($this->value)) { |
||
| 453 | $parts = []; |
||
| 454 | foreach ($this->value as $v) { |
||
| 455 | if (is_int($v) || (is_string($v) && preg_match('/^-?\d+$/', $v))) { |
||
| 456 | $parts[] = (string)(int)$v; |
||
| 457 | } else { |
||
| 458 | $parts[] = $db->quote((string)$v); |
||
| 459 | } |
||
| 460 | } |
||
| 461 | return $clause . ' ' . $op . ' (' . implode(',', $parts) . ')'; |
||
| 462 | } |
||
| 463 | |||
| 464 | // Legacy format: preformatted string in parentheses |
||
| 465 | $legacy = (string)$this->value; |
||
| 466 | |||
| 467 | // FIRST: strict validation of legacy syntax |
||
| 468 | if (!self::isSafeLegacyInList($legacy)) { |
||
| 469 | // Malformed → treat as a single literal safely |
||
| 470 | return $clause . ' ' . $op . ' (' . $db->quote($legacy) . ')'; |
||
| 471 | } |
||
| 472 | |||
| 473 | // If legacy logging is not enabled, just pass through |
||
| 474 | if (!self::isLegacyLogEnabled()) { |
||
| 475 | return $clause . ' ' . $op . ' ' . $legacy; |
||
| 476 | } |
||
| 477 | |||
| 478 | // Build log message |
||
| 479 | $message = sprintf( |
||
| 480 | 'Legacy Criteria IN format used for column "%s" with value "%s"', |
||
| 481 | $this->column, |
||
| 482 | $legacy |
||
| 483 | ); |
||
| 484 | |||
| 485 | // Only pay backtrace cost in debug mode |
||
| 486 | if (defined('XOOPS_DEBUG') && XOOPS_DEBUG) { |
||
| 487 | $bt = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3); |
||
| 488 | $caller = $bt[1] ?? []; |
||
| 489 | $file = $caller['file'] ?? 'unknown'; |
||
| 490 | $line = $caller['line'] ?? 0; |
||
| 491 | $message .= sprintf(' at %s:%d', $file, $line); |
||
| 492 | } |
||
| 493 | |||
| 494 | if (class_exists('XoopsLogger')) { |
||
| 495 | \XoopsLogger::getInstance() |
||
| 496 | ->addExtra('CriteriaLegacyIN', $message); |
||
| 497 | } else { |
||
| 498 | error_log($message); |
||
| 499 | } |
||
| 500 | |||
| 501 | if (defined('XOOPS_DEBUG') && XOOPS_DEBUG) { |
||
| 502 | trigger_error($message, E_USER_DEPRECATED); |
||
| 503 | } |
||
| 504 | |||
| 505 | return $clause . ' ' . $op . ' ' . $legacy; |
||
| 506 | } |
||
| 507 | |||
| 508 | // NOW it's safe to cast to string for other operators |
||
| 509 | $valStr = (string)$this->value; |
||
| 510 | |||
| 511 | // Empty value check |
||
| 512 | if (trim($valStr) === '' && !$this->allowEmptyValue) { |
||
| 513 | return ''; |
||
| 514 | } |
||
| 515 | |||
| 516 | /** |
||
| 517 | * LIKE / NOT LIKE |
||
| 518 | * - Preserves leading/trailing % as wildcards |
||
| 519 | * - Escapes inner backslashes |
||
| 520 | * - Optionally escapes inner % and _ when allowInnerWildcards is false |
||
| 521 | */ |
||
| 522 | if ($op === 'LIKE' || $op === 'NOT LIKE') { |
||
| 523 | $pattern = (string)$this->value; |
||
| 524 | |||
| 525 | // If pattern is only % signs, it's effectively "match everything" → no predicate |
||
| 526 | if ($op === 'LIKE' && $pattern !== '' && strspn($pattern, '%') === strlen($pattern)) { |
||
| 527 | return ''; |
||
| 528 | } |
||
| 529 | |||
| 530 | $len = strlen($pattern); |
||
| 531 | $lead = strspn($pattern, '%'); |
||
| 532 | $trail = strspn(strrev($pattern), '%'); |
||
| 533 | $coreLen = $len - $lead - $trail; |
||
| 534 | |||
| 535 | if ($coreLen <= 0) { |
||
| 536 | $final = $pattern; |
||
| 537 | } else { |
||
| 538 | $left = $lead > 0 ? substr($pattern, 0, $lead) : ''; |
||
| 539 | $core = substr($pattern, $lead, $coreLen); |
||
| 540 | $right = $trail > 0 ? substr($pattern, -$trail) : ''; |
||
| 541 | |||
| 542 | // Always escape backslashes in the core |
||
| 543 | $core = str_replace('\\', '\\\\', $core); |
||
| 544 | |||
| 545 | // If inner wildcards are NOT allowed, escape % and _ inside core |
||
| 546 | if (!$this->allowInnerWildcards) { |
||
| 547 | $core = strtr($core, [ |
||
| 548 | '%' => '\\%', |
||
| 549 | '_' => '\\_', |
||
| 550 | ]); |
||
| 551 | } |
||
| 552 | |||
| 553 | $final = $left . $core . $right; |
||
| 554 | } |
||
| 555 | |||
| 556 | $quoted = $db->quote($final); |
||
| 557 | return $clause . ' ' . $op . ' ' . $quoted; |
||
| 558 | } |
||
| 559 | |||
| 560 | /** |
||
| 561 | * All other operators: =, <, >, <=, >=, !=, <> |
||
| 562 | */ |
||
| 563 | |||
| 564 | // Backtick bypass for column-to-column comparisons |
||
| 565 | if (strlen($valStr) > 2 && $valStr[0] === '`' && $valStr[strlen($valStr) - 1] === '`') { |
||
| 566 | $inner = substr($valStr, 1, -1); |
||
| 567 | |||
| 568 | // Allow alphanumeric, underscore, dot, and dollar sign |
||
| 569 | // (valid in MySQL identifiers when backticked, incl. db.table) |
||
| 570 | if (preg_match('/^[a-zA-Z0-9_.$\\-]+$/', $inner)) { |
||
| 571 | $safeValue = $valStr; |
||
| 572 | } else { |
||
| 573 | // Old behavior: empty backticks on invalid identifier content |
||
| 574 | $safeValue = '``'; |
||
| 575 | } |
||
| 576 | } else { |
||
| 577 | // Regular value - keep integers numeric; quote strings |
||
| 578 | if (is_int($this->value) || (is_string($this->value) && preg_match('/^-?\d+$/', $this->value))) { |
||
| 579 | $safeValue = (string)(int)$this->value; |
||
| 580 | } else { |
||
| 581 | $safeValue = $db->quote((string)$this->value); |
||
| 582 | } |
||
| 583 | } |
||
| 584 | |||
| 585 | return $clause . ' ' . $op . ' ' . $safeValue; |
||
| 586 | } |
||
| 634 |