[HOLD for payment 2025-01-18] [$250] Web - Workspace - HTML characters are incorrectly shown as workspace name

[vincdargento]

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: v9.0.77-6
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): N/A
Issue reported by: Applause Internal Team

Action Performed:

1. Navigate to staging.new.expensify.com
2. Go to workspace settings - profile
3. Rename the workspace as "Hello"
4. Navigate to admin room
5. Note the updated workspace name
6. Go to workspace settings and delete the workspace
7. Navigate to admin room
8. Now note correct workspace name is shown

Expected Result:

Code based characters name must be updated as workspace name in admins room.

Actual Result:

Inconsistency in workspace display name. Instead of html code based character name, "hello" is displayed when workspace name is altered. Hello is incorrectly shown as workspace name in admins room when workspace is renamed.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

bug.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021873881354318386175
• Upwork Job ID: 1873881354318386175
• Last Price Increase: 2024-12-30
• Automatic offers:
• suneox | Reviewer | 105537897
• truph01 | Contributor | 105537898

Issue Owner

Current Issue Owner: @mallenexpensify

[melvin-bot]

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

[Tony-MK]

Proposal

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

Web - Workspace - HTML characters are incorrectly shown as workspace name

What is the root cause of that problem?

The message has the & character, which is interpreted as a hexadecimal in HTML.

App/src/pages/home/report/PureReportActionItem.tsx

