1
0
mirror of https://github.com/Seldaek/monolog.git synced 2025-08-01 19:00:20 +02:00

Fix LogRecord "extra" data leaking between handlers (#1819)

Co-authored-by: Jordi Boggiano <j.boggiano@seld.be>
This commit is contained in:
Cosmin Ardeleanu
2023-10-27 17:54:39 +03:00
committed by GitHub
parent 8ff4ab5c94
commit b0f4bf7eb7
9 changed files with 128 additions and 19 deletions

View File

@@ -33,7 +33,7 @@ class FallbackGroupHandler extends GroupHandler
}
foreach ($this->handlers as $handler) {
try {
$handler->handle($record);
$handler->handle(clone $record);
break;
} catch (Throwable $e) {
// What throwable?
@@ -58,7 +58,7 @@ class FallbackGroupHandler extends GroupHandler
foreach ($this->handlers as $handler) {
try {
$handler->handleBatch($records);
$handler->handleBatch(array_map(fn ($record) => clone $record, $records));
break;
} catch (Throwable $e) {
// What throwable?

View File

@@ -70,7 +70,7 @@ class GroupHandler extends Handler implements ProcessableHandlerInterface, Reset
}
foreach ($this->handlers as $handler) {
$handler->handle($record);
$handler->handle(clone $record);
}
return false === $this->bubble;
@@ -90,7 +90,7 @@ class GroupHandler extends Handler implements ProcessableHandlerInterface, Reset
}
foreach ($this->handlers as $handler) {
$handler->handleBatch($records);
$handler->handleBatch(array_map(fn ($record) => clone $record, $records));
}
}

View File

@@ -33,7 +33,7 @@ class WhatFailureGroupHandler extends GroupHandler
foreach ($this->handlers as $handler) {
try {
$handler->handle($record);
$handler->handle(clone $record);
} catch (Throwable) {
// What failure?
}
@@ -57,7 +57,7 @@ class WhatFailureGroupHandler extends GroupHandler
foreach ($this->handlers as $handler) {
try {
$handler->handleBatch($records);
$handler->handleBatch(array_map(fn ($record) => clone $record, $records));
} catch (Throwable) {
// What failure?
}

View File

@@ -355,11 +355,11 @@ class Logger implements LoggerInterface, ResettableInterface
$recordInitialized = count($this->processors) === 0;
$record = new LogRecord(
datetime: $datetime ?? new DateTimeImmutable($this->microsecondTimestamps, $this->timezone),
channel: $this->name,
level: self::toMonologLevel($level),
message: $message,
context: $context,
level: self::toMonologLevel($level),
channel: $this->name,
datetime: $datetime ?? new DateTimeImmutable($this->microsecondTimestamps, $this->timezone),
extra: [],
);
$handled = false;
@@ -386,7 +386,7 @@ class Logger implements LoggerInterface, ResettableInterface
// once the record is initialized, send it to all handlers as long as the bubbling chain is not interrupted
try {
$handled = true;
if (true === $handler->handle($record)) {
if (true === $handler->handle(clone $record)) {
break;
}
} catch (Throwable $e) {

View File

@@ -19,10 +19,8 @@ class ExceptionTestHandler extends TestHandler
/**
* @inheritDoc
*/
public function handle(LogRecord $record): bool
protected function write(LogRecord $record): void
{
throw new Exception("ExceptionTestHandler::handle");
parent::handle($record);
}
}

View File

@@ -12,6 +12,7 @@
namespace Monolog\Handler;
use Monolog\Level;
use Monolog\LogRecord;
use Monolog\Test\TestCase;
class FallbackGroupHandlerTest extends TestCase
@@ -138,4 +139,37 @@ class FallbackGroupHandlerTest extends TestCase
$this->assertTrue($records[0]['extra']['foo2']);
$this->assertTrue($records[1]['extra']['foo2']);
}
public function testProcessorsDoNotInterfereBetweenHandlers()
{
$t1 = new ExceptionTestHandler();
$t2 = new TestHandler();
$handler = new FallbackGroupHandler([$t1, $t2]);
$t1->pushProcessor(function (LogRecord $record) {
$record->extra['foo'] = 'bar';
return $record;
});
$handler->handle($this->getRecord());
self::assertSame([], $t2->getRecords()[0]->extra);
}
public function testProcessorsDoNotInterfereBetweenHandlersWithBatch()
{
$t1 = new ExceptionTestHandler();
$t2 = new TestHandler();
$handler = new FallbackGroupHandler([$t1, $t2]);
$t1->pushProcessor(function (LogRecord $record) {
$record->extra['foo'] = 'bar';
return $record;
});
$handler->handleBatch([$this->getRecord()]);
self::assertSame([], $t2->getRecords()[0]->extra);
}
}

View File

@@ -11,6 +11,7 @@
namespace Monolog\Handler;
use Monolog\LogRecord;
use Monolog\Test\TestCase;
use Monolog\Level;
@@ -117,4 +118,37 @@ class GroupHandlerTest extends TestCase
$this->assertTrue($records[1]['extra']['foo2']);
}
}
public function testProcessorsDoNotInterfereBetweenHandlers()
{
$t1 = new TestHandler();
$t2 = new TestHandler();
$handler = new GroupHandler([$t1, $t2]);
$t1->pushProcessor(function (LogRecord $record) {
$record->extra['foo'] = 'bar';
return $record;
});
$handler->handle($this->getRecord());
self::assertSame([], $t2->getRecords()[0]->extra);
}
public function testProcessorsDoNotInterfereBetweenHandlersWithBatch()
{
$t1 = new TestHandler();
$t2 = new TestHandler();
$handler = new GroupHandler([$t1, $t2]);
$t1->pushProcessor(function (LogRecord $record) {
$record->extra['foo'] = 'bar';
return $record;
});
$handler->handleBatch([$this->getRecord()]);
self::assertSame([], $t2->getRecords()[0]->extra);
}
}

View File

@@ -11,6 +11,7 @@
namespace Monolog\Handler;
use Monolog\LogRecord;
use Monolog\Test\TestCase;
use Monolog\Level;
@@ -136,4 +137,37 @@ class WhatFailureGroupHandlerTest extends TestCase
$records = $test->getRecords();
$this->assertTrue($records[0]['extra']['foo']);
}
public function testProcessorsDoNotInterfereBetweenHandlers()
{
$t1 = new TestHandler();
$t2 = new TestHandler();
$handler = new WhatFailureGroupHandler([$t1, $t2]);
$t1->pushProcessor(function (LogRecord $record) {
$record->extra['foo'] = 'bar';
return $record;
});
$handler->handle($this->getRecord());
self::assertSame([], $t2->getRecords()[0]->extra);
}
public function testProcessorsDoNotInterfereBetweenHandlersWithBatch()
{
$t1 = new TestHandler();
$t2 = new TestHandler();
$handler = new WhatFailureGroupHandler([$t1, $t2]);
$t1->pushProcessor(function (LogRecord $record) {
$record->extra['foo'] = 'bar';
return $record;
});
$handler->handleBatch([$this->getRecord()]);
self::assertSame([], $t2->getRecords()[0]->extra);
}
}

View File

@@ -638,6 +638,21 @@ class LoggerTest extends TestCase
];
}
public function testProcessorsDoNotInterfereBetweenHandlers()
{
$logger = new Logger('foo');
$logger->pushHandler($t1 = new TestHandler());
$logger->pushHandler($t2 = new TestHandler());
$t1->pushProcessor(function (LogRecord $record) {
$record->extra['foo'] = 'bar';
return $record;
});
$logger->error('Foo');
self::assertSame([], $t2->getRecords()[0]->extra);
}
/**
* @covers Logger::setExceptionHandler
*/
@@ -804,9 +819,6 @@ class LoggerTest extends TestCase
}
}
/**
* @requires PHP 8.1
*/
public function testLogCycleDetectionWithFibersWithoutCycle()
{
$logger = new Logger(__METHOD__);
@@ -832,9 +844,6 @@ class LoggerTest extends TestCase
self::assertCount(10, $testHandler->getRecords());
}
/**
* @requires PHP 8.1
*/
public function testLogCycleDetectionWithFibersWithCycle()
{
$logger = new Logger(__METHOD__);