XoopsModules25x / xfguestbook

class/Utility.php

<?php

namespace XoopsModules\Xfguestbook;

use XoopsModules\Xfguestbook;
use XoopsModules\Xfguestbook\Common;
use XoopsModules\Xfguestbook\Constants;

/**
 * Class Utility
 */
class Utility extends Common\SysUtility
{
    //--------------- Custom module methods -----------------------------

    public static function upload()
    {
        global $xoopsModule, $preview_name, $msgstop;
        /** @var Helper $helper */
        $helper = Helper::getInstance();

        $created = \time();
        $ext     = \preg_replace("/^.+\.([^.]+)$/sU", '\\1', $_FILES['photo']['name']);

The assignment to $ext is dead and can be removed.

        require_once XOOPS_ROOT_PATH . '/class/uploader.php';
        $field = $_POST['xoops_upload_file'][0];
        if (!empty($field) || '' !== $field) {
            // Check if file uploaded
            if ('' === $_FILES[$field]['tmp_name'] || !\is_readable($_FILES[$field]['tmp_name'])) {
                $msgstop .= \sprintf(MD_XFGUESTBOOK_FILEERROR, $helper->getConfig('photo_maxsize'));
            } else {
                $photos_dir              = XOOPS_UPLOAD_PATH . '/' . $xoopsModule->getVar('dirname');
                $array_allowed_mimetypes = ['image/gif', 'image/pjpeg', 'image/jpeg', 'image/x-png'];
                $uploader                = new \XoopsMediaUploader($photos_dir, $array_allowed_mimetypes, $helper->getConfig('photo_maxsize'), $helper->getConfig('photo_maxwidth'), $helper->getConfig('photo_maxheight'));
                if ($uploader->fetchMedia($field) && $uploader->upload()) {
                    if (null !== $preview_name) {
                        @\unlink("$photos_dir/" . $preview_name);

It seems like you do not handle an error condition for unlink(). This can introduce security issues, and is generally not recommended.

                    }
                    $tmp_name     = $uploader->getSavedFileName();
                    $ext          = \preg_replace("/^.+\.([^.]+)$/sU", '\\1', $tmp_name);
                    $preview_name = 'tmp_' . $created . '.' . $ext;
                    \rename("$photos_dir/$tmp_name", "$photos_dir/$preview_name");
                } else {
                    $msgstop .= $uploader->getErrors();
                }
            }
        }
    }

    /**
     * @param null $criteria

Are you sure the doc-type for parameter $criteria is correct as it would always require null to be passed?

     * @param int  $limit
     * @param int  $start
     * @return array
     */
    public static function getCountry($criteria = null, $limit = 0, $start = 0)
    {
        global $xoopsDB, $action;
        $ret = [];
        $sql = 'SELECT * FROM ' . $xoopsDB->prefix('xfguestbook_country');
        if (null !== $criteria && '' !== $criteria) {

The condition null !== $criteria is always false.

            $sql .= ' WHERE ' . $criteria;
        }
        $sql    .= ' ORDER BY country_code ASC';
        $result = $xoopsDB->query($sql, $limit, $start);
        while (false !== ($myrow = $xoopsDB->fetchArray($result))) {
            $ret[] = $myrow;
        }

        return $ret;
    }

    /**
     * @param null $criteria

Are you sure the doc-type for parameter $criteria is correct as it would always require null to be passed?

     * @param int  $limit
     * @param int  $start
     * @return array
     */
    public static function getAllCountry($criteria = null, $limit = 0, $start = 0)
    {
        global $xoopsDB, $action;
        /** @var Helper $helper */
        $helper = Helper::getInstance();

        $ret = [];
        $sql = 'SELECT country_code, country_name FROM ' . $xoopsDB->prefix('xfguestbook_country');
        if (null !== $criteria && '' !== $criteria) {

The condition null !== $criteria is always false.

            $sql .= ' WHERE ' . $criteria;
        }
        $sql    .= ' ORDER BY country_code ASC';
        $result = $xoopsDB->query($sql, $limit, $start);
        while (false !== ($myrow = $xoopsDB->fetchArray($result))) {
            //      $ret[$myrow['country_code']] = $myrow['country_name'];
            $ret[$helper->getConfig('flagdir') . '/' . $myrow['country_code']] = $myrow['country_name'];
        }

        return $ret;
    }

