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

Allow plus (+) character in username #10086

Closed
skroczek opened this issue Jul 3, 2018 · 10 comments
Closed

Allow plus (+) character in username #10086

skroczek opened this issue Jul 3, 2018 · 10 comments
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: users and groups

Comments

@skroczek
Copy link

skroczek commented Jul 3, 2018

Steps to reproduce

  1. Create user with username containing plus char

Expected behaviour

User should be created

Actual behaviour

An error message with text "Only the following characters are allowed in a username: "a-z", "A-Z", "0-9", and "_.@-'"" occured.

@MorrisJobke
Copy link
Member

cc @blizzz @nickvergessen @rullzer Opinion on this?

@nextcloud nextcloud deleted a comment from nextcloud-bot Jul 3, 2018
@nickvergessen
Copy link
Member

causes behaviour problems, because " " is replaced with "+" on urlencode, so we would need to change this everywhere with the full trail of problems.

@MorrisJobke
Copy link
Member

causes behaviour problems, because " " is replaced with "+" on urlencode, so we would need to change this everywhere with the full trail of problems.

Full ack.

@MorrisJobke MorrisJobke added the 0. Needs triage Pending check for reproducibility or if it fits our roadmap label Jul 3, 2018
@skroczek
Copy link
Author

skroczek commented Jul 3, 2018

urlencode encodes " " as +, that's true. But it encodes '+' as '%2B', so I'm not sure if it would be such a big problem. A real problem exists, if you mix urldecode and rawurlencode. In this case, the already allowed white space leads to a problem:

<?php
// Result: [email protected]
echo rawurldecode(urlencode('foo [email protected]'));

All other combinations of rawurldecode and urlencode with '+' and ' ' are fine. And this problem already exists in the current code, so allowing the plus character is not that bad.

@skroczek skroczek closed this as completed Jul 3, 2018
@skroczek skroczek reopened this Jul 3, 2018
@blizzz
Copy link
Member

blizzz commented Jul 3, 2018

👎 No fan of adding strange characters there. It's not supposed to be a user facing value anyhow.

@skroczek
Copy link
Author

skroczek commented Jul 4, 2018

@blizzz I understand, that for commonly used usernames plus must sound as a strange character. But if you allow email addresses as usernames, what you are doing right now, it isn't. Even gmail allows + as a delimiter to tag mail addresses.
But I think it would actually be the best if the characters could be configured. However, I fear that it would take a much greater effort to implement this.

@nickvergessen
Copy link
Member

But I think it would actually be the best if the characters could be configured.

No, that does not help at all. Apps need to know what they have to expect. So they would always have to implement for the biggest result set anyway. The thing is if we all of a sudden allow new characters that might cause undefined big problems for apps.

Counting all the Nops in here I see 4 so far, so I'm closing this for now.

@skroczek
Copy link
Author

skroczek commented Jul 4, 2018

Well, that's a pity. Now I have to think about a workaround 🤔. Maybe next time. ☺️

@nickvergessen
Copy link
Member

Workaround: $uid = str_replace('+', '-', $uid); in the right place.
If you have 2 people with the same email just one using + and the other one - that's bad luck

@Escubaer
Copy link

Some of my users really require this for google emails as user names ... maybe time to reconsider: owncloud/core#36613

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: users and groups
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants