#60638 Conditionally use SHA-256 or MD5 for Gravatar hashing

[johnbillion]

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

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props johnbillion, peterwilsoncc, dd32.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[peterwilsoncc]

As the test suite is run by various hosting providers, https://make.wordpress.org/hosting/test-results/, you'll need to make the tests conditional too.

The contents of tests/qunit/fixtures/wp-api-generated.js will vary depending on whether the system generating them uses md5 or sha256 so I am not sure what will be needed to handle that edge case.

[johnbillion]

Oh that's nasty. Good spot. In practice, do any hosts have hash disabled? I guess we'll only know by committing the change to core and waiting for the host tests to run. There's no way to run the host tests against a PR, correct?

[johnbillion]

I ask because if there are none then I would personally be happy to retain the hardcoded references to sha256 in the test suite. If we can get the minimum supported version of PHP bumped to 7.4 later this year then the issue goes away then anyway.

[dd32]

In practice, do any hosts have hash disabled?

Personally: I don't think so.

The stats included in WP 6.1+ show that a single site in the data-set has json extension but doesn't have the hash extension. That could easily be a rounding error or other data-error.

The extension being disabled did cross my mind when I saw this originally, but thought it was already a required extension, due to the stats showing it being installed almost everywhere. Core does conditionally check for it in a few places, but PHPMailer doesn't (although it looks to be only in the HTML emails code paths, which is not at all run by many)

There's no way to run the host tests against a PR, correct?

AFAIK, that's correct.

I'm totally cool for this to be reverted and wait for PHP7.4 bump.

[peterwilsoncc]

Oh that's nasty. Good spot. In practice, do any hosts have hash disabled?

I'd be very surprised if this is the case. I can't see why anyone would want to do this but I've been surprised before.

I guess we'll only know by committing the change to core and waiting for the host tests to run. There's no way to run the host tests against a PR, correct?

That's correct, there's no way to run them against a PR. I think commit and see is the best we can do.

Reviewing the PHP stats, it appears likely we're a few months from being able to bump the minimum version. The KISS approach would be to revert the change and reapply it once the minimum is bumped as Automattic are retaining MD5 support for the foreseeable future.

Hash lookup tables exist for all one way hashes, so the benefit of switching can wait.

[peterwilsoncc]

The stats included in WP 6.1+ show that a single site in the data-set has json extension but doesn't have the hash extension. That could easily be a rounding error or other data-error.

The extension being disabled did cross my mind when I saw this originally, but thought it was already a required extension, due to the stats showing it being installed almost everywhere. Core does conditionally check for it in a few places, but PHPMailer doesn't (although it looks to be only in the HTML emails code paths, which is not at all run by many)

Being generous with the rounding error and assuming a few thousand sites don't have the extension available, I think it's fine to require the extension rather than do all this juggling.

@johnbillion What do you think of a make post/trac ticket to discuss this so contributors can figure out if that's a better approach?