Skip to content

Commit

Permalink
Prevented entity "Not Found" events from being logged
Browse files Browse the repository at this point in the history
- Added testing to cover, which was more hassle than thought
  since Laravel did not have built in log test helpers, so:
- Added Log testing helper.

Related to BookStackApp#2110
  • Loading branch information
ssddanbrown committed May 23, 2020
1 parent bf4a3b7 commit 19bfc8a
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 6 deletions.
7 changes: 7 additions & 0 deletions app/Config/logging.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@
'driver' => 'monolog',
'handler' => NullHandler::class,
],

// Testing channel
// Uses a shared testing instance during tests
// so that logs can be checked against.
'testing' => [
'driver' => 'testing',
],
],

];
2 changes: 1 addition & 1 deletion app/Exceptions/Handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;
use Illuminate\Http\Response;
use Illuminate\Validation\ValidationException;
use Symfony\Component\HttpKernel\Exception\HttpException;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
Expand All @@ -26,6 +25,7 @@ class Handler extends ExceptionHandler
HttpException::class,
ModelNotFoundException::class,
ValidationException::class,
NotFoundException::class,
];

/**
Expand Down
1 change: 1 addition & 0 deletions phpcs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<description>The coding standard for BookStack.</description>
<file>app</file>
<exclude-pattern>*/migrations/*</exclude-pattern>
<exclude-pattern>*/tests/*</exclude-pattern>
<arg value="np"/>
<rule ref="PSR2"/>
</ruleset>
20 changes: 20 additions & 0 deletions tests/ErrorTest.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
<?php namespace Tests;

use BookStack\Entities\Book;
use Illuminate\Support\Facades\Log;

class ErrorTest extends TestCase
{

Expand All @@ -18,4 +21,21 @@ public function test_404_page_does_not_show_login()
$notFound->assertDontSeeText('Log in');
$notFound->assertSeeText('tester');
}

public function test_item_not_found_does_not_get_logged_to_file()
{
$this->actingAs($this->getViewer());
$handler = $this->withTestLogger();
$book = Book::query()->first();

// Ensure we're seeing errors
Log::error('cat');
$this->assertTrue($handler->hasErrorThatContains('cat'));

$this->get('/books/arandomnotfouindbook');
$this->get($book->getUrl('/chapter/arandomnotfouindchapter'));
$this->get($book->getUrl('/chapter/arandomnotfouindpages'));

$this->assertCount(1, $handler->getRecords());
}
}
31 changes: 26 additions & 5 deletions tests/SharedTestHelpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
use BookStack\Settings\SettingService;
use BookStack\Uploads\HttpFetcher;
use Illuminate\Support\Env;
use Illuminate\Support\Facades\Log;
use Mockery;
use Monolog\Handler\TestHandler;
use Monolog\Logger;
use Throwable;

trait SharedTestHelpers
Expand Down Expand Up @@ -69,14 +72,14 @@ protected function getEditor() {
}

/**
* Get an instance of a user with 'viewer' permissions
* @param $attributes
* @return mixed
* Get an instance of a user with 'viewer' permissions.
*/
protected function getViewer($attributes = [])
protected function getViewer(array $attributes = []): User
{
$user = Role::getRole('viewer')->users()->first();
if (!empty($attributes)) $user->forceFill($attributes)->save();
if (!empty($attributes)) {
$user->forceFill($attributes)->save();
}
return $user;
}

Expand Down Expand Up @@ -277,4 +280,22 @@ protected function assertPermissionError($response)
$this->assertStringStartsWith('You do not have permission to access', $error);
}

/**
* Set a test handler as the logging interface for the application.
* Allows capture of logs for checking against during tests.
*/
protected function withTestLogger(): TestHandler
{
$monolog = new Logger('testing');
$testHandler = new TestHandler();
$monolog->pushHandler($testHandler);

Log::extend('testing', function() use ($monolog) {
return $monolog;
});
Log::setDefaultDriver('testing');

return $testHandler;
}

}

0 comments on commit 19bfc8a

Please sign in to comment.