[Payment Due June 21] [$250] Can't open tag with tag name with % special characters #42549

m-natarajan · 2024-05-23T18:33:38Z

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: 1.4.75-0
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: @DylanDylann
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1716460972676239

Action Performed:

Sign into https://staging.new.expensify.com/
Click Account Settings > Workspace > More Features > tags
Create a tag name !@#$%^&(&^%$#@ (it seems to be the `%' that throws the error'
Save the Tag > Click on the tag again - notice the "uh-oh, something went wrong!" error

Expected Result:

The user should access to tag setting page

Actual Result:

App crashes - notice the "uh-oh, something went wrong!" error

Workaround:

unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Recording.17.mp4

2024-05-27_16-04-06.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01e766d47beaa045bb
Upwork Job ID: 1795004446572822528
Last Price Increase: 2024-05-27
Automatic offers:

DylanDylann | Reviewer | 102522628

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @DylanDylann

melvin-bot · 2024-05-23T18:33:47Z

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

abzokhattab · 2024-05-23T20:45:14Z

Proposal

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

app crashes when opening a tag with a name having special characters

What is the root cause of that problem?

when clicking on a tag inside the tags page, the tagName is passed as a route param after being encoded as URI:

App/src/pages/workspace/tags/WorkspaceTagsPage.tsx

Line 161 in b951638

Navigation.navigate(ROUTES.WORKSPACE_TAG_SETTINGS.getRoute(policyID, tag.value));

App/src/ROUTES.ts

Line 677 in 788b3f6

 getRoute: (policyID: string, tagName: string) => `settings/workspaces/${policyID}/tag/${encodeURIComponent(tagName)}` as const, 

then on clicking on navigating to the tag settings, the tagName is parsed back to the original value using decodeURIComponent inside the navigation configs

App/src/libs/Navigation/linkingConfig/config.ts

Lines 433 to 438 in 788b3f6

},

