Logging: move logm() from index.php to application/Utils.php

Relates to #436

Modifications:
- inject dependencies to global variables ($_SERVER, $GLOBALS)
- apply coding conventions
- add test coverage

Signed-off-by: VirtualTam <virtualtam@flibidi.net>

diff --git a/application/Utils.php b/application/Utils.php
index aeaef9ff143a84e67f16bb99365fcdbd70680608..a9a10ece572a02e36c151931c719be739894faaa 100644 (file)
--- a/application/Utils.php
+++ b/application/Utils.php
@@ -3,6 +3,19 @@
  * Shaarli utilities
  */
 
+/**
+ * Logs a message to a text file
+ *
+ * @param string $logFile  where to write the logs
+ * @param string $clientIp the client's remote IPv4/IPv6 address
+ * @param string $message  the message to log
+ */
+function logm($logFile, $clientIp, $message)
+{
+    $line = strval(date('Y/m/d_H:i:s')).' - '.$clientIp.' - '.strval($message).'\n';
+    file_put_contents($logFile, $line, FILE_APPEND);
+}
+
 /**
  *  Returns the small hash of a string, using RFC 4648 base64url format
  *

diff --git a/index.php b/index.php
index 600b2f5540242d24b81a8fb48e5c5e0574c7830a..553f65e4b1ba2b778653495b117273203c35f37f 100644 (file)
--- a/index.php
+++ b/index.php
@@ -309,14 +309,6 @@ function setup_login_state() {
 $userIsLoggedIn = setup_login_state();
 
 
-// -----------------------------------------------------------------------------------------------
-// Log to text file
-function logm($message)
-{
-    $t = strval(date('Y/m/d_H:i:s')).' - '.$_SERVER["REMOTE_ADDR"].' - '.strval($message)."\n";
-    file_put_contents($GLOBALS['config']['LOG_FILE'], $t, FILE_APPEND);
-}
-
 // ------------------------------------------------------------------------------------------
 // Sniff browser language to display dates in the right format automatically.
 // (Note that is may not work on your server if the corresponding local is not installed.)
@@ -380,10 +372,10 @@ function check_auth($login,$password)
     if ($login==$GLOBALS['login'] && $hash==$GLOBALS['hash'])
     {   // Login/password is correct.
                fillSessionInfo();
-        logm('Login successful');
+        logm($GLOBALS['config']['LOG_FILE'], $_SERVER['REMOTE_ADDR'], 'Login successful');
         return True;
     }
-    logm('Login failed for user '.$login);
+    logm($GLOBALS['config']['LOG_FILE'], $_SERVER['REMOTE_ADDR'], 'Login failed for user '.$login);
     return False;
 }
 
@@ -420,7 +412,7 @@ function ban_loginFailed()
     if ($gb['FAILURES'][$ip]>($GLOBALS['config']['BAN_AFTER']-1))
     {
         $gb['BANS'][$ip]=time()+$GLOBALS['config']['BAN_DURATION'];
-        logm('IP address banned from login');
+        logm($GLOBALS['config']['LOG_FILE'], $_SERVER['REMOTE_ADDR'], 'IP address banned from login');
     }
     $GLOBALS['IPBANS'] = $gb;
     file_put_contents($GLOBALS['config']['IPBANS_FILENAME'], "<?php\n\$GLOBALS['IPBANS']=".var_export($gb,true).";\n?>");
@@ -444,7 +436,7 @@ function ban_canLogin()
         // User is banned. Check if the ban has expired:
         if ($gb['BANS'][$ip]<=time())
         {   // Ban expired, user can try to login again.
-            logm('Ban lifted.');
+            logm($GLOBALS['config']['LOG_FILE'], $_SERVER['REMOTE_ADDR'], 'Ban lifted.');
             unset($gb['FAILURES'][$ip]); unset($gb['BANS'][$ip]);
             file_put_contents($GLOBALS['config']['IPBANS_FILENAME'], "<?php\n\$GLOBALS['IPBANS']=".var_export($gb,true).";\n?>");
             return true; // Ban has expired, user can login.
@@ -641,7 +633,7 @@ class pageBuilder
             $this->tpl->assign('versionError', '');
 
         } catch (Exception $exc) {
-            logm($exc->getMessage());
+            logm($GLOBALS['config']['LOG_FILE'], $_SERVER['REMOTE_ADDR'], $exc->getMessage());
             $this->tpl->assign('newVersion', '');
             $this->tpl->assign('versionError', escape($exc->getMessage()));
         }

diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php
index 02eecda216f8c03adafe50f57bcb32bdf7ace787..869a9695be1b2075eac0e6f0e9eadcdc170ed5b7 100644 (file)
--- a/tests/UtilsTest.php
+++ b/tests/UtilsTest.php
@@ -18,6 +18,13 @@ class UtilsTest extends PHPUnit_Framework_TestCase
     // Session ID hashes
     protected static $sidHashes = null;
 
+    // Log file
+    protected static $testLogFile = 'tests.log';
+
+    // Expected log date format
+    protected static $dateFormat = 'Y/m/d_H:i:s';
+    
+
     /**
      * Assign reference data
      */
@@ -26,6 +33,65 @@ class UtilsTest extends PHPUnit_Framework_TestCase
         self::$sidHashes = ReferenceSessionIdHashes::getHashes();
     }
 
+    /**
+     * Resets test data before each test
+     */
+    protected function setUp()
+    {
+        if (file_exists(self::$testLogFile)) {
+            unlink(self::$testLogFile);
+        }
+    }
+
+    /**
+     * Returns a list of the elements from the last logged entry
+     *
+     * @return list (date, ip address, message)
+     */
+    protected function getLastLogEntry()
+    {
+        $logFile = file(self::$testLogFile);
+        return explode(' - ', trim(array_pop($logFile), '\n'));
+    }
+
+    /**
+     * Log a message to a file - IPv4 client address
+     */
+    public function testLogmIp4()
+    {
+        $logMessage = 'IPv4 client connected';
+        logm(self::$testLogFile, '127.0.0.1', $logMessage);
+        list($date, $ip, $message) = $this->getLastLogEntry();
+
+        $this->assertInstanceOf(
+            'DateTime',
+            DateTime::createFromFormat(self::$dateFormat, $date)
+        );
+        $this->assertTrue(
+            filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4) !== false
+        );
+        $this->assertEquals($logMessage, $message);
+    }
+
+    /**
+     * Log a message to a file - IPv6 client address
+     */
+    public function testLogmIp6()
+    {
+        $logMessage = 'IPv6 client connected';
+        logm(self::$testLogFile, '2001:db8::ff00:42:8329', $logMessage);
+        list($date, $ip, $message) = $this->getLastLogEntry();
+
+        $this->assertInstanceOf(
+            'DateTime',
+            DateTime::createFromFormat(self::$dateFormat, $date)
+        );
+        $this->assertTrue(
+            filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false
+        );
+        $this->assertEquals($logMessage, $message);
+    }
+
     /**
      * Represent a link by its hash
      */