Broken Gravatar on Trac

[SergeyBiryukov]

See ​#WP29640.

The ticket reporter's username is "let me see...". It's not urlencoded in the profile link, but the link still redirects to the correct URL:

https://profiles.wordpress.org/let me see... → https://profiles.wordpress.org/let-me-see/

However, the Gravatar URL is not redirected and displays a broken image:
//wordpress.org/grav-redirect.php?user=let me see...&s=48

[Otto42]

That URL seems to work for me. I see his gravatar.

[SergeyBiryukov]

Replying to Otto42:

That URL seems to work for me. I see his gravatar.

I see nginx's "403 Forbidden" page when opening the Gravatar URL from ticket description.

[Otto42]

I don't, I get a redirect to ​https://secure.gravatar.com/avatar/ea92088e2c7cd2d71748bd093d576f97?s=48&d=retro

[SergeyBiryukov]

I'm testing with the latest Chrome and Firefox on Windows.

If I had to guess, it might have to do with a dotorg sandbox.

[swissspidy]

I get the same 403 error on OS X.

Works: Safari
Broken: Chrome, Firefox, Opera

[ocean90]

This is a known issue, the spaces are breaking HiDPI avatars too.

[SergeyBiryukov]

Replying to ocean90:

This is a known issue, the spaces are breaking HiDPI avatars too.

I think it's the dots rather than spaces:

• https://wordpress.org/grav-redirect.php?user=let-me-see...&s=48 has the same issue (403).
• https://wordpress.org/grav-redirect.php?user=let-me-see&s=48 works as expected.

[ocean90]

Both. :)

[SergeyBiryukov]

Should we use the sanitized username that Profiles redirect to, i.e. let-me-see instead of let me see...?

[ocean90]

In 3140:

Trac: Use user's nicename for gravatars and profile links.

Adds wporg_sanitize_user_nicename() which simulates bb_sanitize_with_dashes() from bbPress.

See #1462.

[stephdau]

@ocean90 : I think dot (.) is missing from the regex in r3140 (str = re.sub(r'[^%a-z0-9\x80-\xff _-]', '', str) ), leading to the user henry.wright to get the Gravatar for henrywright on Trac: ​https://href.li/?https://core.trac.wordpress.org/ticket/38486

The generated grav redirect URL is:
​https://href.li/?https://wordpress.org/grav-redirect.php?user=henrywright&s=96

Leading to the wrong hash:
​https://secure.gravatar.com/avatar/0f6a8d63a73d65a3e5418b3040bc9b25?s=96&d=retro

This was reported to us (Automattic) by the user.

Looks like the Trac integration should match what grav-redirect.php is itself doing: [^a-z0-9_. @-].

[ocean90]

@stephdau The regex is fine because dots are not allowed in user_nicename. That this will produce a wrong gravatar for a few people is a known issue and can be fixed case-by-case, see ​https://wordpress.slack.com/archives/meta-tracdev/p1477221873000365 for a similar report.

[stephdau]

Hmmm, ok then.

[mdawaffe]

Replying to ocean90:

@stephdau The regex is fine because dots are not allowed in user_nicename. That this will produce a wrong gravatar for a few people is a known issue and can be fixed case-by-case, see ​https://wordpress.slack.com/archives/meta-tracdev/p1477221873000365 for a similar report.

Trying to look things up by user_nicename from Trac seems broken.

• grav-redirect.php looks up the user by user_login first, then falls back to user_nicename.
• user_login is what we display publicly on Trac (as far as I can tell).
• Trac's wporg_sanitize_user_nicename() cannot determine a user_nicename from a user_login. Only DB access can do that. For example, in this case (as in the one referenced at ​https://wordpress.slack.com/archives/meta-tracdev/p1477221873000365), there is a user_nicename "conflict": user_login=henry.wright, user_nicename=henrywright v. user_login=henrywright, user_nicename=henrywright-1. In the case discussed on Slack, both accounts were (supposedly) owned by the same person, so we could edit the DB at will. That's not always going to be the case.

In this specific instance, if grav-redirect.php looked things up by user_nicename first (then fell back to user_login), we'd get the correct Gravatar, but that will just break things in some other way (if user_login=henrywright creates a Trac ticket, for example).

It seems like we should just look things up by user_login all the time: it's what Trac uses internally and publicly and is always predictable.

[dd32]

I'm going to echo @mdawaffe here

We should encode user_logins if needed, and if .. is causing issue, we should either change affected user_logins (as harsh as that sounds), disable that check for this URL, or find a way to encode it (unfortunately it looks like %2e triggers the check too).

I'd rather have this broken for users with insane user_logins than have it broken for those wish clashing user_nicenames - there's about 20k users with slugs which don't match tracs current slug generation, about 500 with .. in their logins, not going to check spaces but they're easily dealt with by encoding them.

[henry.wright]

If you take a look at the "Notifications" section below, you'll see my Gravatar is displayed (I'm watching this ticket). Contrast that with my photo on this comment. Not sure if that helps but thought I'd mention.

[mdawaffe]

@dd32, in lieu of a real fix, can you work your DB magic on @henry.wright's accounts? (Can we verify both accounts belong to the same person somehow?)

Account 1:
user_login=henry.wright
user_nicename=henrywright

Account 2:
user_login=henrywright
user_nicename=henrywright-1

He says:

I use Account 1 so would like to keep that. Account 2 can be deleted if that makes things easier. Perhaps Account 1 could end up looking like this?

user_login=henrywright
user_nicename=henrywright

[mdawaffe]

Replying to mdawaffe:

In the case discussed on Slack, both accounts were (supposedly) owned by the same person

Sorry - I didn't see ​https://wordpress.slack.com/archives/C0C89GD35/p1477230539000375 earlier. I didn't mean to imply you weren't being careful :)

[obenland]

#2483 was marked as a duplicate.

[ocean90]

#3531 was marked as a duplicate.

[xedin.unknown]

I am having the same exact issue as @henrywright.

[SergeyBiryukov]

Related: #2539

[dd32]

In [dotorg15864] I've updated the grav-redirect.php script to lookup by user_nicename in preference to user_login which I think will fix the remaining few cases after [3140].

The only other option for fixing this would be to revert [dotorg15864] and send the raw user_login over encoded using something that encoded all special chars not using URL encoding, like Base32 or similar encoding that doesn't include special chars.