[$500] Web - Task - Task title starting with “<<<test” and the word “test” is not displayed in the chat.

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.1.5
Reproducible in staging?: y
Reproducible in production?: y
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: Applause - Internal Team
Slack conversation:

Action Performed:

1. Open a chat.
2. Go to the "Assign task" section.
3. Create a task by entering a title with the format “<<<Test.”
4. Now, go back to the chat.

Expected Result:

The word "Test" should be displayed in the chat if the title starts with “<<<”.

Actual Result:

Task title starting with “<<<test,” and the word “test” is not displayed in the chat.

Workaround:

Unknown

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

Bug6285413_1700569102911.2023-11-21_17-09-07.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0158ca9e1d4c5abb0b
• Upwork Job ID: 1726987274682650624
• Last Price Increase: 2023-11-21
• Automatic offers:
• FitseTLT | Contributor | 27887201

[melvin-bot]

Triggered auto assignment to @MitchExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[FitseTLT]

Proposal

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

Task title starting with “<<<test” and the word “test” is not displayed in the chat.

What is the root cause of that problem?

We are not escaping the task title here

App/src/components/ReportActionItem/TaskPreview.js

Lines 91 to 94 in a722235

	const htmlForTaskPreview =
	taskAssignee && taskAssigneeAccountID !== 0
	? `<comment><mention-user accountid="${taskAssigneeAccountID}">@${taskAssignee}</mention-user> ${taskTitle}</comment>`
	: `<comment>${taskTitle}</comment>`;

so it is considering < in the task title as part of the html as we pass it to the html renderer

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

We should escape task title because we are rendering it as html

App/src/components/ReportActionItem/TaskPreview.js

Line 86 in a722235

const taskTitle = props.taskReport.reportName || props.action.childReportName;

 const taskTitle = _.escape(props.taskReport.reportName || props.action.childReportName);

What alternative solutions did you explore? (Optional)

[BhuvaneshPatil]

Proposal

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

When we add title that has <<< at the starting doesn't show full title

What is the root cause of that problem?

< characters are part of HTML syntax and when we render the task preview, it parses the string to HTML and as the task name is not valid html, we are seeing the error.

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

We should encode the HTML before rendering it. Example - < becomes &lt

App/src/components/ReportActionItem/TaskPreview.js

Line 127 in 689338c

<RenderHTML html={htmlForTaskPreview} />

By using

    const taskTitle = Str.htmlEncode(props.taskReport.reportName || props.action.childReportName);

We will encode the title, and whenever it's being shown in the TaskPreview will be encoded. And it needs to be encoded in TaskPreview because we are showing it as HTML content using RenderHTML

What alternative solutions did you explore? (Optional)

[vjurcutiu]

This may be a security concern. If there's someone I can DM about this, let me know.

[MitchExpensify]

@vjurcutiu can you let @mountiny know your concern in DM please?

[vjurcutiu]

@MitchExpensify Thanks, I sent him an email. Didn't know GitHub doesn't have a DM function.

[mountiny]

@vjurcutiu ask per Contributing.md instructions, can you please send the email to [email protected]?

Thank you!

[vjurcutiu]

@mountiny Sure

[thesahindia]

@BhuvaneshPatil's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[shubham1206agra]

@aimane-chnaif Can you confirm if this is similar to #21133?

[aimane-chnaif]

Similar but different place. No need to put on hold for that issue

[FitseTLT]

@thesahindia My solution is equivalent to the selected one as _.escape and Str.htmlEncode in this case basically do the same thing with different approaches and also tested it works well and I see we are using _.escape (not Str.htmlEncode) in our code base extensively where we need to escape for rendering as html, I don't get why my solution isn't selected as it came first?
Here is an instance where _.escape is used where we rendered as HTML ,

App/src/components/ArchivedReportFooter.js