Lines 820 to 821 in 2458635

	} else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_NAME) {
	children = <ReportActionItemBasicMessage message={ReportUtils.getWorkspaceNameUpdatedMessage(action)} />;

Passing the message without converting to Str.htmlDecode will convert the hexadecimal to its respective ASCII.

App/src/pages/home/report/ReportActionItemBasicMessage.tsx

Line 16 in 2458635

<Text style={[styles.chatItemMessage, styles.colorMuted]}>{Str.htmlDecode(message)}</Text>

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

We should use Str.safeEscape or Str.htmlEncode to convert the first before decoding it either in ReportActionItemBasicMessage or PureReportActionItem to target only actions named CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_NAME.

// Either In ReportActionItemBasicMessage.tsx
<Text style={[styles.chatItemMessage, styles.colorMuted]}>
 {Str.htmlDecode(Str.safeEscape(message))} // Or Str.htmlDecode(Str.htmlEncode(message))
</Text> 

// Or In PureReportActionItem.tsx 
children = <ReportActionItemBasicMessage message={
 Str.safeEscape(ReportUtils.getWorkspaceNameUpdatedMessage(action))
} />;

Test Screenshot

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We can create random actions with symbols that HTML decodes, such as the & character, and then, test if the rendered message in ReportActionItemBasicMessage is rendered as intended.

What alternative solutions did you explore? (Optional)

Create a prop called something like isText for the ReportActionItemBasicMessage component, then use it to create a condition to prevent message from being decoded as HTML.

App/src/pages/home/report/ReportActionItemBasicMessage.tsx

Line 16 in 2458635

<Text style={[styles.chatItemMessage, styles.colorMuted]}>{Str.htmlDecode(message)}</Text>

<Text style={[styles.chatItemMessage, styles.colorMuted]}>
 {{isText? message : Str.htmlDecode(message)}
</Text> 

[truph01]

Proposal

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

• Inconsistency in workspace display name. Instead of html code based character name, "hello" is displayed when workspace name is altered. Hello is incorrectly shown as workspace name in admins room when workspace is renamed.

What is the root cause of that problem?

• When we save a new policy name, we simply send the raw text to BE without encoding it, similar to how we handle updates to the description. In the case of updating the description, we encode the text::

App/src/libs/actions/Policy/Policy.ts

Line 1344 in a088878

const parsedDescription = ReportUtils.getParsedComment(description);

For instance, when a user types hello for a policy name, it sends hello to BE. However, for a policy description, it sends the encoded form hell& to BE.

• When displaying the policy change log message, we always decode the message:

App/src/components/LHNOptionsList/OptionRowLHN.tsx

Line 303 in a088878

{Parser.htmlToText(optionItem.alternateText)}

App/src/pages/home/report/ReportActionItemBasicMessage.tsx

Line 16 in 2458635

<Text style={[styles.chatItemMessage, styles.colorMuted]}>{Str.htmlDecode(message)}</Text>

Thus, hello will be decoded to hello. This is why we encounter this bug.

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

• We can address the issue by encoding the policy's name before sending it to the backend, similar to how we handle encoding when updating the policy's description. While this solution resolves the immediate issue, it requires extensive adjustments across various components to ensure the policy's name is consistently displayed. Alternatively, we can encode the source of the policy's name directly on the frontend, streamlining the process and reducing the number of dependent components. So

App/src/libs/ReportUtils.ts

Line 4872 in a088878

return message;

will be:

    return Str.htmlEncode(message);

• Then, update:

App/src/pages/home/report/ContextMenu/ContextMenuActions.tsx

Line 415 in a088878

Clipboard.setString(ReportUtils.getWorkspaceNameUpdatedMessage(reportAction));

                    Clipboard.setString(Str.htmlDecode(ReportUtils.getWorkspaceNameUpdatedMessage(reportAction)));

to get the correct copy message text.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

• We need to add to test for getWorkspaceNameUpdatedMessage function to make sure it always returns the encoded form of the message. Below are test cases in format input > output:

1. hello > hello.
2. hello > hell&.
3. hell& > hell&

What alternative solutions did you explore? (Optional)

• We can update the LHN and report action directly:

App/src/libs/SidebarUtils.ts

Line 475 in a088878

result.alternateText = ReportUtils.getWorkspaceNameUpdatedMessage(lastAction);

            result.alternateText = Str.htmlEncode(ReportUtils.getWorkspaceNameUpdatedMessage(lastAction));

and

App/src/pages/home/report/PureReportActionItem.tsx

Line 821 in a088878

children = <ReportActionItemBasicMessage message={ReportUtils.getWorkspaceNameUpdatedMessage(action)} />;

            children = <ReportActionItemBasicMessage message={Str.htmlEncode(ReportUtils.getWorkspaceNameUpdatedMessage(action))} />;

[bernhardoj]

Proposal

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

The workspace name is decoded in the system message and LHN last message.

What is the root cause of that problem?

The system message is rendered with ReportActionItemBasicMessage. In the component, we always decode the text.

App/src/pages/home/report/ReportActionItemBasicMessage.tsx

Lines 12 to 20 in 8b7096f

	function ReportActionItemBasicMessage({message, children}: ReportActionItemBasicMessageProps) {
	const styles = useThemeStyles();
	return (
	<View>
	<Text style={[styles.chatItemMessage, styles.colorMuted]}>{Str.htmlDecode(message)}</Text>
	{children}
	</View>
	);
	}

On the LHN, it's also the same. We previously had this issue which is caused because we also always convert the HTML to text.

App/src/components/LHNOptionsList/OptionRowLHN.tsx

Line 303 in 8b7096f

{Parser.htmlToText(optionItem.alternateText)}

The main issue here is that we ALWAYS decode the text.

Copying from my proposal from #43563, the LHN issue started to happen after #40845 is merged. In that PR, we are converting HTML to text, so the HTML code is removed but you can still see the text inside the HTML tag. It's to solve this #40348 and #41141 where the last message text of the chat contains an HTML tag. It was a BE issue because the last message text shouldn't include any HTML tag. The one that includes the HTML tag is the last message HTML.

That caused another issue where if we sent a plain text of HTML tag (<p>test</p>), then the LHN last message will only show test.

For the system message issue, it's started to happen after #34081 where the MARKEDREIMBURSED message.text data also contains HTML entities which is also another BE issue since text should contain plain text, HTML should contain the HTML message.

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

For the LHN solution, I suggest applying the same solution from #43563, that is to remove the conversion here because the BE issue that this code is trying to solve isn't happening anymore. If the QA found a similar issue on a different case, then it should be fixed on the BE, not patching it on the FE.

App/src/components/LHNOptionsList/OptionRowLHN.tsx

Line 303 in 8b7096f

{Parser.htmlToText(optionItem.alternateText)}

For the system message, it's also a BE issue and since we are not showing MARKEDREIMBURSED message anymore, so I suggest removing the conversion too.

App/src/libs/ReportActionsUtils.ts

Lines 665 to 669 in 8b7096f

	// Ignore markedAsReimbursed action here since we're already display message that explains the expense was paid
	// elsewhere in the IOU reportAction
	if (reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.MARKED_REIMBURSED) {
	return false;
	}

App/src/pages/home/report/ReportActionItemBasicMessage.tsx

Line 16 in 8b7096f

<Text style={[styles.chatItemMessage, styles.colorMuted]}>{Str.htmlDecode(message)}</Text>

(if we really need to do the conversion/decoding, then we need to apply it case by case, not to all cases)

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

If we decide to remove the FE patch, I'm not sure if a test is necessary here since we just show the text as it's without doing any modification.

[melvin-bot]

@mallenexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[mallenexpensify]

I watched the vid in the OP twice and still don't understand the bug. Added Needs Reproduction to see if a C+ can reproduce. If so... please provide screenshots (if only so that I have know what I'm missing). Thx

[Tony-MK]

I watched the vid in the OP twice and still don't understand the bug. Added Needs Reproduction to see if a C+ can reproduce. If so... please provide screenshots (if only so that I have know what I'm missing). Thx

