Make WordPress Core

Opened 4 months ago

Closed 2 months ago

#65068 closed defect (bug) (fixed)

PHP warnings when a registered oEmbed provider is malformed

Reported by: bradshawtm Owned by: westonruter
Priority: normal Milestone: 7.1
Component: Embeds Version: 4.0
Severity: minor Keywords: has-patch has-unit-tests changes-requested commit
Cc: Focuses:

Description

There's no gate to ensure oEmbed providers are valid, so when they're not, one gets these warnings:

PHP Warning:  Undefined array key 0 in /var/www/html/wp-includes/class-wp-oembed.php on line 276
PHP Warning:  Undefined array key 1 in /var/www/html/wp-includes/class-wp-oembed.php on line 276

To reproduce:

First, add an mu-plugin (this same errant structure was seen in the wild):

<?php
add_filter(
	'oembed_providers',
	function ( $providers ) {
		$providers['bad_provider'] = array(
			'url'      => '#https?://example\.site/.*#i',
			'endpoint' => 'https://example.site/api/oembed',
		);
		return $providers;
	}
);
  1. Go to a new post page.
  2. Paste a URL that doesn't have an oEmbed, e.g. https://en.wikipedia.org/wiki/Rickrolling
  3. Do NOT click "Convert to link".
  4. Refresh the post.

Each refresh will show the set of warnings.

Change History (8)

This ticket was mentioned in PR #11568 on WordPress/wordpress-develop by @sukhendu2002.


4 months ago
#1

  • Keywords has-patch has-unit-tests added

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

## Use of AI Tools

#2 @westonruter
4 months ago

  • Keywords changes-requested added
  • Milestone Awaiting Review7.1

#3 @westonruter
4 months ago

  • Keywords commit added
  • Owner set to westonruter
  • Status newreviewing
  • Version4.0

Queueing up for commit once trunk is unfrozen.

#4 @rollybueno
2 months ago

Reproduction Report

Environment

  • WordPress: 7.1-alpha-62161-src
  • Subdirectory: No
  • PHP: 8.2.29
  • Server: nginx/1.29.4
  • Database: mysqli (Server: 8.4.8 / Client: mysqlnd 8.2.29)
  • Browser: Chrome 149.0.0.0
  • OS: Linux
  • Theme: Twenty Twenty-Five 1.5
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.3.0

Steps taken

  1. Created an MU plugin file at:
wp-content/mu-plugins/bad-oembed-provider.php
  1. Added the following malformed oEmbed provider structure:
<?php
/**
 * Plugin Name: Bad oEmbed Provider Test
 * Description: Adds a malformed test oEmbed provider.
 */

add_filter(
	'oembed_providers',
	function ( $providers ) {
		$providers['bad_provider'] = array(
			'url'      => '#https?://example\.site/.*#i',
			'endpoint' => 'https://example.site/api/oembed',
		);

		return $providers;
	}
);
  1. Confirmed the MU plugin was loaded from the Must-Use plugins screen.
  2. Went to Posts > Add New.
  3. Pasted a URL that does not have an oEmbed provider, for example:
    https://en.wikipedia.org/wiki/Rickrolling
    
  1. Did not click Convert to link.
  2. Saved the post.
  3. Refreshed the post editor page.

🐞 Bug occurs

The following PHP warnings were reproduced:

[15-Jun-2026 00:57:19 UTC] PHP Warning:  Undefined array key 0 in /var/www/src/wp-includes/class-wp-oembed.php on line 276
[15-Jun-2026 00:57:19 UTC] PHP Warning:  Undefined array key 1 in /var/www/src/wp-includes/class-wp-oembed.php on line 276

Expected behavior

WordPress should validate the registered oEmbed provider structure before attempting to access indexed array values.

A malformed provider should not trigger PHP warnings. Instead, WordPress should either skip invalid providers or safely handle them without generating warnings.

Additional Notes

  • The issue only occurs after registering the malformed oEmbed provider through the oembed_providers filter.
  • The malformed provider uses named keys, url and endpoint, instead of the expected indexed provider format.
  • The warning is reproducible when the editor attempts to process a URL that does not have a valid oEmbed response.
  • The issue is reproducible on the latest 7.1-alpha source build tested.
  • This confirms the behavior described in the ticket.

Screenshots/Screencast with results

https://drive.google.com/file/d/1GtgLwMXF7XeW5J4hh7Ib9O9TwwFvh3l1/view?usp=sharing

Support Content

  1. MU plugin snippet used for testing:
<?php
/**
 * Plugin Name: Bad oEmbed Provider Test
 * Description: Adds a malformed test oEmbed provider.
 */

add_filter(
	'oembed_providers',
	function ( $providers ) {
		$providers['bad_provider'] = array(
			'url'      => '#https?://example\.site/.*#i',
			'endpoint' => 'https://example.site/api/oembed',
		);

		return $providers;
	}
);
  1. Test URL used:
https://en.wikipedia.org/wiki/Rickrolling

#5 @rollybueno
2 months ago

Test Report

Patch tested: https://github.com/WordPress/wordpress-develop/pull/11568

Environment

  • WordPress: 7.1-alpha-62161-src
  • Subdirectory: No
  • PHP: 8.2.29
  • Server: nginx/1.29.4
  • Database: mysqli (Server: 8.4.8 / Client: mysqlnd 8.2.29)
  • Browser: Chrome 149.0.0.0
  • OS: Linux
  • Theme: Twenty Twenty-Five 1.4
  • MU Plugins:
    • Bad oEmbed Provider Test
  • Plugins:
    • Test Reports 1.3.0

Steps taken

  1. Checked out the patch from https://github.com/WordPress/wordpress-develop/pull/11568.
  2. Rebuilt the development environment.
  3. Created an MU plugin file at wp-content/mu-plugins/bad-oembed-provider.php.
  4. Added the malformed oEmbed provider structure used to reproduce the issue:
<?php
/**
 * Plugin Name: Bad oEmbed Provider Test
 * Description: Adds a malformed test oEmbed provider.
 */

add_filter(
	'oembed_providers',
	function ( $providers ) {
		$providers['bad_provider'] = array(
			'url'      => '#https?://example\.site/.*#i',
			'endpoint' => 'https://example.site/api/oembed',
		);

		return $providers;
	}
);
  1. Confirmed the MU plugin was loaded from the Must-Use plugins screen.
  2. Went to Posts > Add New.
  3. Pasted a non-oEmbed URL into the editor:
https://en.wikipedia.org/wiki/Rickrolling
  1. Did not click Convert to link.
  2. Saved the post.
  3. Refreshed the post editor page.
  4. Checked the PHP error log.
  1. ✅ Patch is solving the problem

After applying the patch, the PHP warnings were no longer triggered when refreshing the post editor.

Expected result

The patch prevents malformed oEmbed provider entries from triggering PHP warnings.

WordPress should safely handle invalid provider structures registered through the oembed_providers filter by skipping them instead of attempting to access missing indexed array values.

Specifically, the following warnings should no longer appear:

PHP Warning: Undefined array key 0 in wp-includes/class-wp-oembed.php
PHP Warning: Undefined array key 1 in wp-includes/class-wp-oembed.php

Additional Notes

  • I was able to reproduce the issue before applying the patch using the same malformed MU plugin.
  • After applying the patch, the same reproduction steps no longer produced the PHP warnings.
  • Valid oEmbed behavior appears unaffected based on this test.
  • This confirms that the patch addresses the reported issue.

Screenshots/Screencast with results

@westonruter commented on PR #11568:


2 months ago
#6

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 commented on PR #11568:


2 months ago
#7

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

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.

#8 @westonruter
2 months ago

  • Resolutionfixed
  • Status reviewingclosed

In 62501:

Embeds: Validate registered oEmbed providers.

A plugin may register an oEmbed provider through the oembed_providers filter using a malformed structure, such as an associative array rather than the expected tuple of a provider endpoint URL string at index 0 and an optional boolean regex flag at index 1. This previously produced Undefined array key PHP warnings when WP_oEmbed::get_provider() destructured the entry.

Introduce a private sanitize_provider() method that validates the match pattern and provider data, normalizing the optional regex flag to a boolean. The constructor now skips malformed entries and reports each one via _doing_it_wrong(), and get_provider() likewise ignores any invalid entries it encounters at runtime.

Developed in https://github.com/WordPress/wordpress-develop/pull/11568.

Props sukhendu2002, westonruter, bradshawtm, rollybueno.
Fixes #65068.

Note: See TracTickets for help on using tickets.