From 35986a050cf13b04109850443bd2feb85bac4493 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com> Date: Thu, 14 Oct 2021 14:30:18 -0400 Subject: [PATCH] Handle post rendering errors to avoid bricking (#3061) Whether it's due to corrupted content, missing tags, caching issues, or other assorted reasons, post content can't be rendered. Currently, this results in an exception that crashes the entire forum and is hard to debug. Instead, we should log the error and show an indicator message that rendering has failed. Co-authored-by: Sami Mazouz Co-authored-by: David Wheatley --- framework/core/js/src/common/models/Post.js | 2 + .../js/src/forum/components/CommentPost.js | 1 + framework/core/less/forum/Post.less | 4 + framework/core/locale/core.yml | 1 + .../Api/Serializer/BasicPostSerializer.php | 27 +++++- .../tests/integration/api/posts/ShowTest.php | 82 +++++++++++++++++++ 6 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 framework/core/tests/integration/api/posts/ShowTest.php diff --git a/framework/core/js/src/common/models/Post.js b/framework/core/js/src/common/models/Post.js index 5941ffe5e..29a122cb9 100644 --- a/framework/core/js/src/common/models/Post.js +++ b/framework/core/js/src/common/models/Post.js @@ -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), diff --git a/framework/core/js/src/forum/components/CommentPost.js b/framework/core/js/src/forum/components/CommentPost.js index f7433bc0b..730a54969 100644 --- a/framework/core/js/src/forum/components/CommentPost.js +++ b/framework/core/js/src/forum/components/CommentPost.js @@ -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, diff --git a/framework/core/less/forum/Post.less b/framework/core/less/forum/Post.less index e29a2b409..aa6994104 100644 --- a/framework/core/less/forum/Post.less +++ b/framework/core/less/forum/Post.less @@ -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; diff --git a/framework/core/locale/core.yml b/framework/core/locale/core.yml index 40356cdb1..8b7e1c820 100644 --- a/framework/core/locale/core.yml +++ b/framework/core/locale/core.yml @@ -527,6 +527,7 @@ core: payload_too_large_message: The request payload was too large. 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: diff --git a/framework/core/src/Api/Serializer/BasicPostSerializer.php b/framework/core/src/Api/Serializer/BasicPostSerializer.php index f687ca59c..55f04533a 100644 --- a/framework/core/src/Api/Serializer/BasicPostSerializer.php +++ b/framework/core/src/Api/Serializer/BasicPostSerializer.php @@ -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} */ @@ -41,7 +59,14 @@ class BasicPostSerializer extends AbstractSerializer ]; 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; } diff --git a/framework/core/tests/integration/api/posts/ShowTest.php b/framework/core/tests/integration/api/posts/ShowTest.php new file mode 100644 index 000000000..632553af5 --- /dev/null +++ b/framework/core/tests/integration/api/posts/ShowTest.php @@ -0,0 +1,82 @@ +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' => '

valid

'], + ['id' => 2, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => ' [ + $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'], '

valid

'); + } + + /** + * @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']); + } +}