PHP blogging application with AJAX loading

Question

I have been working on an online newspaper/blogging application with CodeIgniter 3.1.8 and Twitter Bootstrap 4.

The latest feature added is lazy loading of posts.

From the Static_model (application\models\Static_model.php), which is responsible with the general settings, I can set $data['is_ajax_loading'] to true and enable loading posts via AJAX.

The Posts controller contains the following code:

class Posts extends CI_Controller
{
  public function __construct()
  {
    parent::__construct();
  }

  private function _initPagination($path, $totalRows, $query_string_segment = 'page')
  {
    // load and configure pagination 
    $this->load->library('pagination');
    $config['base_url']             = base_url($path);
    $config['query_string_segment'] = $query_string_segment;
    $config['enable_query_strings'] = TRUE;
    $config['reuse_query_string']   = TRUE;
    $config['total_rows']           = $totalRows;
    $config['per_page']             = 12;

    if ($this->Static_model->get_static_data()['has_pager']) {
      $config['display_pages'] = FALSE;
      $config['first_link'] = FALSE;
      $config['last_link'] = FALSE;
      $config['prev_tag_open'] = '<li class="prev">';
      $config['prev_tag_close'] = '</li>';
      $config['next_tag_open'] = '<li class="next">';
      $config['next_tag_close'] = '</li>';
    }

    if (!isset($_GET[$config['query_string_segment']]) || $_GET[$config['query_string_segment']] < 1) {
      $_GET[$config['query_string_segment']] = 1;
    }
    $this->pagination->initialize($config);

    $limit  = $config['per_page'];
    $offset = ($this->input->get($config['query_string_segment']) - 1) * $limit;

    return array(
      'limit' => $limit,
      'offset' => $offset
    );
  }

  public function index()
  {
    //call initialization method
    $config = $this->_initPagination("/", $this->Posts_model->get_num_rows());
    $data                  = $this->Static_model->get_static_data();
    $data['base_url']      = base_url("/");
    $data['pages']         = $this->Pages_model->get_pages();
    $data['categories']    = $this->Categories_model->get_categories();
    $data['search_errors'] = validation_errors();
    $data['posts'] = $this->Posts_model->get_posts($config['limit'], $config['offset']);
    $data['max_page'] = ceil($this->Posts_model->get_num_rows() / 12);

    $this->twig->addGlobal('pagination', $this->pagination->create_links());

    // Featured posts
    if ($data['is_featured']) {
      $data['featured'] = $this->Posts_model->featured_posts();
      $this->twig->addGlobal('featuredPosts', "themes/{$data['theme_directory']}/partials/hero.twig");
    }

    $this->twig->display("themes/{$data['theme_directory']}/layout", $data);
  }

  public function search()
  {
    // Force validation since the form's method is GET
    $this->form_validation->set_data($this->input->get());
    $this->form_validation->set_rules('search', 'search term', 'required|trim|min_length[3]', array(
      'min_length' => 'The search term must be at least 3 characters long.'
    ));
    $this->form_validation->set_error_delimiters('<p class = "error search-error">', '</p>');
    // If search fails
    if ($this->form_validation->run() === FALSE) {
      $data['search_errors'] = validation_errors();
      return $this->index();
    } else {
      $expression           = $this->input->get('search');
      $posts_count          = $this->Posts_model->search_count($expression);
      $query_string_segment = 'page';
      $config               = $this->_initPagination("/posts/search", $posts_count, $query_string_segment);
      $data                 = $this->Static_model->get_static_data();
      $data['base_url']     = base_url("/");
      $data['pages']        = $this->Pages_model->get_pages();
      $data['categories']   = $this->Categories_model->get_categories();
      //use limit and offset returned by _initPaginator method
      $data['posts']        = $this->Posts_model->search($expression, $config['limit'], $config['offset']);
      $data['expression']   = $expression;
      $data['posts_count']  = $posts_count;
      $data['max_page'] = ceil($posts_count / 12);
      
      $this->twig->addGlobal('pagination', $this->pagination->create_links());
      $this->twig->display("themes/{$data['theme_directory']}/layout", $data);
    }
  }

