oEmbed: Fix PHP warnings when a registered provider is malformed

[Sukhendu2002]

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

Use of AI Tools

AI assistance: No

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[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

• 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.

[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 sukhendu2002, westonruter, bradshawtm.

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

[tbradsha]

Looks good to me!

[westonruter]

Thank you. Let's queue this up for commit in 7.1 once trunk is unfrozen.

[westonruter]

Review from Claude:

I have enough to finalize. Let me verify one convention point: @covers ::get_provider — the file uses @coversDefaultClass WP_oEmbed, and existing tests use @covers ::get_html, so ::method correctly resolves to the default class. That annotation is fine (not a finding).

Here's my review of PR #11568.

Review Summary

The change adds validation/normalization of oEmbed providers (a new sanitize_provider() helper, a _doing_it_wrong() warning in the constructor, and re-validation in get_provider()). The diff is defensive and largely sound. No crash-class bugs found, but there is one real backward-compatibility regression plus several altitude/hygiene issues worth surfacing.

Findings

1. Constructor narrows the provider tuple shape — drops extra data plugins may have stored (class-wp-oembed.php:247)
Previously $this->providers = apply_filters( 'oembed_providers', $providers ) stored each entry verbatim. The new code rewrites every entry to a strict array( $provider['endpoint'], $provider['is_regex'] ), discarding any additional array elements and coercing index 1 to a bool. A plugin that stored extra elements/keys in a provider tuple (e.g. array( $url, true, 'ttl' => 3600 )) — permitted by the old pass-through — would silently lose that data when reading $wp_oembed->providers afterward. Not in the documented contract, but a real behavior change.

2. get_provider() re-sanitizes already-normalized providers on every call (class-wp-oembed.php:330)
The constructor's entire purpose is to normalize and store clean tuples, yet get_provider() distrusts that and runs sanitize_provider() again on each entry, each call. The validation responsibility is split across two places (the only entries that bypass the constructor are direct writes via wp_oembed_add_provider() at embed.php:150). Consider validating in one place (e.g. normalize on write) so the stored form is authoritative.

3. Test mutates the shared oEmbed singleton without cleanup (tests/phpunit/tests/oembed/wpOembed.php:306)
$this->oembed is _wp_oembed_get_object(), a function-static singleton that is not reset between tests (unlike hooks, which _backup_hooks()/_restore_hooks() restore — which is why removing the remove_filter() calls is safe). test_get_provider_handles_provider_without_regex_flag adds $this->oembed->providers['https://example.site/*'] and never removes it, leaking that provider into subsequent tests in the same process.

4. Malformed providers added post-construction are silently skipped, no _doing_it_wrong() (class-wp-oembed.php:331)
The constructor warns on malformed entries, but get_provider() just continues past them. A malformed provider injected after construction (e.g. via direct $oembed->providers[...] = like wp_oembed_add_provider() does) is dropped with no diagnostic — inconsistent with the constructor path.

5. Dead reassignment (class-wp-oembed.php:334)
$match_mask = $provider_data['match_mask']; is a no-op: sanitize_provider() returns match_mask unchanged from the loop key, so this always assigns $match_mask to itself. Can be removed.

No correctness/crash bugs survived — the validation logic itself (empty-string checks, isset/is_string guards, (bool) ( $data[1] ?? false ), (string) preg_replace(...) cast) is correct, and 7.1.0/@since 7.1.0 matches $wp_version = '7.1-alpha'.

[westonruter]

1. Constructor narrows the provider tuple shape — drops extra data plugins may have stored (class-wp-oembed.php:247)

This isn't really a concern. Any extra array items will be dropped from being stored in the providers array. I don't think we should try to preserve invalid data being passed.

2. get_provider() re-sanitizes already-normalized providers on every call (class-wp-oembed.php:330)

True, but since WP_Embed::$providers is public, we can't really trust it.

3. Test mutates the shared oEmbed singleton without cleanup (tests/phpunit/tests/oembed/wpOembed.php:306)

Fixed in f51d63e.

4. Malformed providers added post-construction are silently skipped, no _doing_it_wrong() (class-wp-oembed.php:331)

Adding providers in this way is non-standard, and emitting _doing_it_wrong() here would be redundant, I feel.

5. Dead reassignment (class-wp-oembed.php:334)

This is invalid. We're using the value returned by the sanitize method in case it does any other mutations in the future.

[github-actions]

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 62501
GitHub commit: 73ed971

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.