[SCREENS.WORKSPACE.TAG_SETTINGS]: {

path: ROUTES.WORKSPACE_TAG_SETTINGS.route,

parse: {

tagName: (tagName: string) => decodeURIComponent(tagName),

},

finally inside the TagSettingsPage
the route.params.tagName is wrongly decoded again causing the decode function to throw an error:

App/src/pages/workspace/tags/TagSettingsPage.tsx

Lines 47 to 49 in 30b0dc8

 
 const currentPolicyTag = 

 policyTag.tags[decodeURIComponent(route.params.tagName)] ?? Object.values(policyTag.tags ?? {}).find((tag) => tag.previousTagName === decodeURIComponent(route.params.tagName));

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

there is no need to decode the tagName inside the TagSettingsPage since the value is already parsed back to the original value inside the navigation config,

so we should modify this to:

    const currentPolicyTag = policyTag.tags[route.params.tagName] ?? Object.values(policyTag.tags ?? {}).find((tag) => tag.previousTagName === route.params.tagName);

POC

Screen.Recording.2024-05-27.at.10.47.53.AM.mov

DylanDylann · 2024-05-24T07:40:33Z

I reported this bug from here, I would love to take over this issue as C+ if this is external

Christinadobrzyn · 2024-05-27T07:17:37Z

@DylanDylann do you think this might be related? #41414

DylanDylann · 2024-05-27T07:51:43Z

@Christinadobrzyn The original bug can still be reproduced. Please try again with this name: !@#$%^&*(*&^%$#@

DylanDylann · 2024-05-27T07:57:18Z

@Christinadobrzyn I can't reproduce your bug here. Could you copy exactly 2 tag names used in your video?

Christinadobrzyn · 2024-05-27T08:06:28Z

Ah thanks @DylanDylann - I can reproduce with !@#$%^&*(*&^%$#@ (I think it's just the % that's causing the error). Updated the OP. Do you think this can be external? Do you think this is related - #41414 - seems to be the same error

DylanDylann · 2024-05-27T08:08:21Z

I checked this bug before reporting it. This is a bug from the front end. Let's make it external

melvin-bot · 2024-05-27T08:09:46Z

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

melvin-bot · 2024-05-27T08:09:50Z

Current assignee @DylanDylann is eligible for the External assigner, not assigning anyone new.

DylanDylann · 2024-05-27T08:11:29Z

Do you think this is related - #41414 - seems to be the same error

@Christinadobrzyn This is different from #41414

ShridharGoel · 2024-05-27T08:39:31Z

Proposal

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

Can't open tag name and can't edit tag name with special characters like %.
Also, can't edit any category name to add a special character.

What is the root cause of that problem?

Encoding is being done while creating the route:

App/src/ROUTES.ts

Lines 674 to 681 in 525ad6f

 WORKSPACE_TAG_EDIT: { 

 route: 'settings/workspace/:policyID/tag/:tagName/edit', 

 getRoute: (policyID: string, tagName: string) => `settings/workspace/${policyID}/tag/${encodeURIComponent(tagName)}/edit` as const, 

 }, 

 WORKSPACE_TAG_SETTINGS: { 

 route: 'settings/workspaces/:policyID/tag/:tagName', 

 getRoute: (policyID: string, tagName: string) => `settings/workspaces/${policyID}/tag/${encodeURIComponent(tagName)}` as const, 

 },

Then, decoding is being done:

App/src/libs/Navigation/linkingConfig/config.ts

Lines 427 to 438 in 525ad6f

 [SCREENS.WORKSPACE.TAG_EDIT]: { 

 path: ROUTES.WORKSPACE_TAG_EDIT.route, 

 parse: { 

 tagName: (tagName: string) => decodeURIComponent(tagName), 

 }, 

 }, 

 [SCREENS.WORKSPACE.TAG_SETTINGS]: { 

 path: ROUTES.WORKSPACE_TAG_SETTINGS.route, 

 parse: { 

 tagName: (tagName: string) => decodeURIComponent(tagName), 

 }, 

 },

But, in the TagSettingsPage, decoding is being done again which leads to issue of URI malformed exception because the tagName is already decoded.

App/src/pages/workspace/tags/TagSettingsPage.tsx

Lines 48 to 49 in 525ad6f

 const currentPolicyTag = 

 policyTag.tags[decodeURIComponent(route.params.tagName)] ?? Object.values(policyTag.tags ?? {}).find((tag) => tag.previousTagName === decodeURIComponent(route.params.tagName));

Also, the same issue happens when editing a category name to use special characters because optimistic data, success data and failure data use the decode method on already decoded category name.

App/src/libs/actions/Policy.ts

Line 3351 in 525ad6f

unencodedName: decodeURIComponent(policyCategory.newName),

App/src/libs/actions/Policy.ts

Line 3370 in 525ad6f

unencodedName: decodeURIComponent(policyCategory.newName),

App/src/libs/actions/Policy.ts

Line 3389 in 525ad6f

unencodedName: decodeURIComponent(policyCategory.oldName),

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

Update to remove decodeURIComponent from the below places:

const currentPolicyTag =
        policyTag.tags[route.params.tagName] ?? Object.values(policyTag.tags ?? {}).find((tag) => tag.previousTagName === route.params.tagName);

function renamePolicyCategory(policyID: string, policyCategory: {oldName: string; newName: string}) {
    const policyCategoryToUpdate = allPolicyCategories?.[`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`]?.[policyCategory.oldName] ?? {};

    const onyxData: OnyxData = {
        optimisticData: [
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`,
                value: {
                    [policyCategory.oldName]: null,
                    [policyCategory.newName]: {
                        ...policyCategoryToUpdate,
                        name: policyCategory.newName,
-                       unencodedName: decodeURIComponent(policyCategory.newName),
+                      unencodedName: policyCategory.newName,
                        pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
                        pendingFields: {
                            name: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
                        },
                        previousCategoryName: policyCategory.oldName,
                    },
                },
            },
        ],
        successData: [
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`,
                value: {
                    [policyCategory.oldName]: null,
                    [policyCategory.newName]: {
                        ...policyCategoryToUpdate,
                        name: policyCategory.newName,
-                      unencodedName: decodeURIComponent(policyCategory.newName),
+                      unencodedName: policyCategory.newName,
                        errors: null,
                        pendingAction: null,
                        pendingFields: {
                            name: null,
                        },
                    },
                },
            },
        ],
        failureData: [
            {
                onyxMethod: Onyx.METHOD.MERGE,
                key: `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`,
                value: {
                    [policyCategory.newName]: null,
                    [policyCategory.oldName]: {
                        ...policyCategoryToUpdate,
                        name: policyCategory.oldName,
-                      unencodedName: decodeURIComponent(policyCategory.oldName),
+                      unencodedName: policyCategory.oldName,
                        errors: ErrorUtils.getMicroSecondOnyxError('workspace.categories.updateFailureMessage'),
                        pendingAction: null,
                    },
                },
            },
        ],
    };

    const parameters = {
        policyID,
        categories: JSON.stringify({[policyCategory.oldName]: policyCategory.newName}),
    };

    API.write(WRITE_COMMANDS.RENAME_WORKSPACE_CATEGORY, parameters, onyxData);
}

