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

Trim param whitespace by default #1141

Closed
paulcsmith opened this issue May 14, 2020 · 2 comments
Closed

Trim param whitespace by default #1141

paulcsmith opened this issue May 14, 2020 · 2 comments
Labels
feature request A new requested feature / option

Comments

@paulcsmith
Copy link
Member

All kinds of sneaky bugs make it into apps because whitespace is not trimmed. Sometimes it can break layouts, it can cause login issues, or it just looks weird to output text with an extra space

Example bug:

No matter how many passwords they try it will not work, because the user accidentally added a space at the end. This also applies to password where an extra trailing space can mess people up

Prior art

Laravel does this by default: https://laravel.com/docs/7.x/requests#input-trimming-and-normalization

We already treat empty strings as nil, but trimming/stripping should also be the default

Disabling

We should have a setting on Lucky to disable whitespace trimming. It can just be global, but eventually we may want to have a params.get_raw that gets the raw untrimmed value as well. In cases where you only have one weird param that you need to have untrimmed

@wout
Copy link
Contributor

wout commented May 17, 2020

I've been bitten by this too!

I think calling params.get_raw explicitly if you need the values as they are, is the best approach. It's better than to configure it globally, as it can cause confusion when moving between projects. I don't know if there are many use cases for leading and trailing whitespace to stay around. Maybe adding the global flag is something we can add later, once the complaints start pouring in. 😀️

@paulcsmith
Copy link
Member Author

I think you are right @wout! I originally wanted to do global because it was simpler to implement, but I actually think get_raw is as-easy or easier to agree that's the better and more explicit approach!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A new requested feature / option
Projects
None yet
Development

No branches or pull requests

2 participants