  public function byauthor($authorid)
  {
    //load and configure pagination 
    $this->load->library('pagination');
    $config['base_url']             = base_url('/posts/byauthor/' . $authorid);
    $config['query_string_segment'] = 'page';
    $config['total_rows']           = $this->Posts_model->posts_by_author_count($authorid);
    $config['per_page']             = 12;

    if ($this->Static_model->get_static_data()['has_pager']) {
      $config['display_pages'] = FALSE;
      $config['first_link'] = FALSE;
      $config['last_link'] = FALSE;
      $config['prev_tag_open'] = '<li class="prev">';
      $config['prev_tag_close'] = '</li>';
      $config['next_tag_open'] = '<li class="next">';
      $config['next_tag_close'] = '</li>';
    }

    if (!isset($_GET[$config['query_string_segment']]) || $_GET[$config['query_string_segment']] < 1) {
      $_GET[$config['query_string_segment']] = 1;
    }

    $limit  = $config['per_page'];
    $offset = ($this->input->get($config['query_string_segment']) - 1) * $limit;
    $this->pagination->initialize($config);

    $data                 = $this->Static_model->get_static_data();
    $data['base_url']     = base_url("/");
    $data['pages']        = $this->Pages_model->get_pages();
    $data['categories']   = $this->Categories_model->get_categories();
    $data['posts']        = $this->Posts_model->get_posts_by_author($authorid, $limit, $offset);
    $data['posts_count']  = $this->Posts_model->posts_by_author_count($authorid);
    $data['posts_author'] = $this->Posts_model->posts_author($authorid);
    $data['max_page'] = ceil($data['posts_count'] / $limit);
    $data['tagline']      = "Posts by " . $data['posts_author']->first_name . " " . $data['posts_author']->last_name;

    $this->twig->addGlobal('pagination', $this->pagination->create_links());
    $this->twig->display("themes/{$data['theme_directory']}/layout", $data);
  }

  public function post($slug)
  {
    $data                 = $this->Static_model->get_static_data();
    $data['base_url']     = base_url("/");
    $data['pages']        = $this->Pages_model->get_pages();
    $data['categories']   = $this->Categories_model->get_categories();
    $data['authors']      = $this->Usermodel->getAuthors();
    $data['posts']        = $this->Posts_model->sidebar_posts($limit = 5, $offset = 0);
    $data['post']         = $this->Posts_model->get_post($slug);
    $data['next_post']    = $this->Posts_model->get_next_post($slug);
    $data['prev_post']    = $this->Posts_model->get_prev_post($slug);
    $data['author_image'] = isset($data['post']->avatar) && $data['post']->avatar !== '' ? $data['post']->avatar : 'default-avatar.png';

    if ($data['categories']) {
      foreach ($data['categories'] as &$category) {
        $category->posts_count = $this->Posts_model->count_posts_in_category($category->id);
      }
    }

    if (!empty($data['post'])) {
      // Overwrite the default tagline with the post title
      $data['tagline']  = $data['post']->title;
      // Get post comments
      $post_id          = $data['post']->id;
      $data['comments'] = $this->Comments_model->get_comments($post_id);
      $this->twig->addGlobal('singlePost', "themes/{$data['theme_directory']}/templates/singlepost.twig");
    } else {
      $data['tagline'] = "Page not found";
      $this->twig->addGlobal('notFound', "themes/{$data['theme_directory']}/templates/404.twig");
    }
    $this->twig->display("themes/{$data['theme_directory']}/layout", $data);
  }
}

The script that does the loading:

(function($) {

    var currentPage = 2,
        maxPage = $('#postsContainer').data('max-page'),
        posts = null,
        pageUrl = $(location).attr('href'),
        pageBaseUrl = pageUrl.split('?')[0],
        searchStr = pageUrl.split('?')[1];

    $('.pagination').hide();

    $(window).scroll(function() {
        var toBottom = $(window).scrollTop() >= $(document).height() - $(window).height() - 25;

        if (toBottom && currentPage <= maxPage) {
            loadMore();
        }
    });

    function loadMore() {

        $.ajax({
                url: `${pageBaseUrl}?${typeof searchStr === 'string' ? searchStr : ''}&page=${currentPage}`,
                type: 'GET',
                beforeSend: function() {
                    if (typeof posts != 'undefined') {
                        $('.loader').show();
                    }
                }
            })
            .done(function(data) {
                $('.loader').hide();
                posts = $(data).find('#postsContainer').html();

                if (typeof posts != 'undefined') {

                    $('#postsContainer').append(posts);
                    currentPage = currentPage + 1;

                    if (currentPage > maxPage) {
                        $('#postsContainer').append('<p class="text-center text-muted">No more posts to load</p>');
                    }
                }
            });
    }

})(jQuery);

In the posts-lists.twig view:

{% if posts %}
    {% if is_ajax_loading %}
    <div id="postsContainer" data-max-page="{{max_page}}">
    {% endif %}
        {% for post in posts %}
            <div class="post-preview">
                <a href="{{base_url}}{{post.slug}}">
                    <h2 class="post-title">{{post.title}}</h2>
                    <h3 class="post-subtitle">{{post.description}}</h3>
                </a>
                <p class="post-meta">
                    Posted {% if category_name is null %}in <a href="{{base_url}}categories/posts/{{post.cat_id}}" title="All posts in {{post.post_category}}">{{post.post_category}}</a>,{% endif %} on {{post.created_at | date("M d, Y")}}
               </p>
            </div>
      <hr class="posts-separator">
        {% endfor %}

    {% if is_ajax_loading %}
    </div>
    {% endif %} 
{% else %}
    {% if posts_author is not defined and expression is not defined %}
        <p class="mt-0 text-muted text-center">There are no posts {% if category_name is defined %} in this category {% endif %} yet.</p>
    {% endif %}
{% endif %} 

