Add html input validation in the form component #15315

ctkochan22 · 2023-02-21T06:03:07Z

Details

WAF input rules filter out html/xml tags in most API parameters. Given that we rarely want users to submit html tags as inputs, we have decided to validate all inputs for html. And we will do so in the Form component.

Here is what it would look like:
First/Last Name-

Workspage Settings -> General -

Fixed Issues

$ #14610
PROPOSAL: GH - Slack

Tests

Verify that no errors appear in the JS console

Offline tests

QA Steps

Let's test that it picks up for html tags, but the focus here is to make sure we don't have regressions

WorkspaceNewRoomPage.js
- Create a new workspace room.
- Add a name with html tags test. Verify that there is an error
- Now delete part of it so that it looks like test< and verify that you get an error saying you can only have characters
- Now delete it so that its just text test. Verify that error is gone.
- Select a workspace below that field. And submit and verify that you can create a workspace room
- https://recordit.co/JgPnzb7FZf
CloseAccountPage.js
- Have an account you're willing to close
- Go to Security -> Close Account
- Type in a message with html tags hello . Verify you get an error
- Now remove the html tag, hello
- Add the email and close the account.
- https://recordit.co/64J5ZbsT7i
WorkspaceSettingsPage.js
- Go to Workspace Settings -> General Settings. Input   as the workspace name. Verify that you see an error when you blur the field or submit the form
- Remove the tags, add a valid name, and confirm that workspace name saves successfully
- https://recordit.co/WW3Ne7UKHZ
DisplayName
- Go to settings -> profile. Display Name
- Add a name with a html tag in it Kosuke. Verify that you get an error message
- Remove the tag and verify that you can add a name successfully

PR Author Checklist

Screenshots/Videos

Web

https://recordit.co/0eTSnGSP6r

Mobile Web - Chrome

https://recordit.co/cIOjb7gHc7

Mobile Web - Safari

https://recordit.co/0L0CmlQRJF

Desktop

https://recordit.co/rrvronUa7p

iOS

https://recordit.co/6mxyX6mqLe

Android

https://recordit.co/F8zbIrxmPn

MelvinBot · 2023-02-21T06:04:00Z

@eVoloshchak @tgolen One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

MelvinBot · 2023-02-21T06:05:33Z

Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers!

ctkochan22 · 2023-02-22T20:56:44Z

Waiting on copy, but ready for an initial review. Been testing it pretty heavily

eVoloshchak · 2023-02-23T14:44:48Z

Tests well on all platforms
I think there are cases where this error superseding other errors isn't the best option, for instance on BankAccountManualStep. The error will be Please enter a valid routing number for every symbol or invalid combination of symbols, except for when there is a tag.

This is a small issue though, it will look better with Invalid character error

ctkochan22 · 2023-02-23T17:21:53Z

Yeah that is a good catch @eVoloshchak . I'd say its a rare case in general. If someone putting anything resembling tags where a routing number is supposed to be, they should be notified that that act is not allowed.

The "Please enter valid Routing Number" to me, is a "friendly" message for "friendly" users

ctkochan22 · 2023-02-24T01:12:18Z

Copy updated. Ready for review

tgolen · 2023-02-24T14:58:47Z

src/components/Form.js

        const validationErrors = this.props.validate(values);

