[$250] Incorrect parsing for backticks with a space in between them

[dangrous]

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:
Reproducible in staging?: yes
Reproducible in production?: yes
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation: not Slack but #46917 (comment)

Action Performed:

1. Send the following input:

it uses`;`separated fields because the line number column could have`:`in them ` ` with space

Expected Result:

The input should look like

it uses;separated fields because the line number column could have:in them with space

(The space at the end is in a code block)

Actual Result:

The input looks like

(The space at the end is not in a code block, the backticks are visible)

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Yes

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

See above.

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e928bacaaf9c0339
• Upwork Job ID: 1831034909462647027
• Last Price Increase: 2024-09-03
• Automatic offers:
• hoangzinh | Reviewer | 103821976
• tsa321 | Contributor | 103821978

Issue Owner

Current Issue Owner: @hoangzinh

[melvin-bot]

Triggered auto assignment to @muttmuure (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.

[dangrous]

The parsing is actually wonkier than this on staging/prod so this bug isn't visible yet, but it will be when THAT parsing is fixed by #47838. Making this so I don't forget, but no action at the moment @muttmuure!

[melvin-bot]

📣 @c0ffincolors! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[Nodebrute]

Edited by proposal-police: This proposal was edited at 2024-08-27 16:19:13 UTC.

Proposal

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

Incorrect parsing for backticks with a space in between them

What is the root cause of that problem?

We are checking that there is at least one whitespace character
https://github.com/Expensify/expensify-common/blob/d11466167bc562447d2dfdad624145040153efae/lib/ExpensiMark.ts#L193

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

Remove \S+? from this regex
https://github.com/Expensify/expensify-common/blob/d11466167bc562447d2dfdad624145040153efae/lib/ExpensiMark.ts#L193

Updated regex

                regex: /(\B|_|)&#x60;((?:&#x60;)*)(?!&#x60;)(.*?.*?)(?<!&#x60;)((?:&#x60;)*)(&#x60;)(\B|_|)(?!&#x60;|[^<]*<\/pre>|[^<]*<\/video>)/gm,

Result:

Or

                regex: /(\B|_|)&#x60;((?:&#x60;)*)(?!&#x60;)(?<!&#x60;)((?:&#x60;)*)(&#x60;)(\B|_|)(?!&#x60;|[^<]*<\/pre>|[^<]*<\/video>)/gm,

What alternative solutions did you explore? (Optional)

[tsa321]

Edited by proposal-police: This proposal was edited at 2024-08-28 11:10:18 UTC.

Proposal

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

Incorrect parsing for backticks with a space in between them

What is the root cause of that problem?

The regex for inline code:

https://github.com/Expensify/expensify-common/blob/d11466167bc562447d2dfdad624145040153efae/lib/ExpensiMark.ts#L193

that will be modified into :

/(\B|_|)&#x60;((?:&#x60;)*)(?!&#x60;)(.*?\S+?.*?)(?<!&#x60;)((?:&#x60;)*)(&#x60;)(\B|_|)(?!&#x60;|[^<]*<\/pre>|[^<]*<\/video>)/gm

won't allow space only inside backtick, in the (.*?\S+?.*?) part

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

Modify the regex into:

/(\B|_|)&#x60;((?:&#x60;)*)(?!&#x60;)(.+?)(?<!&#x60;)((?:&#x60;)*)(&#x60;)(\B|_|)(?!&#x60;|[^<]*<\/pre>|[^<]*<\/video>)/gm

We also need to escape the space character as   due to the issue #8720, which is addressed by Expensify/expensify-common#443. Sending an unescaped space or a space-only inlinecode or code block, results in a server error response.

We should add ' ' : ' ' in:

lib/utils.ts#L13

Additionally, in:

lib/utils.ts#L33

we add ' ': ' '

And in:

lib/utils.ts#L43

we add |nbsp|

but this will breaks many regexes. We could adjust the regexes accordingly.

---

Alternatively, we could escape the space in before return of replace function of ExpensiMark:

https://github.com/Expensify/expensify-common/blob/331f53580a4ddd7f1c8e73c7c1494d0bb22962c7/lib/ExpensiMark.ts#L855

Or right before sending the comment:

App/src/libs/actions/Report.ts

Line 455 in ac5ee2e

reportCommentText = reportComment.commentText;

and in here:

App/src/libs/actions/Report.ts

Line 468 in ac5ee2e

reportCommentText = ReportUtils.getParsedComment(text ?? '', {reportID});

or directly in here:

App/src/libs/actions/Report.ts

Line 511 in ac5ee2e

reportComment: reportCommentText,

Or another more appropriate place.

[dangrous]

almost ready to open this up officially, PR is merged but not yet deployed

[melvin-bot]

@muttmuure Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[muttmuure]

Will wait until we need an external contributor

[muttmuure]

Not overdue

[dangrous]

Okay we're ready to go on this one, will make it external

[melvin-bot]

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

[melvin-bot]

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

[hoangzinh]

@tsa321 @Nodebrute Thanks for proposals, everyone. I think both of you pointed out the correct root cause. But both solutions would cause another issue where a user sends backticks with a space.

Steps to reproduce

1. Go to any chats
2. Send a message
3. Expected that it's sent successfully

[tsa321]

@hoangzinh yes, that is way we must convert the space with   like what I mentioned in my proposal

[hoangzinh]

@tsa321 I tried to apply your solution, but still got same error. Can you share your recording and the AddComment API request body in this case? Thank you

[tsa321]

@hoangzinh here is the video:

api_req.mp4

[hoangzinh]

@tsa321 Thanks for the updates. Your proposal looks good to me

Link to proposal #48101 (comment)

🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @deetergp, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[tsa321]

@hoangzinh PR is ready

[melvin-bot]

This issue has not been updated in over 15 days. @deetergp, @hoangzinh, @muttmuure, @tsa321 eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[deetergp]

@muttmuure I think our automations failed to update, but the PR for this issue has been deployed to production.

[dangrous]

I think @muttmuure is still on leave, I'll reapply the label

[melvin-bot]

Triggered auto assignment to @anmurali (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.

[melvin-bot]

@deetergp, @hoangzinh, @anmurali, @tsa321 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[hoangzinh]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR: No offending PR, it's just a follow up issue from this comment [HOLD for Payment 2024-09-09][$250] Wrong parsing when single backticks used in 2 separate places #46917 (comment)
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• Determine if we should create a regression test for this bug. ⛔ I don't think we need, it's already covered in unit tests of Add support for inline code with space only character as content expensify-common#797

[hoangzinh]

not overdue, it's ready for payment cc @anmurali

[melvin-bot]

@deetergp, @hoangzinh, @anmurali, @tsa321 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[hoangzinh]

cc @anmurali on payment

[melvin-bot]

@deetergp, @hoangzinh, @anmurali, @tsa321 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[hoangzinh]

Ping in Slack https://expensify.slack.com/archives/C01GTK53T8Q/p1730457342555099

[anmurali]

Paid.