Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle post rendering errors to avoid bricking #3061

Merged
merged 11 commits into from
Oct 14, 2021
2 changes: 2 additions & 0 deletions js/src/common/models/Post.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ Object.assign(Post.prototype, {

createdAt: Model.attribute('createdAt', Model.transformDate),
user: Model.hasOne('user'),

contentType: Model.attribute('contentType'),
content: Model.attribute('content'),
contentHtml: Model.attribute('contentHtml'),
renderFailed: Model.attribute('renderFailed'),
contentPlain: computed('contentHtml', getPlainContent),

editedAt: Model.attribute('editedAt', Model.transformDate),
Expand Down
1 change: 1 addition & 0 deletions js/src/forum/components/CommentPost.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export default class CommentPost extends Post {
' ' +
classList({
CommentPost: true,
'Post--renderFailed': post.renderFailed(),
'Post--hidden': post.isHidden(),
'Post--edited': post.isEdited(),
revealContent: this.revealContent,
Expand Down
4 changes: 4 additions & 0 deletions less/forum/Post.less
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@
}
}

.Post--renderFailed {
background-color: @control-danger-bg;
}

.Post--hidden {
.Post-header, .Post-header a, .PostUser h3, .PostUser h3 a {
color: @muted-more-color;
Expand Down
1 change: 1 addition & 0 deletions locale/core.yml
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,7 @@ core:
not_found_message: The requested resource was not found.
permission_denied_message: You do not have permission to do that.
rate_limit_exceeded_message: You're going a little too quickly. Please try again in a few seconds.
render_failed_message: Sorry, we encountered an error while displaying this content. If you're a user, please try again later. If you're an administrator, take a look in your Flarum log files for more information.

# These translations are used in the loading indicator component.
loading_indicator:
Expand Down
27 changes: 26 additions & 1 deletion src/Api/Serializer/BasicPostSerializer.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,30 @@

namespace Flarum\Api\Serializer;

use Exception;
use Flarum\Foundation\ErrorHandling\LogReporter;
use Flarum\Post\CommentPost;
use Flarum\Post\Post;
use InvalidArgumentException;
use Symfony\Contracts\Translation\TranslatorInterface;

class BasicPostSerializer extends AbstractSerializer
{
/**
* @var LogReporter
*/
protected $log;

/**
* @var TranslatorInterface
*/
protected $translator;

public function __construct(LogReporter $log, TranslatorInterface $translator)
{
$this->log = $log;
$this->translator = $translator;
}
/**
* {@inheritdoc}
*/
Expand All @@ -41,7 +59,14 @@ protected function getDefaultAttributes($post)
];

if ($post instanceof CommentPost) {
$attributes['contentHtml'] = $post->formatContent($this->request);
try {
$attributes['contentHtml'] = $post->formatContent($this->request);
$attributes['renderFailed'] = false;
} catch (Exception $e) {
$attributes['contentHtml'] = $this->translator->trans('core.lib.error.render_failed_message');
$this->log->report($e);
$attributes['renderFailed'] = true;
}
} else {
$attributes['content'] = $post->content;
}
Expand Down
82 changes: 82 additions & 0 deletions tests/integration/api/posts/ShowTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php

/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/

namespace Flarum\Tests\integration\api\posts;

use Carbon\Carbon;
use Flarum\Testing\integration\RetrievesAuthorizedUsers;
use Flarum\Testing\integration\TestCase;

class ShowTest extends TestCase
{
use RetrievesAuthorizedUsers;

/**
* @inheritDoc
*/
protected function setUp(): void
{
parent::setUp();

$this->prepareDatabase([
'discussions' => [
['id' => 1, 'title' => 'Discussion with post', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'first_post_id' => 1, 'comment_count' => 1, 'is_private' => 0],
],
'posts' => [
['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '<t><p>valid</p></t>'],
['id' => 2, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '<tMALFORMED'],
],
'users' => [
$this->normalUser(),
]
]);
}

/**
* @test
*/
public function properly_formatted_post_rendered_correctly()
{
$response = $this->send(
$this->request('GET', '/api/posts/1', [
'authenticatedAs' => 2,
])
);

$this->assertEquals(200, $response->getStatusCode());

$body = (string) $response->getBody();
$this->assertJson($body);

$data = json_decode($body, true);

$this->assertEquals($data['data']['attributes']['contentHtml'], '<p>valid</p>');
}

/**
* @test
*/
public function malformed_post_caught_by_renderer()
{
$response = $this->send(
$this->request('GET', '/api/posts/2', [
'authenticatedAs' => 2,
])
);

$this->assertEquals(200, $response->getStatusCode());

$body = (string) $response->getBody();
$this->assertJson($body);

$data = json_decode($body, true);

$this->assertEquals("Sorry, we encountered an error while displaying this content. If you're a user, please try again later. If you're an administrator, take a look in your Flarum log files for more information.", $data['data']['attributes']['contentHtml']);
}
}