truph01 · 2024-05-27T12:12:08Z

Proposal

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

App crashes - notice the "uh-oh, something went wrong!" error.
We have the same problem when adding a special character and saving the category.

What is the root cause of that problem?

The tagName was decoded here

App/src/libs/Navigation/linkingConfig/config.ts

Line 436 in 525ad6f

tagName: (tagName: string) => decodeURIComponent(tagName),

, but then we try to decode it again here, leading to the URI malformed error
We try to save the undecodedName in

App/src/libs/actions/Policy.ts

Line 3351 in 525ad6f

unencodedName: decodeURIComponent(policyCategory.newName),

by using decodeURIComponent, however the URI was already decoded so causing the same error

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

Update

App/src/pages/workspace/tags/TagSettingsPage.tsx

Line 49 in 525ad6f

policyTag.tags[decodeURIComponent(route.params.tagName)] ?? Object.values(policyTag.tags ?? {}).find((tag) => tag.previousTagName === decodeURIComponent(route.params.tagName));

to remove the usage of decodeURIComponent
The unencodedName is legacy and unused, we should remove it in

App/src/libs/actions/Policy.ts

Line 3351 in 525ad6f

unencodedName: decodeURIComponent(policyCategory.newName),

App/src/libs/actions/Policy.ts

Line 3370 in 525ad6f

unencodedName: decodeURIComponent(policyCategory.newName),

App/src/libs/actions/Policy.ts

Line 3389 in 525ad6f

unencodedName: decodeURIComponent(policyCategory.oldName),

And remove it in the type object and tests too. This will help save the app's Onyx storage, and help us get rid of unused tech debt.

What alternative solutions did you explore? (Optional)

NA

Christinadobrzyn · 2024-05-28T05:02:29Z

Awesome - we have some proposals to review when you have a moment @DylanDylann!

DylanDylann · 2024-05-28T05:04:31Z

Sure, I will review proposals by the end of today or tomorrow

truph01 · 2024-05-28T10:21:35Z

I updated the proposal a bit to fix a malformed permalink

DylanDylann · 2024-05-29T02:46:46Z

All proposals work well. I prefer @truph01's proposal because they also address the bug on the category page with an appropriate solution

🎀 👀 🎀 C+ reviewed

melvin-bot · 2024-05-29T02:46:58Z

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

ShridharGoel · 2024-05-29T06:02:57Z

@DylanDylann Is it sure that unencodedName needs to be removed?

truph01 · 2024-05-29T06:36:59Z

@ShridharGoel It's unused in the codebase so there's no reason to keep it. The back-end also doesn't return that field, it just exists in optimistic data.

Christinadobrzyn · 2024-06-12T08:28:05Z

Ah okay, let me see if I can get someone else to review the PR. Asking the team - https://expensify.slack.com/archives/C066HJM2CAZ/p1718180871595969

Christinadobrzyn · 2024-06-13T01:39:08Z

Just a heads up - I'm going to be ooo until June 24th so going to assign a teammate to watch this while I'm away

@JmillsExpensify you're perfect for this! we're trying to find an engineer to review the PR while @lakchote is ooo. Could you help us find a volunteer?

melvin-bot · 2024-06-13T01:39:35Z

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

DylanDylann · 2024-06-13T02:55:48Z

The PR is merged. Waiting for deploying to production

DylanDylann · 2024-06-19T04:51:26Z

@JmillsExpensify The PR is deployed to production 4 days ago. Please update the status of this issue