You can copy Hell&#x006F as the new workspace name, and the workspace doesn't necessarily need to be archived.

[melvin-bot]

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

[melvin-bot]

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

[mallenexpensify]

Thanks @Tony-MK, I was able to reproduce. Added External

[Salmansulehry]

The issue you're encountering, where editing a name field in your app results in the text being converted to hexadecimal codes (like Hell&#x006F), is almost certainly due to HTML entity encoding happening somewhere in your code.

Here's a breakdown of the problem and the most likely causes:

HTML Entity Encoding: This is a process of converting certain characters that have special meaning in HTML (like <, >, &, ", ') into their corresponding "entities." This prevents them from being interpreted as HTML markup. For example:

< becomes <

becomes >
& becomes &
" becomes "
' becomes ' (or ' in HTML5)
In your case, each character is being converted to its numeric character reference, which is the &#x followed by the hexadecimal representation of the character's Unicode code point. For example, H is Unicode U+0048, so it becomes H.

Where the Encoding Might Be Happening:

Server-Side Encoding: If you're sending the data to a server and then retrieving it, the server-side code (e.g., in Node.js, Python, PHP, etc.) might be encoding the data before sending it back to the client. This is a common security practice to prevent Cross-Site Scripting (XSS) vulnerabilities.

Incorrectly Using dangerouslySetInnerHTML: If you're using dangerouslySetInnerHTML in your React code, and the content you're setting contains HTML entities, React won't automatically decode them. This is because dangerouslySetInnerHTML bypasses React's normal rendering process and directly sets the HTML content. Only use this if you have absolute trust in the source of the HTML.

A Library or Utility Function: You might be using a third-party library or a utility function in your code that's performing the encoding.

Accidental Double Encoding: It's also possible that the data is being encoded twice, compounding the issue.

[melvin-bot]

📣 @Salmansulehry! 📣
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>

[suneox]

Thanks for all proposals. Currently, this issue only occurs when we decode the updated ws name change log displayed on LHN & BasicMessage. In other places, we show the original ws name. Therefore, @truph01 proposal encode getWorkspaceNameUpdatedMessage before decoding to keep the original ws name LGTM

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

Offer link
Upwork job

[melvin-bot]

📣 @truph01 🎉 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 📖

[truph01]

@suneox PR is ready

[suneox]

@Tony-MK In my comment, I've pointed out that this issue only occurs when we decode the updated WS name change log, and the RCA, solution from selected proposal is different from yours.

1. Your RCA doesn't specify that the issue occurs on LHN or provide a solution for it.

2. Your solution to encode the message in ReportActionItemBasicMessage may introduce regression for other message types.

3. Your solution does not cover the use case where the WS name change log is copied
4. Your test scenarios are not clearly defined.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.83-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: decoding the update workspace name message before decoding #54779

If no regressions arise, payment will be issued on 2025-01-18. 🎊

For reference, here are some details about the assignees on this issue:

• @suneox requires payment automatic offer (Reviewer)
• @truph01 requires payment automatic offer (Contributor)

[melvin-bot]

@suneox @mallenexpensify @suneox The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[suneox]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

  N/A: It isn't an impactful bug

[melvin-bot]

@carlosmiceli, @suneox, @mallenexpensify, @truph01 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mallenexpensify]

Contributor: @truph01 paid $250 via Upwork
Contributor+: @suneox paid $250 via Upwork

I created a test case, to err on the safe side.

• https://github.com/Expensify/Expensify/issues/463225

Thx