awesomemotive / easy-digital-downloads

Sell digital downloads through WordPress
https://easydigitaldownloads.com
GNU General Public License v2.0
862 stars 475 forks source link

Better error reporting for usernames with UPPERCASE letters. #643

Closed adampickering closed 11 years ago

adampickering commented 11 years ago

We need to display a better warning/error message for users who try to use any uppercase letters or strange symbols within there username as currently the message just says: Invalid Username which isn't enough information for the customer to correct the issue and proceed in the checkout.

I'm not sure what it should say but something like, Please do not use uppercase letter in the username. Or Please use all lowercase letters in the username.

Geczy commented 11 years ago

Why should it matter if users have uppercase or lowercase characters? I agree with strange symbols being illegal, though

adampickering commented 11 years ago

If a user enters an username with an Uppercase letter EDD Just says its invalid.. We then get emails from users asking why they can't purchase the digital product, and ask why it says why they get the error of "Invalid Username" kinda useless error reporting and not very descriptive and detailed.

Geczy commented 11 years ago

Gotcha; there should instead be an issue to remove the uppercase invalidation, since even WP allows those.

adampickering commented 11 years ago

I remember asking Pippin about this and he said WP Core doesn't allow Uppercase letters in username.

Geczy commented 11 years ago

They allow it, check yourself on wp-login.php?action=register

pippinsplugins commented 11 years ago

@Geczy It's a WordPress limitation, believe it or not.

I spent a while tracking it down. In EDD, we're using the validate_username() function, http://codex.wordpress.org/Function_Reference/validate_username, which will return false if there are uppercase letters.

Geczy commented 11 years ago

Hm? Validate username returns true even with uppercase letters.

You could always use sanitize_username as well?

pippinsplugins commented 11 years ago

Did you test it? That is the only function we were using when this was first reported.

Now we have an edd_validate_username() function that calls sanitize_user( $username, false ).

validate_username() is basically just this:

return sanitize_user( $username, true );
Geczy commented 11 years ago

Yes I tested it, have you as well? I'm returning true for:

validate_username('geczyGECZY');
pippinsplugins commented 11 years ago

Hmm, interesting. Perhaps it's also a combination of _ or - or other characters.

Regardless of what the exact limitations are, we need to figure out exactly what they are and display error messages accordingly. A lot of users have had problems.

Geczy commented 11 years ago

The following validates true as well:

validate_username('gec_z--__-yG--ASDECZY');

Believe all that would invalidate it is symbols / cyrillic characters. Should really find out exactly what username users are reporting invalid before digging further into this, imo

pippinsplugins commented 11 years ago

I know for sure that at least one user had tried with an underscore and an upper case and he got the error.

On Thursday, December 27, 2012, Matt wrote:

The following validates true as well:

validate_username('gec_z--__-yG--ASDECZY');

Believe all that would invalidate it is symbols / cyrillic characters. Should really find out exactly what username users are reporting invalid before digging further into this, imo

— Reply to this email directly or view it on GitHubhttps://github.com/pippinsplugins/Easy-Digital-Downloads/issues/643#issuecomment-11725005.

Geczy commented 11 years ago

Hm, well that issue can't be reproduced anymore. So not sure what else to do.

I think edd_validate_username() can be done away with, validate_username should be fine.

This issue can be resolved by simply replacing 'invalid username' with:

ERROR: This username is invalid because it uses illegal characters. Please enter a valid username.

(Which is what WP does, using validate_username)

adampickering commented 11 years ago

Try checking out with uppercase letters using PayPal on mintthemes.com it should say that the username is invalid even without symbols.

pippinsplugins commented 11 years ago

@Geczy I added edd_validate_username() in order to set the $strict parameter to false for sanitize_username(). There isn't a good filter in place to do it.

Geczy commented 11 years ago

Why does it have to be false? Wouldn't false allow cyrillic characters and symbols?

pippinsplugins commented 11 years ago

False makes it allow more names in, which (at least at the time) was better than what was happening.

Geczy commented 11 years ago

Hmm..

If $strict is true, only alphanumeric characters plus these: _, space, ., -, *, and @ are returned.

That's what a valid username should be, not !#$%^ etc..So I'm not sure if I'm following?

pippinsplugins commented 11 years ago

This whole thing is weird. I'll see if I can replicate the problems locally and then dig deeper from there.

pippinsplugins commented 11 years ago

Proof. This comes from the WordPress MS User Add screen when trying to add a user with an uppercase name. ![Uploading Screenshot from 2013-01-04 18:30:10.png . . .]()

pippinsplugins commented 11 years ago

Screenshot from 2013-01-04 18:30:10

Geczy commented 11 years ago

Hm that's a MS only restriction. I'm not sure why it exists >,<

pippinsplugins commented 11 years ago

That explains why it happens to Adam and I but not you: we both use MS installs for our main sites.

chriscct7 commented 11 years ago

The official reason that I could finc is since the username is also used as the subdomain within WPMU, it has to be lowercased:

It certainly is a common thought that domains must be in lower case. However, this is not actually true. The standard requires two related things:

  1. The url scheme (e.g. http) must be in lower case. This is strict.
  2. The domain-labels must be treated in a case insensitive way, but may be provided with any mix of upper and lower case.

These conditions are loosely indicated in section 2.1 of http://www.ietf.org/rfc/rfc1738.txt and then made unambiguously clear in the specification on page 18-20.

Therefore, there is no reason this restriction couldn't be lifted

pippinsplugins commented 11 years ago

Dang, thanks @chriscct7

I'd say there are two things we should do then:

  1. Find a way to disable this for the EDD user registration (I might have already achieved this)
  2. See if we can create an appropriate patch for WP core
chriscct7 commented 11 years ago

This has been logged numerous times already on core Trac. I'll go get you some Trac tickets about it.

pippinsplugins commented 11 years ago

Great, let's try to get some traction on them.

chriscct7 commented 11 years ago

17904,12948, changeset 1689,BP2207 and thats just the first couple of minutes. The latest effort this one was tagged "future release" but slipped through the cracks.

chriscct7 commented 11 years ago

I'd just open a new ticket asking why WPMU has these restrictions. Actually,now that I think of it, im going to do just that.

pippinsplugins commented 11 years ago

I've just updated 17904.

chriscct7 commented 11 years ago

I already filed 23124 :P

chriscct7 commented 11 years ago

I think we should not be overriding WP on this. That seems like a bad policy. BuddyPress came to this conclusion as well.

pippinsplugins commented 11 years ago

If we don't override WP, then we have to provide much better errors that explain the exact limitation on usernames, otherwise people just get frustrated when they continually get the "Your username is invalid" error.

chriscct7 commented 11 years ago

I think we should provide better error messages.

On Sun, Jan 6, 2013 at 6:33 PM, Pippin Williamson notifications@github.comwrote:

If we don't override WP, then we have to provide much better errors that explain the exact limitation on usernames, otherwise people just get frustrated when they continually get the "Your username is invalid" error.

— Reply to this email directly or view it on GitHubhttps://github.com/pippinsplugins/Easy-Digital-Downloads/issues/643#issuecomment-11936756.

JJJ commented 7 years ago

Update: https://core.trac.wordpress.org/ticket/17904#comment:63

pippinsplugins commented 7 years ago

Thanks @JJJ!