mvtglobally · 2024-06-20T14:20:58Z

Issue not reproducible during KI retests. (First week)

Christinadobrzyn · 2024-06-24T23:05:07Z

Looks like the PR was deployed into production on June 14 so the 7 day hold ended on June 21st. This is due for payment. correct? @DylanDylann

Payouts due:

Contributor: $250 @truph01 (can you please accept our offer - https://www.upwork.com/nx/wm/offer/102802999)
Contributor+: $250 @DylanDylann (paid in upwork- https://www.upwork.com/nx/wm/workroom/37426723/details)

Upwork job is here.

@DylanDylann do we need a regression test for this?

Christinadobrzyn · 2024-06-26T15:11:43Z

@truph01 can you please accept the Upwork offer - #42549 (comment)

@DylanDylann do we need a regression test for this?

DylanDylann · 2024-06-26T15:14:34Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@DylanDylann] The PR that introduced the bug has been identified. Link to the PR: NA
[@DylanDylann] 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: NA
[@DylanDylann] 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: NA
[@DylanDylann] Determine if we should create a regression test for this bug. Yes
[@DylanDylann] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

Open app
Click Account Settings > Workspace > More Features > tags
Create a tag name !@#$%^&(&^%$#@ (it seems to be the `%' that throws the error'
Save the Tag > Click on the tag again - notice the "uh-oh, something went wrong!" error
Observe that the user should access to tag setting page

Do we agree 👍 or 👎

Christinadobrzyn · 2024-06-27T20:18:23Z

Regression test - https://github.com/Expensify/Expensify/issues/408381

Waiting on finalising payment - #42549 (comment)

Christinadobrzyn · 2024-06-28T20:27:48Z

This is done other than the payment to @truph01.

Moving this to weekly to wait for @truph01 to accept offer - https://www.upwork.com/nx/wm/offer/102802999

truph01 · 2024-07-01T04:56:33Z

@Christinadobrzyn Offer accepted, sorry for the delay 🙏

Christinadobrzyn · 2024-07-01T16:29:15Z

Thanks @truph01 - paid out based on this payment summary - #42549 (comment)

Closing!

m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels May 23, 2024

melvin-bot bot assigned Christinadobrzyn May 23, 2024

This comment was marked as outdated.

Sign in to view

Christinadobrzyn changed the title ~~Can't open tag with tag name with special characters~~ Can't open tag with tag name with % special characters May 27, 2024

Christinadobrzyn assigned DylanDylann May 27, 2024

Christinadobrzyn added the External Added to denote the issue can be worked on by a contributor label May 27, 2024

melvin-bot bot changed the title ~~Can't open tag with tag name with % special characters~~ [$250] Can't open tag with tag name with % special characters May 27, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label May 27, 2024

melvin-bot bot assigned lakchote May 29, 2024

Christinadobrzyn added Weekly KSv2 and removed Daily KSv2 labels Jun 12, 2024

Christinadobrzyn removed the Bug Something is broken. Auto assigns a BugZero manager. label Jun 13, 2024

Christinadobrzyn removed their assignment Jun 13, 2024

Christinadobrzyn added the Bug Something is broken. Auto assigns a BugZero manager. label Jun 13, 2024

melvin-bot bot assigned JmillsExpensify Jun 13, 2024

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 13, 2024

Christinadobrzyn self-assigned this Jun 13, 2024

Christinadobrzyn changed the title ~~[$250] Can't open tag with tag name with % special characters~~ [Payment Due June 21] [$250] Can't open tag with tag name with % special characters Jun 24, 2024

Christinadobrzyn removed Reviewing Has a PR in review Help Wanted Apply this label when an issue is open to proposals by contributors labels Jun 27, 2024

melvin-bot bot added the Overdue label Jun 28, 2024

Christinadobrzyn added Weekly KSv2 and removed Daily KSv2 labels Jun 28, 2024

melvin-bot bot removed the Overdue label Jun 28, 2024

Christinadobrzyn closed this as completed Jul 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Payment Due June 21] [$250] Can't open tag with tag name with % special characters #42549

[Payment Due June 21] [$250] Can't open tag with tag name with % special characters #42549

