Code auditing, Technology

Exakat 1.5.4 review

Posted on by dams
Exakat 1.5.4 review
Exakat 1.5.4 review

Exakat 1.5.4 Review

Exakat 1.5.4 is an under-the-hood upgrade. On the visible side, we added a check for unreachable class constants, since class constant got their visibility option; refrain from self in interfaces, as they behave unexpectedly, and should have destructor, for long-running PHP applications. On the hidden side, many refactoring have been applied, and most will be invisible : that is often the sign of a good update. It is laying the ground to more visible features in the future, and until now, just believe us. Now, more than ever, is the time for the Exakat 1.5.4 review.

Unreachable class constants

Visibility was added to class constants in PHP 7.1. Until that version, class constant are always accessible, and, by default, bears the public visibility.

Now that class constants have visibility, it is recommended to explicitly add this option to constant unless you need to ensure backward compatibility. If not, then always add the visibility, and try to make it as strict as possible.

The downside of this approach is that private constants may end up inaccessible. This is the ground for this analysis.

  
<?php

class foo {
   private const A = 1;
}

echo foo::A;

?>

That problem eventually creeps up another part of the class structure : default values. Class constants may be used in default values, with static expression. When switching from public to private (for example), the default value can’t be used anymore.

  
<?php

class x {
    private const A = 1;
}

interface i {
    const I = x::A + 1;
}

//Cannot access private const x::A
echo x::A;

//Undefined class constant 'A'
echo i::A;

?>

This actually leads us to investigate another classic PHP pitfall. It is the next section.

Beware of self in interfaces

self, parent and static are special PHP keywords (sic) that represent respectively the current class, the parent class and the current called class.

As such, static is forbidden from being used in an interface, since it is only determined at call time. parent is also forbidden, but does lint : it won’t execute. parent will always complain that Cannot access parent:: when current class scope has no parent, even when called from a class context.

self and , on the other hand, behave differently in an interface. self represents itself, the interface, until it is implemented in a class : at that point, it represents the current class.

  
<?php

interface i {
  public const I = 1;
  // self is i
  public const I2 = self::I + 1;

  // This constant is build on IA, which is defined in class x
  public const I3 = self::IA + 1;

  // parent can be written, but can't be used
  //Cannot access parent:: when current class scope has no parent
  public const I4 = parent::I5 + 1;

  // static is just not allowed
  // "static::" is not allowed in compile-time constants
  // public const I4 = static::I5 + 1;
}

class x {
  const I3 = 4;
  const IA = 5;
}

class y extends x implements i {

}

echo y::I3;

?>

Generally speaking, just beware of any usage of self, parent and static in an interface. They may be allowed or not, but all of them will lead to confusing situations, when the code is refactored. In particular, using self to make an interface depend on another interface, or the implementing class makes a dependent interface, and is very fragile code.

Class should have destructor

__destruct is a magic method in a class, that is called upon the destruction of an object. It is the complement to __construct, which is called at object creation.

Most of the time, PHP handles the object destruction seamlessly, and efficiently. In particular, PHP unsets the scalar properties, and frees the memory.

__destruct are useful when a closing action must be taken before releasing the resources. Some situations like :

  • Remove a temporary file
  • Cleaning close an open file
  • Nicely close a connexion to a server
  • Cleaning any resource that was created beyond PHP
  • Unset object properties
  
<?php

// Can't open a SQLITE database inside a PHAR archive, as 
// sqlite requires read write access to it.
class PharSqlite($path) {
    private $path, $sqlite;
    
    function __construct($path) {
        // generate tmp file
       $this->phar_tmp = tempnam(sys_get_temp_dir(), 'sqliteInPhar').'.sqlite';
       //copy file from Phar to file sytem for read/write access
       copy($path, $this->phar_tmp);
       //open SQLITE database
       $this->sqlite = new \Sqlite3($docPath, \SQLITE3_OPEN_READONLY);
    }

    function __destruct() {
        // close sqlite first
       unset($this->sqlite);
       // remove copied file
       unlink($this->phar_tmp);
    }
  
}

?>

In the previous example, __destruct provides two important steps in the destruction process. First, the sqlite property is unset first, and then, the temporary file is removed. The other way would be wrong. And then, the external file is removed : PHP won’t take care of the phar_tmp property, as it is not an external resource.

What about objects ? Well, PHP unset objects too. The catch is that it will do it when it has time, so basically when gc_collect_cycles is called or when it is time for GC. In the case of Web applications, it will postpone those operations as long as possible : it may happen only at shutdown.

For long-running applications, this may be a memory sink. Objects accumulates, and the memory is only purged randomly. Adding a destructor leads to more predictable times of garbage collections. Also, the GC is searching hard for circular references between objects. By specifying a destructor, a lot of those cycles may be broken, leading to easily freed memory.

Exakat now reports any class that makes usage of objects as properties, and suggests that a __destruct method is written for them.

Speed up exporting results

One of the notable modifications is the exporting of results. Exakat runs the analysis in a Gremlin graph database, and eventually export the results to a SQLite database, for reporting purposes. This step has been sped up significantly by batching the exports. That way, the pause between one analysis and the next should be much shorter.

memory_limit warning

Also, we added a specific message when exakat finally runs out of memory : the application will fail and mention the lack of memory. Generally speaking, a good 2Gb of RAM is sufficient for PHP to execute Exakat and export all the audits. Yet, with large codebases, this may not be sufficient. We recommended using memory_limit = 1 whenever possible, to avoid that problem.

And in a bid to give the engine a lighter footprint, we have started the review of the code to reduce the amount of used memory, both for the PHP scripts and the graph database. More to come in the following weeks.

The Weekly Audits : 2018, Week #47

Exakat includes a ‘weekly’ report : this report is built with a selection of five analyses. This means a short audit report, with few issues to review. This is not a lot to read them, and review them in your code. Everyone in the PHP community can focus on one of the classic coding problems and fix it. Talk about the weekly audit around you : you’ll find programmers facing the same challenges.

To obtain the ‘weekly’ audit, run an audit, and request the ‘Weekly’ report.

# Init the project (skip when it is already done)  php exakat.phar init -p <yourproject> -R https://github.com/Seldaek/monolog.git -git 

# Run the project (skip when it is already done)  php exakat.phar project -p <yourproject> 

# Export the weekly project (every monday)  php exakat.phar report -p <yourproject> -format Weekly 

# Open projects/<yourproject>/weekly/index.html in your browser

Every week, you can find here 5 new analysis to review in your code. In fact, when your code is clean, you can also take a quick look at the upcoming

Weekly recommendations for PHP code review : 2018, week 2018-47

Every week, you can find here 5 new analysis to review in your code. In fact, when your code is clean, you can also take a quick look at the upcoming week with the ‘Go further’ section.

Happy PHP Code Reviews

All the 357 analyzers are presented in the docs, including the faithful Use Pathinfo : Use pathinfo() function instead of string manipulations. It is a rare bug : 6% applications could take advantage of pathinfo().

You can check all of the Exakat reports at the gallery: exakat gallery.

Download Exakat on exakat.io, install it with Docker, upgrade it with ‘exakat.phar upgrade -u’ and like us on github.