{% if is_ajax_loading %}
    <div class="loader"><span></span></div>
{% endif %}

My main concerns are security and code redundancy.

Questions:

1. Is the code DRY enough?
2. Are there any security risks?
3. Improvement opportunities?

Answer 1

1. Is the code DRY enough?

It could be less repetitive. The following lines appear in both _initPagination() and byauthor()

if ($this->Static_model->get_static_data()['has_pager']) {
  $config['display_pages'] = FALSE;
  $config['first_link'] = FALSE;
  $config['last_link'] = FALSE;
  $config['prev_tag_open'] = '<li class="prev">';
  $config['prev_tag_close'] = '</li>';
  $config['next_tag_open'] = '<li class="next">';
  $config['next_tag_close'] = '</li>';
}
if (!isset($_GET[$config['query_string_segment']]) || $_GET[$config['query_string_segment']] < 1) {
  $_GET[$config['query_string_segment']] = 1;
}

Those could be abstracted into a separate method that can be called in both places.

Similarly the following lines appear in four places - in methods index, byauthor(), search(), post():

$data                 = $this->Static_model->get_static_data();
$data['base_url']     = base_url("/");
$data['pages']        = $this->Pages_model->get_pages();
$data['categories']   = $this->Categories_model->get_categories();

---

2. Are there any security risks?

You stated last year that you attempted to follow advice about CSRF protection and XSS filtering but the AJAX forms won't work. Were you able to resolve that? perhaps It didn't work because the AJAX forms weren't submitting the CSRF tokens. The token could be included with the AJAX data, or else the URIs could be excluded from CSRF protection (see the end of the CSRF protection in the documentation).

---

3. Improvement opportunities?

As I mentioned two years ago in my review of your Posts controller the Posts controller (still) has a constructor that merely calls the parent controller. Unless there is a good reason (e.g. provide/override specific parameters) there is not really a point in defining the overriden constructor and it can be eliminated.

Perhaps the constructor is overridden because the documentation mentions them:

Constructors are useful if you need to set some default values, or run a default process when your class is instantiated. Constructors can’t return a value, but they can do some default work.

However in the Posts constructor no default values are set and no default process is run when the class is instantiated, so the override can be removed.

---

In the _initPagination() method the single-use variables - i.e. $limit and $offset - can be eliminated:

return [
  'limit' => $config['per_page'],
  'offset' => ($this->input->get($config['query_string_segment']) - 1) * $limit
];

Note the shorter array syntax is used, presuming PHP 5.4+ is used - hopefully it is 7.3 or newer for the sake of support.

---

In the Javascript code:

pageBaseUrl = pageUrl.split('?')[0],
searchStr = pageUrl.split('?')[1];

The string is being split twice, which isn’t as bad as splitting it many times but it could be done with a single split- e.g.

const [pageBaseUrl, searchStr] = pageUrl.split('?');

Or instead of parsing it manually, the URL interface could be used - including modifying the query string with the URLSearchParams get method on the URL.searchParams property and the href property (or the toString() method).

As I mentioned in this review the AJAX method (i.e. type parameter) can be eliminated by using a shorthand method - i.e. $.get().

And the incrementing of the page number can be simplified from:

currentPage = currentPage + 1;

To

currentPage++;

In fact it can be changed to a prefix increment in the conditional:

 if (++currentPage > maxPage) {

Answer 2

I will briefly mention a few things I see in you're snippets.

1. Is the code DRY enough?

Given this line of code:

pageUrl = $(location).attr('href'),
        pageBaseUrl = pageUrl.split('?')[0],
        searchStr = pageUrl.split('?')[1];

You would be better served using the parse_url function

parse_url(string $url, int $component = -1): mixed 

$url = 'http://www.example.com/news?q=string&f=true&id=1233&sort=true';

$values = parse_url($url);

$host = explode('.',$values['host']);

echo $host[1];

SO source

as far as the templating language twig, that would be out of my familiarity.

I would also like focus on your second question because I feel it is the most important.

Are there any security risks?

There are multiple Cross-site Scripting (XSS) vulnerabilities inherently in your code because it's in the code igniter code base. Here is a list of known vulnerabilities 'CVE' with Code Igniter itself.

Definition: Cross-site Scripting

Un-sanitized input from data from a remote resource flows into append, where it is used to dynamically construct the HTML page on client side. This may result in a DOM Based Cross-Site Scripting attack (DOMXSS).

The good news and hopefully an answer to your 3rd question is that yes! there are solutions.

My first step would be to use built in code analysis tools freely available from GitHub where your repository lives.

CodeQL is the analysis engine built by GitHub to automate security checks, for developers and by security researchers to perform variant analysis.

I will only mention that one by name because you posted your github link, but there are several.

• snyk
• sonarqube
• veracode
• blackduck
• Nessus (The US Govt choice before FEDRAMP)

1. sanitize your inputs.
2. Always be aware of your dependencies and the skeletons they have in their closets.