Make WordPress Core

Opened 10 years ago

Closed 2 years ago

Last modified 15 months ago

#37454 closed defect (bug) (fixed)

get_avatar_data() delivers different url for scheme=https and is_ssl()

Reported by: neoxx Owned by: peterwilsoncc
Priority: normal Milestone: 6.7
Component: Users Version: 4.3
Severity: normal Keywords: has-patch, has-unit-tests
Cc: Focuses:

Description

get_avatar_data() has an option to set the scheme. If https is used, an image URL is returned which is different to the one which is returned in case delivery is conducted via https without the scheme parameter. I think it would be better to serve the same gravatar url for https pages and explicit https-scheme requests.

Regarding performance and depending on the infrastructure, it could also be benefical to drop ​https://%d.gravatar.com completely and serve all requests to gravatar independent of the page's protocol via ​https://secure.gravatar.com/ to benefit from HTTP/2 (http and ​https://%d.gravatar.com requests are served via HTTP/1.1 whereas ​https://secure.gravatar.com/ allows multiplexing via HTTP/2).

Attachments (1)

37454.diff​ (627 bytes ) - added by neoxx 10 years ago.

Download all attachments as: .zip

Change History (12)

@neoxx
10 years ago

#1 @neoxx
10 years ago

  • Keywords has-patch added

#2 @SergeyBiryukov
10 years ago

  • Component General → Users
  • Version trunk → 4.3

Introduced in [32845].

#3 @peterwilsoncc
10 years ago

  • Keywords needs-unit-tests added

This will need some unit tests and a decision re: switching all to https. I'm +1 in line with other externally hosted assets.

#4 @neoxx
10 years ago

Following Matt's statement on SSL (cf. ​https://wordpress.org/news/2016/12/moving-toward-ssl/), we should probably move forward on delivering Gravatars via HTTPS / HTTP/2.

#5 @sippis
10 years ago

This is a quite small change but really important from my point of view. Just noticed that Freedome (privacy VPN from f-secure) blocks requests to gravatar when using http. Haven't tested other privacy products like privacy badger, but I can image that some other tools are also blocking unsecure gravatar.

I think there's really no reason why we shouldn't use always secure.gravatar.com.

This ticket was mentioned in ​PR #6689 on ​WordPress/wordpress-develop by ​@peterwilsoncc.


2 years ago
#6

Gravatar.com now redirects HTTP URLs to HTTPS resulting in unnecessary redirects for sites attempting to serve the URLs over HTTP.

This ignores the scheme setting in get_avatar_data() when generating Gravatar URLs to avoid the redirects. The setting is retained to allow for sites using local/external avatar services that do not use HTTPS to serve the images.

I've retained the use of the secure.gravatar.com domain to keep this change focused on the issue at hand but it's worth noting that the Gravatar docs ​recommend the base domain.

Trac ticket: https://core.trac.wordpress.org/ticket/37454

#7 @peterwilsoncc
2 years ago

  • Milestone Awaiting Review → 6.7

Reviving this as Gravatar now redirects all requests to HTTPS URLs. This can result in unnecessary 301 redirects.

In the ​linked pull requests:

  • retain the scheme setting for get_avatar_data() to account for other services
  • ignore the scheme setting when generating gravatar URLs
  • updates references in the tests to use secure.gravatar.com

As the WordPress 6.6 beta is fast approaching, I've put this on the 6.7 milestone. I think it would be fine to put in earlier but will allow the release squad to decide if they want to bring it forward.

#8 @peterwilsoncc
2 years ago

  • Owner set to peterwilsoncc
  • Resolution → fixed
  • Status new → closed

In 58822:

Users: Always use HTTPS URLs for Gravatar links.

Modifies gravatar image URLs to always use the HTTPS version from secure.gravatar.com.

Gravatar now redirects HTTP image requests to their HTTPS equivalent, resulting in redirects for sites running over an HTTP connection (is_ssl() === false). Since the introduction of HTTP/2 the use of sub-domains for different hashes ([1-3].gravatar.com) now represents a performance hinderance rather than improvement.

The scheme passed to get_avatar_data() is now ignored for the generation of Gravatar URLs but the setting retained to avoid introducing bugs for sites using either local avatars or third party providers.

Props neoxx, SergeyBiryukov, sippis, peterwilsoncc, mukesh27, costdev, dd32.
Fixes #37454.

#10 @ironprogrammer
2 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

#11 @johnbillion
15 months ago

#54363 was marked as a duplicate.

Note: See TracTickets for help on using tickets.