From 1abe655597da7b3c5b59146c4351eef59f69514c Mon Sep 17 00:00:00 2001 From: VirtualTam Date: Sat, 16 Jan 2016 15:48:26 +0100 Subject: [PATCH] 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 --- application/Utils.php | 13 +++++++++ index.php | 18 ++++-------- tests/UtilsTest.php | 66 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 13 deletions(-) diff --git a/application/Utils.php b/application/Utils.php index aeaef9f..a9a10ec 100644 --- 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 600b2f5..553f65e 100644 --- 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'], ""); @@ -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'], ""); 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 02eecda..869a969 100644 --- 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 */