Make WordPress Core

Opened 7 months ago

Last modified 7 months ago

#59716 new defect (bug)

Deprecated messages exporting post meta with meta_value null from PHP 8.1

Reported by: bobbingwide's profile bobbingwide Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Export Keywords: php81
Focuses: Cc:

Description

Today I attempted to export some products from one database to import into another.
I encountered several Deprecated messages emanating from code within wxr_cdata()

It appears that the export code doesn't specifically cater for meta data with null values.
Prior to PHP 8.1 the null values were treated as empty strings.

There are several plugins that are guilty of producing post meta data containing nulls. Two of them are in the top 12 plugins by total downloads: All-In-One-SEO-Pack and WooCommerce.

Actual output

Here's the exported post meta for WooCommerce's _stock field.

						<wp:postmeta>
		<wp:meta_key><![CDATA[_stock]]></wp:meta_key>
		<wp:meta_value><br />
<b>Deprecated</b>:  strlen(): Passing null to parameter #1 ($string) of type string is deprecated in <b>C:\apache\htdocs\gardenvista\wp-includes\formatting.php</b> on line <b>885</b><br />
<br />
<b>Deprecated</b>:  str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in <b>C:\apache\htdocs\gardenvista\wp-admin\includes\export.php</b> on line <b>207</b><br />
<![CDATA[]]></wp:meta_value>
		</wp:postmeta>

Both of these Deprecated messages can be avoided by a simple change as the first line of wxr_cdata()

if ( null === $str ) {
            $str = '';
        }

Expected output:

  • The export should complete without producing the Deprecated message output in the post meta value.
  • Null values should continue to be treated as empty strings

Environment:

  • PHP 8.1.23 or PHP 8.2.10
  • WordPress 6.4-RC1
  • Plugins: WooCommerce and/or All-In-One-SEO

Change History (6)

#1 @jrf
7 months ago

  • Focuses php-compatibility removed
  • Keywords php81 added
  • Summary changed from Deprecated messages exporting post meta with meta_value null from PHP 8.1 and PHP 8.2 to Deprecated messages exporting post meta with meta_value null from PHP 8.1

#2 @jrf
7 months ago

@bobbingwide Thanks for reporting this. This will need a backtrace to see the exact code path as this should be solved in the function where the data is originally passed around, not in the wxr_cdata() function, which is clearly and explicitly marked as only accepting strings.

If the solution suggested in the issue description would be applied, we'd be hiding the problem instead of fixing it.

#3 @bobbingwide
7 months ago

wxr_cdata() was written for WordPress 2.1.0
In those days I would argue that null was accepted as a valid string: an empty one.

Why can't the docblock be changed to reflect the fact that since PHP 8.1 a null value is also acceptable?

In my opinion, PHP 8.1 caused the problem, and this is a valid solution that will reduce the amount of effort to deal with it.

plugins\oik-bwtrace\includes\bwtrace-actions.php(286:2) bw_trace_error_handler(1) 34 1 2023-10-24T12:41:18+00:00 2.461264 0.088387 cf! 22965 187 2592 115343360/115343360 2048M F=2271 err default output handler,default output handler
Array

    [0] => (integer) 8192
    [1] => (string) "Deprecated: strlen(): Passing null to parameter #1 ($string) of type string is deprecated"
    [2] => (string) "\wp-includes\formatting.php"
    [3] => (integer) 885


0. bw_lazy_backtrace \plugins\oik-bwtrace\libs\bwtrace.php:108 0
1. bw_backtrace plugins\oik-bwtrace\includes\bwtrace-actions.php:293 0
2. bw_trace_error_handler(8192,strlen(): Passing null to parameter #1 ($string) of type string is deprecated,\wp-includes\formatting.php,885) \wp-includes\formatting.php:885 4
3. seems_utf8(null) \wp-admin\includes\export.php:200 1
4. wxr_cdata(null) \wp-admin\includes\export.php:629 1
5. export_wp(array) \wp-admin\export.php:123 1

The line that leads to the error is:

	<wp:meta_value><?php echo wxr_cdata( $meta->meta_value ); ?></wp:meta_value>

#4 @jrf
7 months ago

Widening the accepted data type for the function, while the function is only intended to deal with strings, is 100% the wrong solution. If we go down that road, every single function in WP would need to be able to deal with every single type of input.

It should be fixed at the source of the problem. Fixing the symptom is not the way to go.

Think of it this way: would you treat a brain tumor (actual problem) with a paracetamol to combat the headache (symptom) ?

#5 @bobbingwide
7 months ago

Re: while the function is only intended to deal with strings.

  1. The wp_postmeta table's meta_value column is nullable. Ditto: wp_termmeta, wp_usermeta
  2. Therefore it seems reasonable for the field to contain null.
  3. The comment below was added 13 years ago today ( PHP 5.5? )
     * @param string $str String to wrap in XML CDATA tag.
    
  4. Some of the export calls are wrapped to not export data when it's empty()
  5. But those that aren't might also expect null values.
  6. There are 101 calls to wxr_cdata()
  7. I would implement a pragmatic solution to change the function not the callers.

#6 @hellofromTonya
7 months ago

  • Version trunk deleted

Removing trunk as the Version that introduced the reported issue.

Note: See TracTickets for help on using tickets.