+        // Validate the input for html tags. It should superseed any other error
+        _.every(values, (inputValue, inputID) => {
+            if (!inputValue || inputValue.search(/<(.|\n)*?>/g) === -1) {


The CONST.js file has VALIDATE_CODE_REGEX_STRING. Could you please move this regex to the same place and define it there?

tgolen · 2023-02-24T14:59:38Z

src/components/Form.js

+                return;
+            }
+
+            validationErrors[inputID] = this.props.translate('common.error.inputHasHtml');


Let's rename this to common.error.invalidCharacter so that it matches the text

ctkochan22 · 2023-02-24T19:01:04Z

Updated

eVoloshchak · 2023-02-25T12:24:41Z

It seems like it's validating only the first input, on Legal Name page tags aren't allowed only for the first name

Because I'm the author now

tgolen · 2023-03-09T14:28:51Z

@eVoloshchak Looks like this just needs an approved review from you now. Are you able?

tgolen · 2023-03-09T15:11:42Z

Thanks! @marcaaron Could you add a review, please?

src/CONST.js

src/components/Form.js

Co-authored-by: Marc Glasser <marc.aaron.glasser@gmail.com>

tgolen · 2023-03-10T14:39:40Z

Updated

src/components/Form.js

marcaaron · 2023-03-10T19:32:21Z

src/components/Form.js

+        // Validate the input for html tags. It should supercede any other error
+        _.each(values, (inputValue, inputID) => {
+            // Return early if there is no value OR the value is not a string OR there are no HTML characters
+            if (!inputValue || !_.isString(inputValue) || inputValue.search(CONST.VALIDATE_FOR_HTML_TAG_REGEX) === -1)


missing the opening curly brace

Akward :blush

tgolen · 2023-03-10T19:47:17Z

Retested to be sure my curly braces are working

OSBotify · 2023-03-10T20:35:50Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2023-03-13T12:55:33Z

🚀 Deployed to staging by https://github.com/tgolen in version: 1.2.83-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

OSBotify · 2023-03-13T14:44:54Z

🚀 Deployed to staging by https://github.com/tgolen in version: 1.2.83-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2023-03-14T03:00:04Z

🚀 Deployed to production by https://github.com/roryabraham in version: 1.2.83-3 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

Kosuke Tseng added 3 commits February 20, 2023 21:21

validate input on all form inputs

775e74e

add error message

8fb2916

remove html validation in workspace

c9ea4f9

ctkochan22 requested a review from marcaaron February 21, 2023 06:03

ctkochan22 requested a review from a team as a code owner February 21, 2023 06:03

ctkochan22 self-assigned this Feb 21, 2023

melvin-bot bot requested review from eVoloshchak and tgolen and removed request for a team February 21, 2023 06:03

ctkochan22 changed the base branch from master to main February 21, 2023 06:05

Kosuke Tseng added 4 commits February 20, 2023 22:19

fix lint

b51e3c9

lint

9eaacd8

Merge branch 'main' into ckt_form_htmlValidation

df716b1

check if inputValue is empty

cd335bb

ctkochan22 changed the title ~~[WIP] Add html input validation in the form component~~ Add html input validation in the form component Feb 22, 2023

ctkochan22 changed the title ~~Add html input validation in the form component~~ Validate for anything that looks like html/xml tags in the form component Feb 22, 2023

ctkochan22 changed the title ~~Validate for anything that looks like html/xml tags in the form component~~ Add html input validation in the form component Feb 22, 2023

update copy

dc1eeaa

tgolen previously requested changes Feb 24, 2023

View reviewed changes

fix feedback

96cf43f

This was referenced Feb 25, 2023

Escape HTML characters in IOU message #15478

Closed

[$2000] IOU Request message changes & removes some characters after sending. #15026

Closed

eVoloshchak previously approved these changes Mar 9, 2023

View reviewed changes

marcaaron reviewed Mar 9, 2023

View reviewed changes

src/CONST.js Outdated Show resolved Hide resolved

src/components/Form.js Outdated Show resolved Hide resolved

src/components/Form.js Outdated Show resolved Hide resolved

Update src/components/Form.js

6deb44d

Co-authored-by: Marc Glasser <marc.aaron.glasser@gmail.com>

tgolen dismissed eVoloshchak’s stale review via 6deb44d March 10, 2023 14:20

tgolen added 2 commits March 10, 2023 07:33

Merge branch 'main' into ckt_form_htmlValidation

2940ca3

Update comment and only search strings

379acac

tgolen requested a review from marcaaron March 10, 2023 14:39

marcaaron reviewed Mar 10, 2023

View reviewed changes

src/components/Form.js Outdated Show resolved Hide resolved

tgolen added 2 commits March 10, 2023 11:54

Merge branch 'main' into ckt_form_htmlValidation

d36b214

Update error checking logic

1f4e051

tgolen requested a review from marcaaron March 10, 2023 18:56

marcaaron previously approved these changes Mar 10, 2023

View reviewed changes

marcaaron reviewed Mar 10, 2023

View reviewed changes

Fix broken braces

93380e8

tgolen dismissed marcaaron’s stale review via 93380e8 March 10, 2023 19:47

marcaaron approved these changes Mar 10, 2023

View reviewed changes

tgolen merged commit 8998ee7 into main Mar 10, 2023

tgolen deleted the ckt_form_htmlValidation branch March 10, 2023 20:32

OSBotify mentioned this pull request Mar 13, 2023

Deploy Checklist: New Expensify 2023-03-13 #15889

Closed

16 tasks

johnmlee101 mentioned this pull request Mar 14, 2023

[$1000] Trying to send message within angle brackets in the invite new members page doesn’t display in email #15700

Closed

6 tasks

ShogunFire mentioned this pull request Jun 26, 2023

[HOLD for payment 2023-07-10] [$1000] text cannot be wrapped in < > , in welcome message in new room #20874

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add html input validation in the form component #15315

Add html input validation in the form component #15315

ctkochan22 commented Feb 21, 2023 •

edited

Loading

MelvinBot commented Feb 21, 2023

MelvinBot commented Feb 21, 2023

ctkochan22 commented Feb 22, 2023

eVoloshchak commented Feb 23, 2023

ctkochan22 commented Feb 23, 2023

ctkochan22 commented Feb 24, 2023

tgolen Feb 24, 2023

tgolen Feb 24, 2023

ctkochan22 commented Feb 24, 2023

eVoloshchak commented Feb 25, 2023

tgolen commented Mar 9, 2023

tgolen commented Mar 9, 2023

tgolen commented Mar 10, 2023

marcaaron Mar 10, 2023

tgolen Mar 10, 2023

tgolen commented Mar 10, 2023

OSBotify commented Mar 10, 2023

OSBotify commented Mar 13, 2023

OSBotify commented Mar 13, 2023

OSBotify commented Mar 14, 2023

Add html input validation in the form component #15315

Add html input validation in the form component #15315

Conversation

ctkochan22 commented Feb 21, 2023 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

MelvinBot commented Feb 21, 2023

MelvinBot commented Feb 21, 2023

ctkochan22 commented Feb 22, 2023

eVoloshchak commented Feb 23, 2023

ctkochan22 commented Feb 23, 2023

ctkochan22 commented Feb 24, 2023

tgolen Feb 24, 2023

Choose a reason for hiding this comment

tgolen Feb 24, 2023

Choose a reason for hiding this comment

ctkochan22 commented Feb 24, 2023

eVoloshchak commented Feb 25, 2023

tgolen commented Mar 9, 2023

tgolen commented Mar 9, 2023

tgolen commented Mar 10, 2023

marcaaron Mar 10, 2023

Choose a reason for hiding this comment

tgolen Mar 10, 2023

Choose a reason for hiding this comment

tgolen commented Mar 10, 2023

OSBotify commented Mar 10, 2023

OSBotify commented Mar 13, 2023

OSBotify commented Mar 13, 2023

OSBotify commented Mar 14, 2023

ctkochan22 commented Feb 21, 2023 •

edited

Loading