Mattermost-server: MM-28365 PostUtils.formatText crash when formatting text with unicode emoiji

Created on 30 Aug 2020  路  8Comments  路  Source: mattermost/mattermost-server

Summary

Plugin utility function PostUtils.formatText crashes when formatting unicode emoji character (e.g.: 馃殥 ) with {markdown: false} option.

refs: https://github.com/matterpoll/matterpoll/issues/319

Steps to reproduce

  1. Deploy a plugin with calling console.log(PostUtils.formatText('馃殥', { markdown: false })); in webapp
  2. Open mattermost

Expected behavior

Nothing happen in view, and logged 馃殥 in web console

Observed behavior (that appears unintentional)

A white bank screen, and logged error in web console.

text_formatting.tsx:854 Uncaught TypeError: Cannot read property 'hasUnicode' of undefined at text_formatting.tsx:854 at RegExp.[Symbol.replace] (<anonymous>) at String.replace (<anonymous>) at text_formatting.tsx:835 at g (text_formatting.tsx:281) at Object._ [as formatText] (text_formatting.tsx:230) at s.initialize (com.mattermost.plugin-starter-template_63e7ac464064c09d_bundle.js:1) at index.js:135 at HTMLScriptElement.i.onload (index.js:99)

Possible fixes

https://github.com/mattermost/mattermost-webapp/blob/master/utils/text_formatting.tsx#L215
The cause might be in this block. I don't know the details, but I think that replacing in this block cannot handle unicode emoji characters.

Bug ReporOpen

All 8 comments

Thanks for the report! @iomodo or @hanzei are you open to see if you can reproduce the issue and we can create a bug ticket for it?

Good catch @kaakaa :+1: I can reproduce the error and opened https://mattermost.atlassian.net/browse/MM-28365. Are you interested in fixing this bug?

Yes, I would like to resolve this issue.

With some inspection, I found that this issue was introduced by https://github.com/mattermost/mattermost-webapp/pull/4402. #4402 add an argument emojiMap to formatText function for internal invokes. But formatText is also used by plugin developer, and it's hard for plugin developer to create emojiMap.

I understand the concept of #4402 and it makes sense, but I think that the best solution to resolve this is to revert some of #4402 about formatText function. Any other ideas? CC: @nevyangelova @hmhealey

Do plugins have access to the Markdown component? I'd say that would be the best option for them since they wouldn't have to care about emojiMap or any of the other props needed for markdown rendering, and they'll continue to be compatible if we add any additional arguments to it in the future.

I'd be hesitant about reverting part of that PR since it was part of a big cleanup, and I'd rather not reintroduce some of the problems we had before then, especially since that was changed quite a few releases ago. I'd prefer if we could move towards a less complicated way to use Markdown such as the Markdown component.

(At first, I'm sorry that issue links in my previous comment has pointed wrong issue. I talked about https://github.com/mattermost/mattermost-webapp/pull/4402.)

I don't know how to access Markdown component from plugins (, if you mentioned about Markdown coponent in mattermost-webapp), and Developer document also doesn't seem to mention that. And if plugins can access Markdown component, I think that this issue isn't resolved.

Okay, I understand that reverting isn't good way. As an another idea, simple workaround is to check if emojiMap is undefined before emojiMap is used would be a simple workaround. However, the possibility that emojiMap is undefined when invoking formatText from user's plugin remains, so future developers of text_formatting.tsx need to take into account that emojiMap can be undefined.

@kaakaa I agree we can check if emojiMap is 'undefined', @hmhealey what do you think?

Yeah, that should work well enough. I'd be a bit worried that custom emojis in the text won't be rendered without it, but since no one has even reported this crash until now, I assume that wouldn't be a big deal if it is even an issue.

Thanks @nevyangelova and @hmhealey!
I've created PR. https://github.com/mattermost/mattermost-webapp/pull/6495. In my case, custom emoji would not be problem.

Was this page helpful?
0 / 5 - 0 ratings