Lines 70 to 85 in 7c16666

	oldDisplayName = _.escape(oldDisplayName);
	displayName = _.escape(displayName);
	policyName = _.escape(policyName);
	}
	return (
	<Banner
	containerStyles={[styles.archivedReportFooter]}
	text={props.translate(`reportArchiveReasons.${archiveReason}`, {
	displayName: `<strong>${displayName}</strong>`,
	oldDisplayName: `<strong>${oldDisplayName}</strong>`,
	policyName: `<strong>${policyName}</strong>`,
	})}
	shouldRenderHTML={shouldRenderHTML}
	shouldShowIcon
	/>

@blimpich @MitchExpensify please reconsider my proposal and give me your feedback.

[blimpich]

@thesahindia can you comment on why you chose the 2nd proposal over the first?

Looking into this, there are some differences between _.escape() and Str.htmlEncode(). Str.htmlEncode() comes from Expensify's own library and relies on a combination of jquery's .html() and html-entities' encode().

Both solutions are valid I think.

[thesahindia]

The first proposal originally suggested changes at

App/src/components/ReportActionItem/TaskPreview.js

Lines 91 to 94 in a722235

	const htmlForTaskPreview =
	taskAssignee && taskAssigneeAccountID !== 0
	? `<comment><mention-user accountid="${taskAssigneeAccountID}">@${taskAssignee}</mention-user> ${taskTitle}</comment>`
	: `<comment>${taskTitle}</comment>`;

it was edited multiple times. Both proposal were added at the same time. @BhuvaneshPatil also edited their proposal but it was before the recent changes that @FitseTLT made (which were 3 days ago). So I chose @BhuvaneshPatil's proposal because it suggested the change at taskTitle before @FitseTLT

[FitseTLT]

@thesahindia @blimpich But the basic idea is just escaping taskTitle that's what we should focus on I did comment that specific lines of code because I wanted to focus on where the taskTitle is added to the Html which we render with HtmlRender. This is totally unfair We should be focusing on RCA and the basic idea, not minute details. I don't know!!! @blimpich please consider the situation.

[thesahindia]

When multiple people post the same basic solution at the same time then we also consider such details.

[FitseTLT]

But still if you consider the first comment (non-updated one) the chosen proposal doesn't even mention task title line of code which you claimed was the core of your decision. It is totally unfair!! Let's just wait for the final decision from @blimpich

[grgia]

Hey @FitseTLT, thank you for your proposal! I've looked through the discussion, and given the similarity between the two proposals, I can see why it might be frustrating not to have your proposal chosen. We do our best to be as fair as possible in these situations, and I'm sure that @thesahindia considered that when they made their decision. That said, it's best to try to respect everyone involved throughout this process, as we're all here to help.

Because of the timing and similarity between proposals/edits, how about we choose the first posted proposal for this issue? Does that sound fair @FitseTLT @BhuvaneshPatil @thesahindia?

For the proposal that is not chosen, I am happy to help either of you find another issue to work on.

[BhuvaneshPatil]

@grgia Thank you for stepping in. @thesahindia Please select the @FitseTLT 's proposal as that came before mine. Really appreciate that you spent time on reviewing through changes.❤️
Good luck @FitseTLT

[thesahindia]

Cool cool! I have no issues with @FitseTLT. Let's assign them.

Appreciation for the good spirit 😄

[melvin-bot]

📣 @thesahindia Please request via NewDot manual requests for the Reviewer role ($500)

[melvin-bot]

📣 @FitseTLT 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[FitseTLT]

@MitchExpensify the automation didn't work in this case but the linked pr was deployed to production on Dec 14 so payment is due today. Just a heads up 👍 .

[MitchExpensify]

Payment summary:

$500 C+: @thesahindia $500 (NewDot)
$500 C: @FitseTLT
Reporting - N/A

[MitchExpensify]

Paid and contract ended

[JmillsExpensify]

$500 payment approved for @thesahindia based on comment above.