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:
- Open staging.new.expensify.com
- Create new workspace
- 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:
Expensify/Expensify Issue URL: Issue reported by: @kerupuksambel Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1666949886613589
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Comments: 78 (63 by maintainers)
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)
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.
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