Updated not-found image path handling to have better ux
Added test to cover. Started refactoring some of the app error handling in the process of this. Fixes #2696
This commit is contained in:
parent
04c1d0e071
commit
7be7d7d1e7
7 changed files with 58 additions and 19 deletions
|
@ -68,19 +68,6 @@ class Handler extends ExceptionHandler
|
|||
return redirect($e->redirectLocation);
|
||||
}
|
||||
|
||||
// Handle pretty exceptions which will show a friendly application-fitting page
|
||||
// Which will include the basic message to point the user roughly to the cause.
|
||||
if ($this->isExceptionType($e, PrettyException::class) && !config('app.debug')) {
|
||||
$message = $this->getOriginalMessage($e);
|
||||
$code = ($e->getCode() === 0) ? 500 : $e->getCode();
|
||||
return response()->view('errors/' . $code, ['message' => $message], $code);
|
||||
}
|
||||
|
||||
// Handle 404 errors with a loaded session to enable showing user-specific information
|
||||
if ($this->isExceptionType($e, NotFoundHttpException::class)) {
|
||||
return \Route::respondWithRoute('fallback');
|
||||
}
|
||||
|
||||
return parent::render($request, $e);
|
||||
}
|
||||
|
||||
|
|
|
@ -1,6 +1,44 @@
|
|||
<?php namespace BookStack\Exceptions;
|
||||
|
||||
class PrettyException extends \Exception
|
||||
{
|
||||
use Exception;
|
||||
use Illuminate\Contracts\Support\Responsable;
|
||||
use Illuminate\Http\Request;
|
||||
|
||||
class PrettyException extends Exception implements Responsable
|
||||
{
|
||||
/**
|
||||
* @var ?string
|
||||
*/
|
||||
protected $subtitle = null;
|
||||
|
||||
/**
|
||||
* @var ?string
|
||||
*/
|
||||
protected $details = null;
|
||||
|
||||
/**
|
||||
* Render a response for when this exception occurs.
|
||||
* @param Request $request
|
||||
*/
|
||||
public function toResponse($request)
|
||||
{
|
||||
$code = ($this->getCode() === 0) ? 500 : $this->getCode();
|
||||
return response()->view('errors.' . $code, [
|
||||
'message' => $this->getMessage(),
|
||||
'subtitle' => $this->subtitle,
|
||||
'details' => $this->details,
|
||||
], $code);
|
||||
}
|
||||
|
||||
public function setSubtitle(string $subtitle): self
|
||||
{
|
||||
$this->subtitle = $subtitle;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function setDetails(string $details): self
|
||||
{
|
||||
$this->details = $details;
|
||||
return $this;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
<?php namespace BookStack\Http\Controllers\Images;
|
||||
|
||||
use BookStack\Exceptions\ImageUploadException;
|
||||
use BookStack\Exceptions\NotFoundException;
|
||||
use BookStack\Http\Controllers\Controller;
|
||||
use BookStack\Uploads\Image;
|
||||
use BookStack\Uploads\ImageRepo;
|
||||
|
@ -27,12 +28,15 @@ class ImageController extends Controller
|
|||
|
||||
/**
|
||||
* Provide an image file from storage.
|
||||
* @throws NotFoundException
|
||||
*/
|
||||
public function showImage(string $path)
|
||||
{
|
||||
$path = storage_path('uploads/images/' . $path);
|
||||
if (!file_exists($path)) {
|
||||
abort(404);
|
||||
throw (new NotFoundException(trans('errors.image_not_found')))
|
||||
->setSubtitle(trans('errors.image_not_found_subtitle'))
|
||||
->setDetails(trans('errors.image_not_found_details'));
|
||||
}
|
||||
|
||||
return response()->file($path);
|
||||
|
|
|
@ -83,6 +83,9 @@ return [
|
|||
'404_page_not_found' => 'Page Not Found',
|
||||
'sorry_page_not_found' => 'Sorry, The page you were looking for could not be found.',
|
||||
'sorry_page_not_found_permission_warning' => 'If you expected this page to exist, you might not have permission to view it.',
|
||||
'image_not_found' => 'Image Not Found',
|
||||
'image_not_found_subtitle' => 'Sorry, The image file you were looking for could not be found.',
|
||||
'image_not_found_details' => 'If you expected this image to exist it might have been deleted.',
|
||||
'return_home' => 'Return to home',
|
||||
'error_occurred' => 'An Error Occurred',
|
||||
'app_down' => ':appName is down right now',
|
||||
|
|
|
@ -7,8 +7,8 @@
|
|||
<div class="grid half v-center">
|
||||
<div>
|
||||
<h1 class="list-heading">{{ $message ?? trans('errors.404_page_not_found') }}</h1>
|
||||
<h5>{{ trans('errors.sorry_page_not_found') }}</h5>
|
||||
<p>{{ trans('errors.sorry_page_not_found_permission_warning') }}</p>
|
||||
<h5>{{ $subtitle ?? trans('errors.sorry_page_not_found') }}</h5>
|
||||
<p>{{ $details ?? trans('errors.sorry_page_not_found_permission_warning') }}</p>
|
||||
</div>
|
||||
<div class="text-right">
|
||||
@if(!signedInUser())
|
||||
|
|
|
@ -254,4 +254,4 @@ Route::post('/password/email', 'Auth\ForgotPasswordController@sendResetLinkEmail
|
|||
Route::get('/password/reset/{token}', 'Auth\ResetPasswordController@showResetForm');
|
||||
Route::post('/password/reset', 'Auth\ResetPasswordController@reset');
|
||||
|
||||
Route::fallback('HomeController@getNotFound');
|
||||
Route::fallback('HomeController@getNotFound')->name('fallback');
|
|
@ -38,4 +38,11 @@ class ErrorTest extends TestCase
|
|||
|
||||
$this->assertCount(1, $handler->getRecords());
|
||||
}
|
||||
|
||||
public function test_access_to_non_existing_image_location_provides_404_response()
|
||||
{
|
||||
$resp = $this->actingAs($this->getViewer())->get('/uploads/images/gallery/2021-05/anonexistingimage.png');
|
||||
$resp->assertStatus(404);
|
||||
$resp->assertSeeText('Image Not Found');
|
||||
}
|
||||
}
|
Loading…
Reference in a new issue