Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Take a deeper look at Template Description restrictions #1156

Open
mwangggg opened this issue Nov 2, 2023 · 1 comment
Open

[Bug] Take a deeper look at Template Description restrictions #1156

mwangggg opened this issue Nov 2, 2023 · 1 comment
Labels
chore Refactor, rename, cleanup, etc. question Further information is requested

Comments

@mwangggg
Copy link
Member

mwangggg commented Nov 2, 2023

          I don't remember much about reviewing this feature originally, but I'm curious why there is any restriction on the character set allowed for this description text anyway. Especially because this is for the dashboard layout templates, which only get stored in the browser's localstorage - the server never even sees them, so there's absolutely no risk that somehow these get interpreted and special characters get used to do nefarious things.

The deepest question here is "what makes a template description invalid, and why?" It seems to me that even a length restriction is arbitrary and unnecessary unless there are technical constraints on how large browsers allow localstorage entries to be. Restricting the character set seems completely unnecessary. At worst, the client code should encode the user's entered text before storing it, then decode it again before displaying it. If there is some concern about the text being interpreted incorrectly by the client application/browser - for example, if the user enters text that can be interpreted as HTML - then there should also be ways to handle this as well, by escaping the text before rendering it (ex. < becomes &gt;)

Anyway - that's getting off on a tangent and out of scope of what you set out to do here. Your updated text is at least an improvement on the current text, so that's fine. I think it would be worth filing a bug about the restrictions on this text field however, and when those restrictions are lifted, these texts would need to be updated again.

Originally posted by @andrewazores in #1155 (comment)

@mwangggg mwangggg added question Further information is requested chore Refactor, rename, cleanup, etc. labels Nov 2, 2023
@andrewazores
Copy link
Member

FWIW the 2.x server also imposes character set restrictions on things like recording names. I think in that case it's probably reasonable to just block things like the path separator character, but otherwise it should be quite freeform. This is something we should keep in mind for 3.0, and any client-side checks should also be relaxed accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactor, rename, cleanup, etc. question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants