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

@hoangzinh

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:

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?

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

@muttmuure

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.

@muttmuure

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!

@c0ffincolors

📣 @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:

Make sure you've read and understood the contributing guidelines.
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.
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.
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>

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)

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|_|)`((?:`)*)(?!`)(.*?\S+?.*?)(?<!`)((?:`)*)(`)(\B|_|)(?!`|[^<]*<\/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|_|)`((?:`)*)(?!`)(.+?)(?<!`)((?:`)*)(`)(\B|_|)(?!`|[^<]*<\/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.

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

@muttmuure

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

Will wait until we need an external contributor

Not overdue

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

@anmurali

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.

@deetergp

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

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

@anmurali

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

@deetergp

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

@anmurali

cc @anmurali on payment

@deetergp

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

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

Paid.

Navigation Menu

Repository navigation

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

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Proposal

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

What is the root cause of that problem?

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

38 remaining items

BugZero Checklist:

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Aug 27, 2024

dangrous commented on Aug 27, 2024

melvin-bot commented on Aug 27, 2024

Nodebrute commented on Aug 27, 2024

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

tsa321 commented on Aug 28, 2024

Proposal

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

What is the root cause of that problem?

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

dangrous commented on Aug 30, 2024

melvin-bot commented on Aug 30, 2024

muttmuure commented on Sep 1, 2024

muttmuure commented on Sep 3, 2024

dangrous commented on Sep 3, 2024

38 remaining items

melvin-bot commented on Oct 8, 2024

melvin-bot commented on Oct 15, 2024

hoangzinh commented on Oct 16, 2024

BugZero Checklist:

hoangzinh commented on Oct 16, 2024

melvin-bot commented on Oct 23, 2024

hoangzinh commented on Oct 24, 2024

melvin-bot commented on Oct 31, 2024

hoangzinh commented on Nov 1, 2024

anmurali commented on Nov 5, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions