App: [HOLD for payment 2023-02-08] [$250] Strip workspace name of html or handle validation php side reported by @kerupuksambel

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open staging.new.expensify.com
  2. Create new workspace
  3. Change the name of the workspace with <b> </b> and save it

Expected Result:

Either the workspace name changed to after HTML-escaping (something like <b> </b> ), or returned error after the HTML tag got stripped and only the space remained

Actual Result:

The workspace name changed to no name

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Web

Version Number: 1.2.21-4 Reproducible in staging?: y Reproducible in production?: y Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/198730117-135e716c-ada7-4069-b153-df931cee5fc1.mp4

https://user-images.githubusercontent.com/43996225/198730150-69afdab6-b896-461c-945c-97c8914b4e39.mp4

Expensify/Expensify Issue URL: Issue reported by: @kerupuksambel Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1666949886613589

View all open jobs on GitHub

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Comments: 78 (63 by maintainers)

Most upvoted comments

I’m ok with that, but I also don’t see the reason to do it this way 😄

Was ooo last week for a family emergency. I can do it

@rushatgabhane may be external as well (client side validation)

What I am saying is that all fields should disallow HTML. I don’t know of any field that should allow it.

I understand. My issue is we will need to compile all the cases, add php checks, and QA appropriately. I think it’ll be cleaner in another issue.

Created the other github: https://github.com/Expensify/App/issues/14610

Sure, if you want.

list all the other cases we want this validation.

What I am saying is that all fields should disallow HTML. I don’t know of any field that should allow it.

Happened to come across this one scanning through the repo. I think I’m in a similar spot. I don’t see why we’d allow tags, though I bet it stems from either inconsistent or a different OldDot philosophy on inputs/form validation, and I don’t think that should necessarily apply to NewDot. Given that they do nothing, I’d just as well say we shouldn’t allow HTML tags.

Awesome. Thanks for the update!

unassigning since internal