diff --git a/docs/api/app.md b/docs/api/app.md index 448cf5f..ca82526 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -362,6 +362,62 @@ $app = new FrameworkX\App($container); // … ``` +If you do not want to log to the console, you can configure an absolute log file +path by passing an argument to the [`AccessLogHandler`](middleware.md#accessloghandler) +like this: + +=== "Using DI container" + + ```php title="public/index.php" + __DIR__ . '/../logs/access.log', + FrameworkX\AccessLogHandler::class => fn(string $accesslog) => new FrameworkX\AccessLogHandler($accesslog) + ]); + + $app = new FrameworkX\App($container); + + // … + ``` + +=== "Using middleware instances" + + ```php title="public/index.php" + DIRECTORY_SEPARATOR !== '\\' ? '/dev/null' : __DIR__ . '\\nul' + FrameworkX\AccessLogHandler::class => fn(string $accesslog) => new FrameworkX\AccessLogHandler($accesslog), +]); + +$app = new FrameworkX\App($container); + +// … +``` + X supports running behind reverse proxies just fine. However, by default it will see the IP address of the last proxy server as the client IP address (this will often be `127.0.0.1`). You can get the original client IP address if you configure @@ -385,8 +441,6 @@ it to the [`AccessLogHandler`](middleware.md#accessloghandler) like this: new FrameworkX\ErrorHandler() ); - $app = new FrameworkX\App($container); - // … ``` @@ -405,8 +459,6 @@ it to the [`AccessLogHandler`](middleware.md#accessloghandler) like this: FrameworkX\ErrorHandler::class ); - $app = new FrameworkX\App($container); - // … ``` diff --git a/src/AccessLogHandler.php b/src/AccessLogHandler.php index d62016f..7a5bbee 100644 --- a/src/AccessLogHandler.php +++ b/src/AccessLogHandler.php @@ -20,11 +20,18 @@ class AccessLogHandler /** @var bool */ private $hasHighResolution; - /** @throws void */ - public function __construct() + /** + * @param ?string $path (optional) absolute log file path or will log to console output by default + * @throws \InvalidArgumentException if given `$path` is not an absolute file path + * @throws \RuntimeException if given `$path` can not be opened in append mode + */ + public function __construct(?string $path = null) { - /** @throws void because `fopen()` is known to always return a `resource` for built-in wrappers */ - $this->logger = new LogStreamHandler(\PHP_SAPI === 'cli' ? 'php://output' : 'php://stderr'); + if ($path === null) { + $path = \PHP_SAPI === 'cli' ? 'php://output' : 'php://stderr'; + } + + $this->logger = new LogStreamHandler($path); $this->hasHighResolution = \function_exists('hrtime'); // PHP 7.3+ } diff --git a/src/Io/LogStreamHandler.php b/src/Io/LogStreamHandler.php index 5ef078b..39ccb93 100644 --- a/src/Io/LogStreamHandler.php +++ b/src/Io/LogStreamHandler.php @@ -10,9 +10,19 @@ class LogStreamHandler /** @var resource */ private $stream; - /** @throws \RuntimeException if given `$path` can not be opened in append mode */ + /** + * @param string $path absolute log file path + * @throws \InvalidArgumentException if given `$path` is not an absolute file path + * @throws \RuntimeException if given `$path` can not be opened in append mode + */ public function __construct(string $path) { + if (\strpos($path, "\0") !== false || (\stripos($path, 'php://') !== 0 && !$this->isAbsolutePath($path))) { + throw new \InvalidArgumentException( + 'Unable to open log file "' . \addslashes($path) . '": Invalid path given' + ); + } + $errstr = ''; \set_error_handler(function (int $_, string $error) use (&$errstr): bool { // Match errstr from PHP's warning message. @@ -42,4 +52,9 @@ public function log(string $message): void $ret = \fwrite($this->stream, $prefix . $message . \PHP_EOL); assert(\is_int($ret)); } + + private function isAbsolutePath(string $path): bool + { + return \DIRECTORY_SEPARATOR !== '\\' ? \substr($path, 0, 1) === '/' : (bool) \preg_match('#^[A-Z]:[/\\\\]#i', $path); + } } diff --git a/tests/AccessLogHandlerTest.php b/tests/AccessLogHandlerTest.php index 338486e..07563fb 100644 --- a/tests/AccessLogHandlerTest.php +++ b/tests/AccessLogHandlerTest.php @@ -12,6 +12,105 @@ class AccessLogHandlerTest extends TestCase { + public function testCtorWithRelativePathThrows(): void + { + $this->expectException(\InvalidArgumentException::class); + new AccessLogHandler('../access.log'); + } + + public function testCtorWithPathToDirectoryThrows(): void + { + $this->expectException(\RuntimeException::class); + new AccessLogHandler(__DIR__); + } + + public function testCtorWithPathToNewFileWillCreateNewFile(): void + { + $path = tempnam(sys_get_temp_dir(), 'log'); + assert(is_string($path)); + unlink($path); + + new AccessLogHandler($path); + + $this->assertFileExists($path); + unlink($path); + } + + public function testInvokeWithDefaultPathWillLogMessageToConsole(): void + { + $handler = new AccessLogHandler(); + + $request = new ServerRequest('GET', 'http://localhost:8080/users', [], '', '1.1', ['REMOTE_ADDR' => '127.0.0.1']); + $response = new Response(200, [], "Hello\n"); + + $this->expectOutputRegex('#^\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d\.\d\d\d 127\.0\.0\.1 "GET /users HTTP/1\.1" 200 6 0\.0\d\d' . PHP_EOL . '$#'); + $handler($request, function () use ($response) { return $response; }); + } + + public function testInvokeWithPathToNewFileWillCreateNewFileWithLogMessage(): void + { + $path = tempnam(sys_get_temp_dir(), 'log'); + assert(is_string($path)); + unlink($path); + + $handler = new AccessLogHandler($path); + + $request = new ServerRequest('GET', 'http://localhost:8080/users', [], '', '1.1', ['REMOTE_ADDR' => '127.0.0.1']); + $response = new Response(200, [], "Hello\n"); + $handler($request, function () use ($response) { return $response; }); + + $log = file_get_contents($path); + assert(is_string($log)); + + if (method_exists($this, 'assertMatchesRegularExpression')) { + $this->assertMatchesRegularExpression('#^\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d\.\d\d\d 127\.0\.0\.1 "GET /users HTTP/1\.1" 200 6 0\.0\d\d' . PHP_EOL . '$#', $log); + } else { + // legacy PHPUnit < 9.1 + $this->assertRegExp('#^\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d\.\d\d\d 127\.0\.0\.1 "GET /users HTTP/1\.1" 200 6 0\.0\d\d' . PHP_EOL . '$#', $log); + } + + unset($handler); + unlink($path); + } + + public function testInvokeWithPathToExistingFileWillAppendLogMessage(): void + { + $path = tempnam(sys_get_temp_dir(), 'log'); + assert(is_string($path)); + file_put_contents($path, 'first' . PHP_EOL); + + $handler = new AccessLogHandler($path); + + $request = new ServerRequest('GET', 'http://localhost:8080/users', [], '', '1.1', ['REMOTE_ADDR' => '127.0.0.1']); + $response = new Response(200, [], "Hello\n"); + $handler($request, function () use ($response) { return $response; }); + + $log = file_get_contents($path); + assert(is_string($log)); + + if (method_exists($this, 'assertMatchesRegularExpression')) { + $this->assertMatchesRegularExpression('#^first' . PHP_EOL . '\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d\.\d\d\d 127\.0\.0\.1 "GET /users HTTP/1\.1" 200 6 0\.0\d\d' . PHP_EOL . '$#', $log); + } else { + // legacy PHPUnit < 9.1 + $this->assertRegExp('#^first' . PHP_EOL . '\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d\.\d\d\d 127\.0\.0\.1 "GET /users HTTP/1\.1" 200 6 0\.0\d\d' . PHP_EOL . '$#', $log); + } + + unset($handler); + unlink($path); + } + + /** + * @doesNotPerformAssertions + */ + public function testInvokeWithDevNullWritesNothing(): void + { + $handler = new AccessLogHandler(DIRECTORY_SEPARATOR !== '\\' ? '/dev/null' : __DIR__ . '\\nul'); + + $request = new ServerRequest('GET', 'http://localhost:8080/users', [], '', '1.1', ['REMOTE_ADDR' => '127.0.0.1']); + $response = new Response(200, [], "Hello\n"); + $handler($request, function () use ($response) { return $response; }); + } + public function testInvokeLogsRequest(): void { $handler = new AccessLogHandler(); diff --git a/tests/AppMiddlewareTest.php b/tests/AppMiddlewareTest.php index c88115d..36f6a61 100644 --- a/tests/AppMiddlewareTest.php +++ b/tests/AppMiddlewareTest.php @@ -4,7 +4,7 @@ use FrameworkX\AccessLogHandler; use FrameworkX\App; -use FrameworkX\Io\MiddlewareHandler; +use FrameworkX\ErrorHandler; use FrameworkX\Io\RouteHandler; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ResponseInterface; @@ -674,23 +674,10 @@ public function testInvokeWithGlobalMiddlewareReturnsResponseWhenGlobalMiddlewar /** @param callable|class-string ...$middleware */ private function createAppWithoutLogger(...$middleware): App { - $app = new App(...$middleware); - - $ref = new \ReflectionProperty($app, 'handler'); - $ref->setAccessible(true); - $middleware = $ref->getValue($app); - assert($middleware instanceof MiddlewareHandler); - - $ref = new \ReflectionProperty($middleware, 'handlers'); - $ref->setAccessible(true); - $handlers = $ref->getValue($middleware); - assert(is_array($handlers)); - - $first = array_shift($handlers); - $this->assertInstanceOf(AccessLogHandler::class, $first); - - $ref->setValue($middleware, $handlers); - - return $app; + return new App( + new AccessLogHandler(DIRECTORY_SEPARATOR !== '\\' ? '/dev/null' : __DIR__ . '\\nul'), + new ErrorHandler(), + ...$middleware + ); } } diff --git a/tests/AppTest.php b/tests/AppTest.php index 09c35f2..3875e4b 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -1651,23 +1651,10 @@ public function testInvokeWithMatchingRouteReturnsInternalServerErrorResponseWhe private function createAppWithoutLogger(callable ...$middleware): App { - $app = new App(...$middleware); - - $ref = new \ReflectionProperty($app, 'handler'); - $ref->setAccessible(true); - $middleware = $ref->getValue($app); - assert($middleware instanceof MiddlewareHandler); - - $ref = new \ReflectionProperty($middleware, 'handlers'); - $ref->setAccessible(true); - $handlers = $ref->getValue($middleware); - assert(is_array($handlers)); - - $first = array_shift($handlers); - $this->assertInstanceOf(AccessLogHandler::class, $first); - - $ref->setValue($middleware, $handlers); - - return $app; + return new App( + new AccessLogHandler(DIRECTORY_SEPARATOR !== '\\' ? '/dev/null' : __DIR__ . '\\nul'), + new ErrorHandler(), + ...$middleware + ); } } diff --git a/tests/Io/LogStreamHandlerTest.php b/tests/Io/LogStreamHandlerTest.php index 8bb6461..1f28240 100644 --- a/tests/Io/LogStreamHandlerTest.php +++ b/tests/Io/LogStreamHandlerTest.php @@ -7,6 +7,192 @@ class LogStreamHandlerTest extends TestCase { + public static function provideFilesystemPaths(): \Generator + { + yield [ + __FILE__, + true + ]; + yield [ + __FILE__ . "\0", + false + ]; + yield [ + str_replace(DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, __FILE__), + true + ]; + yield [ + str_replace(DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR === '\\' ? '/' : '\\', __FILE__), + DIRECTORY_SEPARATOR === '\\' + ]; + + yield [ + 'access.log', + false + ]; + yield [ + './access.log', + false + ]; + yield [ + '../access.log', + false + ]; + yield [ + '.\\access.log', + false + ]; + yield [ + '..\\access.log', + false + ]; + yield [ + '\\\\access.log', + false + ]; + if (DIRECTORY_SEPARATOR === '\\') { + // invalid paths on Windows, technically valid on Unix but unlikely to be writable here + yield [ + '/access.log', + false + ]; + yield [ + '//access.log', + false + ]; + } + + yield [ + '', + false + ]; + yield [ + '.', + false + ]; + yield [ + '..', + false + ]; + yield [ + __DIR__ . DIRECTORY_SEPARATOR . "\0", + false + ]; + + yield [ + '/dev/null', + DIRECTORY_SEPARATOR !== '\\' + ]; + yield [ + 'nul', + false + ]; + yield [ + '\\\\.\\nul', + false + ]; + if (DIRECTORY_SEPARATOR === '\\') { + // valid path on Windows, but we don't want to write here on Unix + yield [ + __DIR__ . DIRECTORY_SEPARATOR . 'nul', + true + ]; + yield [ + __DIR__ . DIRECTORY_SEPARATOR . 'NUL', + true + ]; + } + + yield [ + 'php://stdout', + true + ]; + yield [ + 'PHP://STDOUT', + true + ]; + yield [ + 'php:stdout', + false + ]; + + yield [ + 'php://stderr', + true + ]; + yield [ + 'PHP://STDERR', + true + ]; + yield [ + 'php:stderr', + false + ]; + } + + public static function provideValidPaths(): \Generator + { + foreach (self::provideFilesystemPaths() as [$path, $valid]) { + if ($valid) { + yield [$path]; + } + } + } + + /** + * @dataProvider providevalidPaths + * @doesNotPerformAssertions + */ + public function testCtorWithValidPathWorks(string $path): void + { + new LogStreamHandler($path); + } + + public static function provideInvalidPaths(): \Generator + { + foreach (self::provideFilesystemPaths() as [$path, $valid]) { + if (!$valid) { + yield [$path]; + } + } + } + + /** + * @dataProvider provideInvalidPaths + */ + public function testCtorWithInvalidPathThrows(string $path): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Unable to open log file "' . addslashes($path) . '": Invalid path given'); + new LogStreamHandler($path); + } + + public function testCtorWithDirectoryInsteadOfFileThrowsWithoutCallingGlobalErrorHandler(): void + { + $called = 0; + set_error_handler($new = function () use (&$called): bool { + ++$called; + return false; + }); + + try { + try { + new LogStreamHandler(__DIR__); + } finally { + $previous = set_error_handler(function (): bool { return false; }); + restore_error_handler(); + restore_error_handler(); + } + $this->fail(); + } catch (\RuntimeException $e) { + $errstr = DIRECTORY_SEPARATOR === '\\' ? 'Permission denied' : 'Is a directory'; + $this->assertEquals('Unable to open log file "' . __DIR__ . '": ' . $errstr, $e->getMessage()); + + $this->assertEquals(0, $called); + $this->assertSame($new, $previous ?? null); + } + } + public function testLogWithMemoryStreamWritesMessageWithCurrentDateAndTime(): void { $logger = new LogStreamHandler('php://memory'); @@ -40,29 +226,65 @@ public function testLogWithOutputStreamPrintsMessageWithCurrentDateAndTime(): vo $logger->log('Hello'); } - public function testCtorWithDirectoryInsteadOfFileThrowsWithoutCallingGlobalErrorHandler(): void + public function testLogWithPathToNewFileWillCreateNewFileWithLogMessageAndCurrentDateAndTime(): void { - $called = 0; - set_error_handler($new = function () use (&$called): bool { - ++$called; - return false; - }); + $path = tempnam(sys_get_temp_dir(), 'log'); + assert(is_string($path)); + unlink($path); - try { - try { - new LogStreamHandler(__DIR__); - } finally { - $previous = set_error_handler(function (): bool { return false; }); - restore_error_handler(); - restore_error_handler(); - } - $this->fail(); - } catch (\RuntimeException $e) { - $errstr = DIRECTORY_SEPARATOR === '\\' ? 'Permission denied' : 'Is a directory'; - $this->assertEquals('Unable to open log file "' . __DIR__ . '": ' . $errstr, $e->getMessage()); + $logger = new LogStreamHandler($path); - $this->assertEquals(0, $called); - $this->assertSame($new, $previous ?? null); + $logger->log('Hello'); + + $output = file_get_contents($path); + assert(is_string($output)); + + // 2021-01-29 12:22:01.717 Hello\n + if (method_exists($this, 'assertMatchesRegularExpression')) { + $this->assertMatchesRegularExpression("/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3} Hello" . PHP_EOL . "$/", $output); // @phpstan-ignore-line + } else { + // legacy PHPUnit < 9.1 + $this->assertRegExp("/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3} Hello" . PHP_EOL . "$/", $output); + } + + unset($logger); + unlink($path); + } + + public function testLogWithPathToExistingFileWillAppendLogMessageWithCurrentDateAndTime(): void + { + $stream = tmpfile(); + assert(is_resource($stream)); + fwrite($stream, 'First' . PHP_EOL); + + $meta = stream_get_meta_data($stream); + assert(is_string($meta['uri'])); + + $logger = new LogStreamHandler($meta['uri']); + + $logger->log('Hello'); + + rewind($stream); + $output = stream_get_contents($stream); + assert(is_string($output)); + + // First\n + // 2021-01-29 12:22:01.717 Hello\n + if (method_exists($this, 'assertMatchesRegularExpression')) { + $this->assertMatchesRegularExpression("/^First" . PHP_EOL . "\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3} Hello" . PHP_EOL . "$/", $output); // @phpstan-ignore-line + } else { + // legacy PHPUnit < 9.1 + $this->assertRegExp("/^First" . PHP_EOL . "\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3} Hello" . PHP_EOL . "$/", $output); } } + + /** + * @doesNotPerformAssertions + */ + public function testLogWithDevNullWritesNothing(): void + { + $logger = new LogStreamHandler(DIRECTORY_SEPARATOR !== '\\' ? '/dev/null' : __DIR__ . '\\nul'); + + $logger->log('Hello'); + } }