From 367d798f432aae14296dfc193c591b5aaf5e90b2 Mon Sep 17 00:00:00 2001 From: "steven.lewis" Date: Thu, 31 Oct 2024 13:48:54 +0000 Subject: [PATCH] ADD PSR-3 exception error logging --- README.markdown | 2 +- composer.json | 7 +- src/JsonRPC/Logger/NullLogger.php | 15 --- src/JsonRPC/Request/BatchRequestParser.php | 2 +- .../Logger/DebugRequestLogger.php} | 11 +- .../Request/Logger/NullRequestLogger.php | 16 +++ .../Logger/RequestLoggerInterface.php} | 6 +- src/JsonRPC/Request/RequestParser.php | 32 ++++-- src/JsonRPC/Response/ResponseBuilder.php | 85 +++++++++------ src/JsonRPC/Server.php | 101 +++++++++--------- tests/ClientTest.php | 4 +- tests/HttpClientTest.php | 22 ++-- tests/Response/HeaderMockTest.php | 4 +- tests/ServerTest.php | 14 +-- 14 files changed, 182 insertions(+), 139 deletions(-) delete mode 100644 src/JsonRPC/Logger/NullLogger.php rename src/JsonRPC/{Logger/DebugLogger.php => Request/Logger/DebugRequestLogger.php} (63%) create mode 100644 src/JsonRPC/Request/Logger/NullRequestLogger.php rename src/JsonRPC/{Logger/LoggerInterface.php => Request/Logger/RequestLoggerInterface.php} (70%) diff --git a/README.markdown b/README.markdown index f5af0a4..154b763 100644 --- a/README.markdown +++ b/README.markdown @@ -13,7 +13,7 @@ Features - Authentication and IP based client restrictions - Custom Middleware - Fully unit tested -- Requirements: PHP >= 7.1|8 +- Requirements: PHP >= 8.0 - License: MIT Author diff --git a/composer.json b/composer.json index bb8c87a..689343a 100644 --- a/composer.json +++ b/composer.json @@ -16,13 +16,14 @@ "fguillot/json-rpc": "1.*" }, "require": { - "php": ">=7.1", - "ext-json": "*" + "php": ">=8.0", + "ext-json": "*", + "psr/log": ">=3" }, "autoload": { "psr-0": {"JsonRPC": "src/"} }, "require-dev": { - "phpunit/phpunit": "^7" + "phpunit/phpunit": "^9 || ^10.0 || ^11.0" } } diff --git a/src/JsonRPC/Logger/NullLogger.php b/src/JsonRPC/Logger/NullLogger.php deleted file mode 100644 index ac7a54d..0000000 --- a/src/JsonRPC/Logger/NullLogger.php +++ /dev/null @@ -1,15 +0,0 @@ -withProcedureHandler($this->procedureHandler) ->withMiddlewareHandler($this->middlewareHandler) ->withLocalException($this->localExceptions) - ->withLogger($this->logger) + ->withRequestLogger($this->requestLogger) ->parse(); } diff --git a/src/JsonRPC/Logger/DebugLogger.php b/src/JsonRPC/Request/Logger/DebugRequestLogger.php similarity index 63% rename from src/JsonRPC/Logger/DebugLogger.php rename to src/JsonRPC/Request/Logger/DebugRequestLogger.php index ef7a8cb..2de82f1 100644 --- a/src/JsonRPC/Logger/DebugLogger.php +++ b/src/JsonRPC/Request/Logger/DebugRequestLogger.php @@ -1,22 +1,23 @@ logs[] = array( 'id' => $id, @@ -28,7 +29,7 @@ public function log($id, $method, $params, $response, $timeTaken = 0, $metadata ); } - public function getLogs() + public function getLogs(): array { return $this->logs; } diff --git a/src/JsonRPC/Request/Logger/NullRequestLogger.php b/src/JsonRPC/Request/Logger/NullRequestLogger.php new file mode 100644 index 0000000..6bc35e7 --- /dev/null +++ b/src/JsonRPC/Request/Logger/NullRequestLogger.php @@ -0,0 +1,16 @@ +psr3Logger = $logger; + return $this; + } + /** - * @param LoggerInterface $logger + * @param RequestLoggerInterface $logger * @return $this */ - public function withLogger(LoggerInterface $logger) + public function withRequestLogger(RequestLoggerInterface $logger) { - $this->logger = $logger; + $this->requestLogger = $logger; return $this; } @@ -182,8 +191,8 @@ public function parse() $endTime = round(microtime(true),5, PHP_ROUND_HALF_UP); $callTime = round(($endTime - $startTime),5, PHP_ROUND_HALF_UP); - if($this->logger){ - $this->logger->log( + if($this->requestLogger){ + $this->requestLogger->log( (empty($this->payload['id']) ? 0 : $this->payload['id']), (empty($this->payload['method']) ? '' : $this->payload['method']), (empty($this->payload['params']) ? array() : $this->payload['params']), @@ -213,9 +222,14 @@ protected function handleExceptions(Exception $e) if ($e instanceof InvalidJsonRpcFormatException || ! $this->isNotification()) { return ResponseBuilder::create() - ->withId(isset($this->payload['id']) ? $this->payload['id'] : null) + ->withPsr3Logger($this->psr3Logger) + ->withId($this->payload['id'] ?? null) ->withException($e) ->build(); + } elseif($this->psr3Logger) { + $this->psr3Logger->error( + $e->getMessage(), ['file' => $e->getFile(), 'line' => $e->getLine(), 'trace' => $e->getTrace()] + ); } return ''; diff --git a/src/JsonRPC/Response/ResponseBuilder.php b/src/JsonRPC/Response/ResponseBuilder.php index a0348ce..112c57a 100644 --- a/src/JsonRPC/Response/ResponseBuilder.php +++ b/src/JsonRPC/Response/ResponseBuilder.php @@ -12,6 +12,7 @@ use JsonRPC\Exception\ResponseEncodingFailureException; use JsonRPC\Exception\ResponseException; use JsonRPC\Validator\JsonEncodingValidator; +use Psr\Log\LoggerInterface; /** * Class ResponseBuilder @@ -87,6 +88,8 @@ class ResponseBuilder */ protected $exception; + protected ?LoggerInterface $psr3Logger; + /** * Get new object instance * @@ -103,7 +106,7 @@ public static function create() * Set id * * @access public - * @param mixed $id + * @param mixed $id * @return $this */ public function withId($id) @@ -116,7 +119,7 @@ public function withId($id) * Set result * * @access public - * @param mixed $result + * @param mixed $result * @return $this */ public function withResult($result) @@ -129,16 +132,16 @@ public function withResult($result) * Set error * * @access public - * @param integer $code - * @param string $message - * @param string $data + * @param integer $code + * @param string $message + * @param string $data * @return $this */ public function withError($code, $message, $data = '') { - $this->errorCode = $code; + $this->errorCode = $code; $this->errorMessage = $message; - $this->errorData = $data; + $this->errorData = $data; return $this; } @@ -146,7 +149,7 @@ public function withError($code, $message, $data = '') * Set exception * * @access public - * @param Exception $exception + * @param Exception $exception * @return $this */ public function withException(Exception $exception) @@ -159,8 +162,8 @@ public function withException(Exception $exception) * Add HTTP header * * @access public - * @param string $name - * @param string $value + * @param string $name + * @param string $value * @return $this */ public function withHeader($name, $value) @@ -173,7 +176,7 @@ public function withHeader($name, $value) * Add HTTP Status * * @access public - * @param string $status + * @param string $status * @return $this */ public function withStatus($status) @@ -182,6 +185,12 @@ public function withStatus($status) return $this; } + public function withPsr3Logger(?LoggerInterface $logger): static + { + $this->psr3Logger = $logger; + return $this; + } + /** * Get status * @@ -233,12 +242,12 @@ public function build() */ public function sendHeaders() { - if (! empty($this->status)) { + if (!empty($this->status)) { header($this->status); } foreach ($this->headers as $name => $value) { - header($name.': '.$value); + header($name . ': ' . $value); } return $this; @@ -255,7 +264,7 @@ protected function buildResponse() $response = array('jsonrpc' => '2.0'); $this->handleExceptions(); - if (! empty($this->errorMessage)) { + if (!empty($this->errorMessage)) { $response['error'] = $this->buildErrorResponse(); } else { $response['result'] = $this->result; @@ -274,11 +283,11 @@ protected function buildResponse() protected function buildErrorResponse() { $response = array( - 'code' => $this->errorCode, + 'code' => $this->errorCode, 'message' => $this->errorMessage, ); - if (! empty($this->errorData)) { + if (!empty($this->errorData)) { $response['data'] = $this->errorData; } @@ -297,40 +306,52 @@ protected function handleExceptions() throw $this->exception; } } catch (InvalidJsonFormatException $e) { - $this->errorCode = -32700; + $this->errorCode = -32700; $this->errorMessage = 'Parse error'; - $this->id = null; + $this->id = null; } catch (InvalidJsonRpcFormatException $e) { - $this->errorCode = -32600; + $this->errorCode = -32600; $this->errorMessage = 'Invalid Request'; - $this->id = null; + $this->id = null; } catch (BadFunctionCallException $e) { - $this->errorCode = -32601; + $this->errorCode = -32601; $this->errorMessage = 'Method not found'; } catch (InvalidArgumentException $e) { - $this->errorCode = -32602; + $this->errorCode = -32602; $this->errorMessage = 'Invalid params'; - $this->errorData = $this->exception->getMessage(); + $this->errorData = $this->exception->getMessage(); } catch (ResponseEncodingFailureException $e) { - $this->errorCode = -32603; + $this->errorCode = -32603; $this->errorMessage = 'Internal error'; - $this->errorData = $this->exception->getMessage(); + $this->errorData = $this->exception->getMessage(); } catch (AuthenticationFailureException $e) { - $this->errorCode = 401; + $this->errorCode = 401; $this->errorMessage = 'Unauthorized'; - $this->status = 'HTTP/1.0 401 Unauthorized'; + $this->status = 'HTTP/1.0 401 Unauthorized'; $this->withHeader('WWW-Authenticate', 'Basic realm="JsonRPC"'); } catch (AccessDeniedException $e) { - $this->errorCode = 403; + $this->errorCode = 403; $this->errorMessage = 'Forbidden'; - $this->status = 'HTTP/1.0 403 Forbidden'; + $this->status = 'HTTP/1.0 403 Forbidden'; } catch (ResponseException $e) { - $this->errorCode = $this->exception->getCode(); + $this->logException($this->exception); + $this->errorCode = $this->exception->getCode(); $this->errorMessage = $this->exception->getMessage(); - $this->errorData = $this->exception->getData(); + $this->errorData = $this->exception->getData(); } catch (Exception $e) { - $this->errorCode = $this->exception->getCode(); + $this->logException($this->exception); + $this->errorCode = $this->exception->getCode(); $this->errorMessage = $this->exception->getMessage(); } } + + protected function logException(\Exception $e): void + { + if ($e->getCode() < -32099 && $e->getCode() > -32000 && $this->psr3Logger) { + $this->psr3Logger->error( + $e->getMessage(), + ['file' => $e->getFile(), 'line' => $e->getLine(), 'trace' => $e->getTrace()] + ); + } + } } diff --git a/src/JsonRPC/Server.php b/src/JsonRPC/Server.php index fdda49e..289ef5b 100644 --- a/src/JsonRPC/Server.php +++ b/src/JsonRPC/Server.php @@ -4,13 +4,14 @@ use Closure; use Exception; -use JsonRPC\Logger\LoggerInterface; use JsonRPC\Request\BatchRequestParser; +use JsonRPC\Request\Logger\RequestLoggerInterface; use JsonRPC\Request\RequestParser; use JsonRPC\Response\ResponseBuilder; use JsonRPC\Validator\HostValidator; use JsonRPC\Validator\JsonFormatValidator; use JsonRPC\Validator\UserValidator; +use Psr\Log\LoggerInterface; /** * JsonRPC server class @@ -121,32 +122,35 @@ class Server * Server call logger * * @access protected - * @var LoggerInterface + * @var RequestLoggerInterface */ - protected $logger; + protected $requestLogger; + + protected ?LoggerInterface $psr3Logger; /** * Constructor * * @access public - * @param string $request - * @param array $server - * @param ResponseBuilder|null $responseBuilder - * @param RequestParser|null $requestParser - * @param BatchRequestParser|null $batchRequestParser - * @param ProcedureHandler|null $procedureHandler - * @param MiddlewareHandler|null $middlewareHandler - * @param LoggerInterface|null $logger + * @param string $request + * @param array $server + * @param ResponseBuilder|null $responseBuilder + * @param RequestParser|null $requestParser + * @param BatchRequestParser|null $batchRequestParser + * @param ProcedureHandler|null $procedureHandler + * @param MiddlewareHandler|null $middlewareHandler + * @param RequestLoggerInterface|null $requestLogger */ public function __construct( - $request = '', + string $request = '', array $server = array(), ?ResponseBuilder $responseBuilder = null, ?RequestParser $requestParser = null, ?BatchRequestParser $batchRequestParser = null, ?ProcedureHandler $procedureHandler = null, ?MiddlewareHandler $middlewareHandler = null, - ?LoggerInterface $logger = null + ?RequestLoggerInterface $requestLogger = null, + ?LoggerInterface $psr3Logger = null ) { if ($request !== '') { $this->payload = json_decode($request, true); @@ -154,29 +158,30 @@ public function __construct( $this->payload = json_decode(file_get_contents('php://input'), true); } - $this->serverVariable = $server ?: $_SERVER; - $this->responseBuilder = $responseBuilder ?: ResponseBuilder::create(); - $this->requestParser = $requestParser ?: RequestParser::create(); + $this->serverVariable = $server ?: $_SERVER; + $this->responseBuilder = $responseBuilder ?: ResponseBuilder::create(); + $this->requestParser = $requestParser ?: RequestParser::create(); $this->batchRequestParser = $batchRequestParser ?: BatchRequestParser::create(); - $this->procedureHandler = $procedureHandler ?: new ProcedureHandler(); - $this->middlewareHandler = $middlewareHandler ?: new MiddlewareHandler(); - $this->logger = $logger ?: new Logger\NullLogger(); + $this->procedureHandler = $procedureHandler ?: new ProcedureHandler(); + $this->middlewareHandler = $middlewareHandler ?: new MiddlewareHandler(); + $this->requestLogger = $requestLogger ?: new Request\Logger\NullRequestLogger(); + $this->psr3Logger = $psr3Logger; } /** * Define alternative authentication header * * @access public - * @param string $header Header name + * @param string $header Header name * @return $this */ public function setAuthenticationHeader($header) { - if (! empty($header)) { - $header = 'HTTP_'.str_replace('-', '_', strtoupper($header)); - $value = $this->getServerVariable($header); + if (!empty($header)) { + $header = 'HTTP_' . str_replace('-', '_', strtoupper($header)); + $value = $this->getServerVariable($header); - if (! empty($value)) { + if (!empty($value)) { list($this->username, $this->password) = explode(':', base64_decode($value)); } } @@ -232,7 +237,7 @@ public function getPassword() * IP based client restrictions * * @access public - * @param array $hosts List of hosts + * @param array $hosts List of hosts * @return $this */ public function allowHosts(array $hosts) @@ -245,7 +250,7 @@ public function allowHosts(array $hosts) * HTTP Basic authentication * * @access public - * @param array $users Dictionary of username/password + * @param array $users Dictionary of username/password * @return $this */ public function authentication(array $users) @@ -257,11 +262,11 @@ public function authentication(array $users) /** * Register a new procedure * - * @access public - * @deprecated Use $server->getProcedureHandler()->withCallback($procedure, $callback) - * @param string $procedure Procedure name - * @param closure $callback Callback + * @access public + * @param string $procedure Procedure name + * @param closure $callback Callback * @return $this + * @deprecated Use $server->getProcedureHandler()->withCallback($procedure, $callback) */ public function register($procedure, Closure $callback) { @@ -272,12 +277,12 @@ public function register($procedure, Closure $callback) /** * Bind a procedure to a class * - * @access public - * @deprecated Use $server->getProcedureHandler()->withClassAndMethod($procedure, $class, $method); - * @param string $procedure Procedure name - * @param mixed $class Class name or instance - * @param string $method Procedure name + * @access public + * @param string $procedure Procedure name + * @param mixed $class Class name or instance + * @param string $method Procedure name * @return $this + * @deprecated Use $server->getProcedureHandler()->withClassAndMethod($procedure, $class, $method); */ public function bind($procedure, $class, $method = '') { @@ -288,10 +293,10 @@ public function bind($procedure, $class, $method = '') /** * Bind a class instance * - * @access public - * @deprecated Use $server->getProcedureHandler()->withObject($instance); - * @param mixed $instance Instance name + * @access public + * @param mixed $instance Instance name * @return $this + * @deprecated Use $server->getProcedureHandler()->withObject($instance); */ public function attach($instance) { @@ -303,7 +308,7 @@ public function attach($instance) * Exception classes that should not be relayed to the client * * @access public - * @param Exception|string $exception + * @param Exception|string $exception * @return $this */ public function withLocalException($exception) @@ -327,11 +332,9 @@ public function execute() $this->middlewareHandler ->withUsername($this->getUsername()) - ->withPassword($this->getPassword()) - ; + ->withPassword($this->getPassword()); $response = $this->parseRequest(); - } catch (Exception $e) { $response = $this->handleExceptions($e); } @@ -344,7 +347,7 @@ public function execute() * Handle exceptions * * @access protected - * @param Exception $e + * @param Exception $e * @return string * @throws Exception */ @@ -356,7 +359,7 @@ protected function handleExceptions(Exception $e) } } - return $this->responseBuilder->withException($e)->build(); + return $this->responseBuilder->withPsr3Logger($this->psr3Logger)->withException($e)->build(); } /** @@ -373,7 +376,8 @@ protected function parseRequest() ->withProcedureHandler($this->procedureHandler) ->withMiddlewareHandler($this->middlewareHandler) ->withLocalException($this->localExceptions) - ->withLogger($this->logger) + ->withRequestLogger($this->requestLogger) + ->withPsr3Logger($this->psr3Logger) ->parse(); } @@ -382,7 +386,8 @@ protected function parseRequest() ->withProcedureHandler($this->procedureHandler) ->withMiddlewareHandler($this->middlewareHandler) ->withLocalException($this->localExceptions) - ->withLogger($this->logger) + ->withRequestLogger($this->requestLogger) + ->withPsr3Logger($this->psr3Logger) ->parse(); } @@ -390,11 +395,11 @@ protected function parseRequest() * Check existence and get value of server variable * * @access protected - * @param string $variable + * @param string $variable * @return string|null */ protected function getServerVariable($variable) { - return isset($this->serverVariable[$variable]) ? $this->serverVariable[$variable] : null; + return $this->serverVariable[$variable] ?? null; } } diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 1777478..5ebadfd 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -9,11 +9,11 @@ class ClientTest extends TestCase { private $httpClient; - public function setUp() + protected function setUp(): void { $this->httpClient = $this ->getMockBuilder('\JsonRPC\HttpClient') - ->setMethods(array('execute')) + ->onlyMethods(array('execute')) ->getMock(); } diff --git a/tests/HttpClientTest.php b/tests/HttpClientTest.php index ec4ee96..d64d2ba 100644 --- a/tests/HttpClientTest.php +++ b/tests/HttpClientTest.php @@ -61,11 +61,11 @@ class HttpClientTest extends TestCase { public static $functions; - public function setUp() + public function setUp(): void { self::$functions = $this ->getMockBuilder('stdClass') - ->setMethods(array( + ->addMethods(array( 'extension_loaded', 'fopen', 'stream_context_create', @@ -123,13 +123,13 @@ public function testWithAccessNotAllowed() public function testWithCallback() { self::$functions - ->expects($this->at(0)) + ->expects($this->atLeast(0)) ->method('extension_loaded') ->with('curl') ->will($this->returnValue(false)); self::$functions - ->expects($this->at(1)) + ->expects($this->atLeast(1)) ->method('stream_context_create') ->with(array( 'http' => array( @@ -155,7 +155,7 @@ public function testWithCallback() ->will($this->returnValue('context')); self::$functions - ->expects($this->at(2)) + ->expects($this->atMost(2)) ->method('fopen') ->with('url', 'r', false, 'context') ->will($this->returnValue(false)); @@ -172,18 +172,18 @@ public function testWithCallback() public function testWithCurl() { self::$functions - ->expects($this->at(0)) + ->expects($this->atLeast(0)) ->method('extension_loaded') ->with('curl') ->will($this->returnValue(true)); self::$functions - ->expects($this->at(1)) + ->expects($this->atMost(1)) ->method('curl_init') ->will($this->returnValue('curl')); self::$functions - ->expects($this->at(2)) + ->expects($this->atMost(2)) ->method('curl_setopt_array') ->with('curl', array( CURLOPT_URL => 'url', @@ -207,18 +207,18 @@ public function testWithCurl() )); self::$functions - ->expects($this->at(3)) + ->expects($this->atMost(3)) ->method('curl_setopt') ->with('curl', CURLOPT_CAINFO, 'test.crt'); self::$functions - ->expects($this->at(4)) + ->expects($this->atMost(4)) ->method('curl_exec') ->with('curl') ->will($this->returnValue(false)); self::$functions - ->expects($this->at(5)) + ->expects($this->atMost(5)) ->method('curl_close') ->with('curl'); diff --git a/tests/Response/HeaderMockTest.php b/tests/Response/HeaderMockTest.php index 6b2bc29..513785b 100644 --- a/tests/Response/HeaderMockTest.php +++ b/tests/Response/HeaderMockTest.php @@ -14,11 +14,11 @@ abstract class HeaderMockTest extends TestCase { public static $functions; - public function setUp() + protected function setUp(): void { self::$functions = $this ->getMockBuilder('stdClass') - ->setMethods(array('header')) + ->addMethods(array('header')) ->getMock(); } } diff --git a/tests/ServerTest.php b/tests/ServerTest.php index 1da1458..7b9041e 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -23,10 +23,10 @@ public function execute($username, $password, $procedureName) } } -class DummyLogger implements JsonRPC\Logger\LoggerInterface +class DummyRequestLogger implements \JsonRPC\Request\Logger\RequestLoggerInterface { protected $logs = array(); - public function log($id, $method, $params, $response, $timeTaken = 0, $metadata = array()) + public function log($id, $method, $params, $response, int $timeTaken = 0, array $metadata = array()) { $this->logs[] = array('id' => $id,'method' => $method, 'params' => $params, 'response' => $response, 'metadata' => $metadata); } @@ -103,7 +103,7 @@ public function testExecuteRequestParserOverride() $requestParser->method('withProcedureHandler')->willReturn($requestParser); $requestParser->method('withMiddlewareHandler')->willReturn($requestParser); $requestParser->method('withLocalException')->willReturn($requestParser); - $requestParser->method('withLogger')->willReturn($requestParser); + $requestParser->method('withRequestLogger')->willReturn($requestParser); $server = new Server($this->payload, array(), null, $requestParser); @@ -122,7 +122,7 @@ public function testExecuteBatchRequestParserOverride() $batchRequestParser->method('withProcedureHandler')->willReturn($batchRequestParser); $batchRequestParser->method('withMiddlewareHandler')->willReturn($batchRequestParser); $batchRequestParser->method('withLocalException')->willReturn($batchRequestParser); - $batchRequestParser->method('withLogger')->willReturn($batchRequestParser); + $batchRequestParser->method('withRequestLogger')->willReturn($batchRequestParser); $server = new Server('["...", "..."]', array(), null, null, $batchRequestParser); @@ -156,7 +156,7 @@ public function testExecuteProcedureHandlerOverride() $batchRequestParser->method('withProcedureHandler')->willReturn($batchRequestParser); $batchRequestParser->method('withMiddlewareHandler')->willReturn($batchRequestParser); $batchRequestParser->method('withLocalException')->willReturn($batchRequestParser); - $batchRequestParser->method('withLogger')->willReturn($batchRequestParser); + $batchRequestParser->method('withRequestLogger')->willReturn($batchRequestParser); $server = new Server('["...", "..."]', array(), null, null, $batchRequestParser, $procedureHandler); @@ -276,7 +276,7 @@ public function testCustomResponseException() public function testLoggingWithResult() { - $logger = new DummyLogger(); + $logger = new DummyRequestLogger(); $server = new Server( $this->payload, array(), @@ -315,7 +315,7 @@ public function testLoggingWithResult() public function testLoggingWithException() { - $logger = new DummyLogger(); + $logger = new DummyRequestLogger(); $server = new Server( $this->payload, array(),