[$250] IOS - Compose Box - App crashes when pasting a text with an image

IuliiaHerets commented 2 days ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: 9.0.61-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y Issue was found when executing this PR: https://github.com/Expensify/App/pull/51855 Email or phone of affected tester (no customers): nathanmulugetatesting+2060@gmail.com Issue reported by: Applause Internal Team

Action Performed:

Open IOS app
Open any chat and paste this text inside of the compose box _test
Try to send the message or go back

Expected Result:

The text with the image gets sent

Actual Result:

App freezes at first and crashes after a while

Workaround:

Unknown

Platforms:

[ ] Android: Standalone
[ ] Android: HybridApp
[ ] Android: mWeb Chrome
[x] iOS: Standalone
[x] iOS: HybridApp
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/27701433-691f-49fe-b122-aec2738e57e5

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021856775016601889974
Upwork Job ID: 1856775016601889974
Last Price Increase: 2024-11-13

Issue Owner

Current Issue Owner: @ntdiary

melvin-bot[bot] commented 2 days ago

Triggered auto assignment to @garrettmknight (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

garrettmknight commented 2 days ago

Commented on the offending issue.

melvin-bot[bot] commented 2 days ago

Job added to Upwork: https://www.upwork.com/jobs/~021856775016601889974

melvin-bot[bot] commented 2 days ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @ntdiary (External)

garrettmknight commented 2 days ago

@ntdiary can you take a look at this PR and see if it's causing the problem?

badeggg commented 1 day ago

@IuliiaHerets Can you please comment the exact text that was pasted inside the compose box?

ntdiary commented 1 day ago

@ntdiary can you take a look at this PR and see if it's causing the problem?

@garrettmknight, I think this issue wasn't introduced by that PR. The problem is likely due to performance issues with our input component when parsing url in the text, which causes the app to become unresponsive. This is likely another regular expression catastrophic backtracking problem, similar to #34324 😂

test video:

https://github.com/user-attachments/assets/42caa88d-ebac-4eec-b066-4f86a0f27942

the origin text in the OP: ![test](https://camo.githubusercontent.com/4848d0f965f332077b77a1a0488c3e66b4769032104f4de6890bae218b4add8d/68747470733a2f2f70696373756d2e70686f746f732f69642f313036372f3230302f333030)_test cc @badeggg

garrettmknight commented 1 day ago

Thanks @ntdiary - we'll solve here then!

IuliiaHerets commented 1 day ago

@badeggg Posted the exact text that was pasted inside the compose box. It is the same as @ntdiary mentioned ![test](https://camo.githubusercontent.com/4848d0f965f332077b77a1a0488c3e66b4769032104f4de6890bae218b4add8d/68747470733a2f2f70696373756d2e70686f746f732f69642f313036372f3230302f333030 )_test

badeggg commented 18 hours ago

The root cause of this proposal maybe not right, I am investigating...

~Proposal~

Please re-state the problem that we are trying to solve in this issue.

App stalls when pasting specific text inside the compose box.

What is the root cause of that problem?

We are using few huge regexp when parse the text. Check how big it is, this is the regexp we are using to check image url:

Similar regexp size with 'video' and 'link'. Huge regexp is consuming huge executing resource, which cause the stall.

Relate codes:

regexp of link/image/video
most regexp parts are coming from file Url.js
the huge part is coming from tlds, to be honest, this tlds file does not really make sense to me

Why only happen in iOS

Web application works fine on web, I guess it's because web is javascript oriented which has a way better js runtime and hence works fine.
While in iOS, this piece of code is running in hermes runtime, which has limit resource.
For android, this piece of code is running in hermes runtime, which has limit resource too. The stall should happen on android too, though I didn't check it.

What changes do you think we should make in order to solve the problem?

Though I don't really agree the existence of file tlds.jsx, for the sake of minimal change principle, I suggest change regexp of link/image/video to:

const MARKDOWN_LINK_REGEX = new RegExp(`\\[([^\\][]*(?:\\[[^\\][]*][^\\][]*)*)]\\(${UrlPatterns.LOOSE_URL_REGEX}\\)(?![^<]*(<\\/pre>|<\\/code>))`, 'gi');
const MARKDOWN_IMAGE_REGEX = new RegExp(`\\!(?:\\[([^\\][]*(?:\\[[^\\][]*][^\\][]*)*)])?\\(${UrlPatterns.LOOSE_URL_REGEX}\\)(?![^<]*(<\\/pre>|<\\/code>))`, 'gi');
const MARKDOWN_VIDEO_REGEX = new RegExp(`\\!(?:\\[([^\\][]*(?:\\[[^\\][]*][^\\][]*)*)])?\\(((${UrlPatterns.LOOSE_URL_REGEX})\\.(?:${Constants.CONST.VIDEO_EXTENSIONS.join('|')}))\\)(?![^<]*(<\\/pre>|<\\/code>))`, 'gi');

That is saying to replace UrlPatterns.MARKDOWN_URL_REGEX with UrlPatterns.LOOSE_URL_REGEX, which has a reasonable regexp part.

What alternative solutions did you explore? (Optional)

N/A

Expensify / App