m-natarajan commented May 23, 2024 •

edited by Christinadobrzyn

Loading

melvin-bot bot commented May 23, 2024

abzokhattab commented May 23, 2024 •

edited

Loading

DylanDylann commented May 24, 2024

This comment was marked as outdated.

Christinadobrzyn commented May 27, 2024

DylanDylann commented May 27, 2024

DylanDylann commented May 27, 2024

Christinadobrzyn commented May 27, 2024 •

edited

Loading

DylanDylann commented May 27, 2024

melvin-bot bot commented May 27, 2024

melvin-bot bot commented May 27, 2024

DylanDylann commented May 27, 2024

ShridharGoel commented May 27, 2024 •

edited

Loading

truph01 commented May 27, 2024 •

edited

Loading

Christinadobrzyn commented May 28, 2024

DylanDylann commented May 28, 2024

truph01 commented May 28, 2024

DylanDylann commented May 29, 2024 •

edited

Loading

melvin-bot bot commented May 29, 2024

ShridharGoel commented May 29, 2024

truph01 commented May 29, 2024

Christinadobrzyn commented Jun 12, 2024

Christinadobrzyn commented Jun 13, 2024 •

edited

Loading

melvin-bot bot commented Jun 13, 2024

DylanDylann commented Jun 13, 2024

DylanDylann commented Jun 19, 2024

mvtglobally commented Jun 20, 2024

Christinadobrzyn commented Jun 24, 2024 •

edited

Loading

Christinadobrzyn commented Jun 26, 2024

DylanDylann commented Jun 26, 2024

Christinadobrzyn commented Jun 27, 2024

Christinadobrzyn commented Jun 28, 2024

truph01 commented Jul 1, 2024

Christinadobrzyn commented Jul 1, 2024

[Payment Due June 21] [$250] Can't open tag with tag name with % special characters #42549

[Payment Due June 21] [$250] Can't open tag with tag name with % special characters #42549

Comments

m-natarajan commented May 23, 2024 • edited by Christinadobrzyn Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented May 23, 2024

abzokhattab commented May 23, 2024 • edited Loading

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?

POC

DylanDylann commented May 24, 2024

This comment was marked as outdated.

Christinadobrzyn commented May 27, 2024

DylanDylann commented May 27, 2024

DylanDylann commented May 27, 2024

Christinadobrzyn commented May 27, 2024 • edited Loading

DylanDylann commented May 27, 2024

melvin-bot bot commented May 27, 2024

melvin-bot bot commented May 27, 2024

DylanDylann commented May 27, 2024

ShridharGoel commented May 27, 2024 • edited Loading

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?

truph01 commented May 27, 2024 • edited Loading

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)

Christinadobrzyn commented May 28, 2024

DylanDylann commented May 28, 2024

truph01 commented May 28, 2024

DylanDylann commented May 29, 2024 • edited Loading

melvin-bot bot commented May 29, 2024

ShridharGoel commented May 29, 2024

truph01 commented May 29, 2024

Christinadobrzyn commented Jun 12, 2024

Christinadobrzyn commented Jun 13, 2024 • edited Loading

melvin-bot bot commented Jun 13, 2024

DylanDylann commented Jun 13, 2024

DylanDylann commented Jun 19, 2024

mvtglobally commented Jun 20, 2024

Christinadobrzyn commented Jun 24, 2024 • edited Loading

Christinadobrzyn commented Jun 26, 2024

DylanDylann commented Jun 26, 2024

Christinadobrzyn commented Jun 27, 2024

Christinadobrzyn commented Jun 28, 2024

truph01 commented Jul 1, 2024

Christinadobrzyn commented Jul 1, 2024

m-natarajan commented May 23, 2024 •

edited by Christinadobrzyn

Loading

abzokhattab commented May 23, 2024 •

edited

Loading

Christinadobrzyn commented May 27, 2024 •

edited

Loading

ShridharGoel commented May 27, 2024 •

edited

Loading

truph01 commented May 27, 2024 •

edited

Loading

DylanDylann commented May 29, 2024 •

edited

Loading

Christinadobrzyn commented Jun 13, 2024 •

edited

Loading

Christinadobrzyn commented Jun 24, 2024 •

edited

Loading