    /**
     * @param $user_id
     * @return bool
     */
    public static function get_user_data($user_id)
    {
        global $xoopsUser;
        /** @var Helper $helper */
        $helper = Helper::getInstance();

        if (!(int)$user_id) {
            return false;
        }

        $poster = new \XoopsUser($user_id);
        if ($poster->isActive()) {
            $xoopsUser ? $a_poster['poster'] = "<a href='../../userinfo.php?uid=$user_id'>" . $poster->uname() . '</a>' : $a_poster['poster'] = $poster->uname();

$a_poster was never initialized. Although not strictly required by PHP, it is generally a good practice to add $a_poster = array(); before regardless.

            if ($helper->getConfig('display_avatar')) {
                $rank = $poster->rank();
                $rank['title'] ? $a_poster['rank'] = $rank['title'] : $a_poster['rank'] = '';
                $rank['image'] ? $a_poster['rank_img'] = "<img src='" . XOOPS_URL . '/uploads/' . $rank['image'] . '\' alt=\'\'>' : $a_poster['rank_img'] = '';
                $poster->user_avatar() ? $a_poster['avatar'] = "<img src='" . XOOPS_URL . '/uploads/' . $poster->user_avatar() . '\' alt=\'\'>' : $a_poster['avatar'] = '';
            } else {
                $a_poster['rank']     = '';
                $a_poster['avatar']   = '';
                $a_poster['rank_img'] = '';
            }

            return $a_poster;
        }

        return false;
    }

    // Effacement fichiers temporaires

    /**
     * @param         $dir_path
     * @param string  $prefix
     * @return int
     */
    public static function clear_tmp_files($dir_path, $prefix = 'tmp_')
    {
        if (!($dir = @\opendir($dir_path))) {
            return 0;
        }
        $ret        = 0;
        $prefix_len = mb_strlen($prefix);

The assignment to $prefix_len is dead and can be removed.

        while (false !== ($file = \readdir($dir))) {
            //        if (strncmp($file, $prefix, $prefix_len) === 0) {
            if (0 === mb_strpos($file, $prefix)) {
                if (@\unlink("$dir_path/$file")) {
                    $ret++;
                }
            }
        }
        \closedir($dir);

        return $ret;
    }

    // IP bannies (modérés automatiquement)

    /**
     * @param null $all

Are you sure the doc-type for parameter $all is correct as it would always require null to be passed?

     * @return array
     */
    public static function get_badips($all = null)
    {
        global $xoopsDB;
        $ret    = [];
        $sql    = 'SELECT * FROM ' . $xoopsDB->prefix('xfguestbook_badips');
        $result = $xoopsDB->query($sql);
        if ($all) {

$all is of type null, thus it always evaluated to false.

            while (false !== ($myrow = $xoopsDB->fetchArray($result))) {
                $ret[] = $myrow;
            }
        } else {
            while (list($ip_id, $ip_value) = $xoopsDB->fetchRow($result)) {
                $ret[] = $ip_value;
            }
        }

        return $ret;
    }

    /**
     * @param $email
     * @return bool
     */
    public static function email_exist($email)
    {
        if (!\filter_var($email, \FILTER_VALIDATE_EMAIL)) {
            return false;
        }

        if (!\checkdnsrr(\array_pop(\explode('@', $email)), 'MX')) {

explode('@', $email) cannot be passed to array_pop() as the parameter $array expects a reference.

            return false;
        }

        return true;
    }
}