Fix Uncaught ValueError in Gravity Forms and WordPress SEO connectors

.gitignore

@@ -10,6 +10,7 @@
 /stream.zip
 /stream-*.zip
 npm-debug.log
+debug.log
 package.lock
 .phpunit.result.cache
 

classes/class-connector.php

@@ -294,4 +294,14 @@ function( $value ) {
 	public function is_dependency_satisfied() {
 		return true;
 	}
+
+	/**
+	 * Escape % characters in a string to avoid Uncaught ValueErrors in $this->log().
+	 *
+	 * @param string $value The string value to be escaped.
+	 * @return string The escaped string.
+	 */
+	public function escape_percentages( $value ) {
+		return str_replace( '%', '%%', $value );
+	}
 }

composer.json

@@ -31,6 +31,7 @@
     "wpackagist-plugin/easy-digital-downloads": "2.9.23",
     "wpackagist-plugin/jetpack": "10.0",
     "wpackagist-plugin/user-switching": "1.5.5",
+    "wpackagist-plugin/wordpress-seo": "23.0",
     "wpackagist-theme/twentytwentythree": "^1.0",
     "xwp/wait-for": "^0.0.1",
     "yoast/phpunit-polyfills": "^1.1"
@@ -70,6 +71,10 @@
       "phpunit --coverage-text",
       "php local/scripts/make-clover-relative.php ./tests/reports/clover.xml"
     ],
+    "test-one": [
+      "phpunit",
+      "WP_MULTISITE=1 phpunit"
+    ],
     "test-multisite": [
       "WP_MULTISITE=1 phpunit --coverage-text",
       "php local/scripts/make-clover-relative.php ./tests/reports/clover.xml"

connectors/class-connector-gravityforms.php

@@ -221,20 +221,23 @@ public function register() {
 	 * @param bool  $is_new  Is this a new form?.
 	 */
 	public function callback_gform_after_save_form( $form, $is_new ) {
-		$title = $form['title'];
-		$id    = $form['id'];
+		$id = $form['id'];
 
 		$this->log(
 			sprintf(
 				/* translators: %1$s a form title, %2$s a status (e.g. "Contact Form", "created") */
 				__( '"%1$s" form %2$s', 'stream' ),
-				$title,
-				$is_new ? esc_html__( 'created', 'stream' ) : esc_html__( 'updated', 'stream' )
+				$this->get_form_title_for_message( $form ),
+				$this->get_status_for_message(
+					$is_new,
+					esc_html__( 'created', 'stream' ),
+					esc_html__( 'updated', 'stream' )
+				)
 			),
 			array(
 				'action' => $is_new,
 				'id'     => $id,
-				'title'  => $title,
+				'title'  => $form['title'],
 			),
 			$id,
 			'forms',
@@ -258,9 +261,13 @@ public function callback_gform_pre_confirmation_save( $confirmation, $form, $is_
 			sprintf(
 				/* translators: %1$s: a confirmation name, %2$s: a status, %3$s: a form title (e.g. "Email", "created", "Contact Form") */
 				__( '"%1$s" confirmation %2$s for "%3$s"', 'stream' ),
-				$confirmation['name'],
-				$is_new ? esc_html__( 'created', 'stream' ) : esc_html__( 'updated', 'stream' ),
-				$form['title']
+				$this->get_name_for_message( $confirmation ),
+				$this->get_status_for_message(
+					$is_new,
+					esc_html__( 'created', 'stream' ),
+					esc_html__( 'updated', 'stream' )
+				),
+				$this->get_form_title_for_message( $form )
 			),
 			array(
 				'is_new'  => $is_new,
@@ -291,9 +298,13 @@ public function callback_gform_pre_notification_save( $notification, $form, $is_
 			sprintf(
 				/* translators: %1$s: a notification name, %2$s: a status, %3$s: a form title (e.g. "Email", "created", "Contact Form") */
 				__( '"%1$s" notification %2$s for "%3$s"', 'stream' ),
-				$notification['name'],
-				$is_new ? esc_html__( 'created', 'stream' ) : esc_html__( 'updated', 'stream' ),
-				$form['title']
+				$this->get_name_for_message( $notification ),
+				$this->get_status_for_message(
+					$is_new,
+					esc_html__( 'created', 'stream' ),
+					esc_html__( 'updated', 'stream' )
+				),
+				$this->get_form_title_for_message( $form )
 			),
 			array(
 				'is_update' => $is_new,
@@ -318,8 +329,8 @@ public function callback_gform_pre_notification_deleted( $notification, $form )
 			sprintf(
 				/* translators: %1$s: a notification name, %2$s: a form title (e.g. "Email", "Contact Form") */
 				__( '"%1$s" notification deleted from "%2$s"', 'stream' ),
-				$notification['name'],
-				$form['title']
+				$this->get_name_for_message( $notification ),
+				$this->get_form_title_for_message( $form )
 			),
 			array(
 				'form_id'      => $form['id'],
@@ -342,8 +353,8 @@ public function callback_gform_pre_confirmation_deleted( $confirmation, $form )
 			sprintf(
 				/* translators: %1$s: a confirmation name, %2$s: a form title (e.g. "Email", "Contact Form") */
 				__( '"%1$s" confirmation deleted from "%2$s"', 'stream' ),
-				$confirmation['name'],
-				$form['title']
+				$this->get_name_for_message( $confirmation ),
+				$this->get_form_title_for_message( $form )
 			),
 			array(
 				'form_id'      => $form['id'],
@@ -367,9 +378,13 @@ public function callback_gform_confirmation_status( $confirmation, $form, $is_ac
 			sprintf(
 				/* translators: %1$s: a confirmation name, %2$s: a status, %3$s: a form title (e.g. "Email", "activated", "Contact Form") */
 				__( '"%1$s" confirmation %2$s from "%3$s"', 'stream' ),
-				$confirmation['name'],
-				$is_active ? esc_html__( 'activated', 'stream' ) : esc_html__( 'deactivated', 'stream' ),
-				$form['title']
+				$this->get_name_for_message( $confirmation ),
+				$this->get_status_for_message(
+					$is_active,
+					esc_html__( 'activated', 'stream' ),
+					esc_html__( 'deactivated', 'stream' )
+				),
+				$this->get_form_title_for_message( $form )
 			),
 			array(
 				'form_id'      => $form['id'],
@@ -394,9 +409,13 @@ public function callback_gform_notification_status( $notification, $form, $is_ac
 			sprintf(
 				/* translators: %1$s: a notification name, %2$s: a status, %3$s: a form title (e.g. "Email", "activated", "Contact Form") */
 				__( '"%1$s" notification %2$s from "%3$s"', 'stream' ),
-				$notification['name'],
-				$is_active ? esc_html__( 'activated', 'stream' ) : esc_html__( 'deactivated', 'stream' ),
-				$form['title']
+				$this->get_name_for_message( $notification ),
+				$this->get_status_for_message(
+					$is_active,
+					esc_html__( 'activated', 'stream' ),
+					esc_html__( 'deactivated', 'stream' )
+				),
+				$this->get_form_title_for_message( $form )
 			),
 			array(
 				'form_id'      => $form['id'],
@@ -514,7 +533,11 @@ public function check_rg_gforms_key( $old_value, $new_value ) {
 			sprintf(
 				/* translators: %s: a status (e.g. "updated") */
 				__( 'Gravity Forms license key %s', 'stream' ),
-				$is_update ? esc_html__( 'updated', 'stream' ) : esc_html__( 'deleted', 'stream' )
+				$this->get_status_for_message(
+					$is_update,
+					esc_html__( 'updated', 'stream' ),
+					esc_html__( 'deleted', 'stream' )
+				)
 			),
 			compact( 'option', 'old_value', 'new_value' ),
 			null,
@@ -755,8 +778,8 @@ public function callback_gform_update_status( $lead_id, $status, $prev = '' ) {
 				/* translators: %1$d: an ID, %2$s: a status, %3$s: a form title (e.g. "42", "activated", "Contact Form") */
 				__( 'Lead #%1$d %2$s on "%3$s" form', 'stream' ),
 				$lead_id,
-				$actions[ $status ],
-				$form['title']
+				$this->escape_percentages( $actions[ $status ] ),
+				$this->get_form_title_for_message( $form )
 			),
 			array(
 				'lead_id'    => $lead_id,
@@ -782,16 +805,19 @@ public function callback_gform_update_status( $lead_id, $status, $prev = '' ) {
 	public function callback_gform_update_is_read( $lead_id, $status ) {
 		$lead   = $this->get_lead( $lead_id );
 		$form   = $this->get_form( $lead['form_id'] );
-		$status = ( ! empty( $status ) ) ? esc_html__( 'read', 'stream' ) : esc_html__( 'unread', 'stream' );
 
 		$this->log(
 			sprintf(
 				/* translators: %1$d: a lead ID, %2$s: a status, %3$s: a form ID, %4$s: a form title (e.g. "42", "unread", "Contact Form") */
 				__( 'Entry #%1$d marked as %2$s on form #%3$d ("%4$s")', 'stream' ),
 				$lead_id,
-				$status,
+				$this->get_status_for_message(
+					! empty( $status ),
+					esc_html__( 'read', 'stream' ),
+					esc_html__( 'unread', 'stream' )
+				),
 				$form['id'],
-				$form['title']
+				$this->get_form_title_for_message( $form )
 			),
 			array(
 				'lead_id'     => $lead_id,
@@ -814,19 +840,23 @@ public function callback_gform_update_is_read( $lead_id, $status ) {
 	 * @param int $status   Status.
 	 */
 	public function callback_gform_update_is_starred( $lead_id, $status ) {
-		$lead   = $this->get_lead( $lead_id );
-		$form   = $this->get_form( $lead['form_id'] );
-		$status = ( ! empty( $status ) ) ? esc_html__( 'starred', 'stream' ) : esc_html__( 'unstarred', 'stream' );
-		$action = $status;
+		$lead               = $this->get_lead( $lead_id );
+		$form               = $this->get_form( $lead['form_id'] );
+		$status_for_message = $this->get_status_for_message(
+			! empty( $status ),
+			esc_html__( 'starred', 'stream' ),
+			esc_html__( 'unstarred', 'stream' )
+		);
+		$action = $status_for_message;
 
 		$this->log(
 			sprintf(
 				/* translators: %1$d: an ID, %2$s: a status, %3$d: a form title (e.g. "42", "starred", "Contact Form") */
 				__( 'Entry #%1$d %2$s on form #%3$d ("%4$s")', 'stream' ),
 				$lead_id,
-				$status,
+				$status_for_message, // This has been escaped above.
 				$form['id'],
-				$form['title']
+				$this->get_form_title_for_message( $form )
 			),
 			array(
 				'lead_id'     => $lead_id,
@@ -945,8 +975,8 @@ public function log_form_action( $form_id, $action ) {
 				/* translators: %1$d: an ID, %2$s: a form title, %3$s: a status (e.g. "42", "Contact Form", "Activated") */
 				__( 'Form #%1$d ("%2$s") %3$s', 'stream' ),
 				$form_id,
-				$form['title'],
-				strtolower( $actions[ $action ] )
+				$this->get_form_title_for_message( $form ),
+				strtolower( $this->escape_percentages( $actions[ $action ] ) )
 			),
 			array(
 				'form_id'     => $form_id,
@@ -976,4 +1006,45 @@ private function get_lead( $lead_id ) {
 	private function get_form( $form_id ) {
 		return \GFFormsModel::get_form_meta( $form_id );
 	}
+
+	/**
+	 * Get the name from an associative array with percentages escaped.
+	 * To be used when calling $this->log().
+	 *
+	 * @param array $data_array The array with a 'name' key to be escaped.
+	 * @return string The name value with percentages escaped.
+	 */
+	private function get_name_for_message( $data_array ) {
+		if ( empty( $data_array['name'] ) ) {
+			return $this->escape_percentages( __( 'This does not have a name key', 'stream' ) );
+		}
+
+		return $this->escape_percentages( $data_array['name'] );
+	}
+
+	/**
+	 * Get the form title from a form array with percentages escaped.
+	 * To be used when calling $this->log().
+	 *
+	 * @param array $form The form data array.
+	 * @return string The title value with percentages escaped.
+	 */
+	private function get_form_title_for_message( $form ) {
+		if ( empty( $form['title'] ) ) {
+			return $this->escape_percentages( __( 'This does not have a title key', 'stream' ) );
+		}
+
+		return $this->escape_percentages( $form['title'] );
+	}
+
+	/**
+	 * Get the status to use in a message with percentages in translation escaped.
+	 *
+	 * @param bool $conditional Whether or not it's a new form.
+	 * @return string The status with percentages escaped.
+	 */
+	private function get_status_for_message( $conditional, $true_string, $false_string ) {
+		$status = $conditional ? $true_string : $false_string;
+		return $this->escape_percentages( $status );
+	}
 }

connectors/class-connector-wordpress-seo.php

@@ -419,9 +419,9 @@ private function meta( $object_id, $meta_key, $meta_value ) {
 			sprintf(
 				/* translators: %1$s: a meta field title, %2$s: a post title, %3$s: a post type (e.g. "Description", "Hello World", "Post") */
 				__( 'Updated "%1$s" of "%2$s" %3$s', 'stream' ),
-				$field['title'],
-				$post->post_title,
-				$post_type_label
+				$this->escape_percentages( $field['title'] ),
+				$this->escape_percentages( $post->post_title ),
+				$this->escape_percentages( $post_type_label )
 			),
 			array(
 				'meta_key'   => $meta_key,

local/public/wp-config.php

@@ -25,6 +25,7 @@
 $table_prefix = 'wp_';
 
 define( 'WP_DEBUG', true );
+define( 'WP_DEBUG_LOG', true );
 define( 'JETPACK_DEV_DEBUG', true );
 
 // Keep the wp-contents outside of WP core directory.

package.json

@@ -46,6 +46,7 @@
     "test": "npm-run-all test:*",
     "test:php": "npm run cli -- composer test --working-dir=wp-content/plugins/stream-src",
     "test:php-multisite": "npm run cli -- composer test-multisite --working-dir=wp-content/plugins/stream-src",
+    "test:php:one": "npm run cli -- composer test-one --working-dir=wp-content/plugins/stream-src --",
     "test-report": "npm run cli -- composer test-report --working-dir=wp-content/plugins/stream-src",
     "build-containers": "docker compose --file docker-compose.build.yml build",
     "push-containers": "docker compose --file docker-compose.build.yml push",

phpunit.xml

@@ -10,7 +10,7 @@
 	<php>
 		<const
 			name="WP_TEST_ACTIVATED_PLUGINS"
-			value="advanced-custom-fields/acf.php,easy-digital-downloads/easy-digital-downloads.php,jetpack/jetpack.php,user-switching/user-switching.php"
+			value="advanced-custom-fields/acf.php,easy-digital-downloads/easy-digital-downloads.php,jetpack/jetpack.php,user-switching/user-switching.php,wordpress-seo/wp-seo.php"
 		/>
 	</php>
 	<testsuites>

tests/bootstrap.php

@@ -93,6 +93,7 @@ function( $status = false, $args = array(), $url = '') {
 define( 'EDD_USE_PHP_SESSIONS', false );
 define( 'WP_USE_THEMES', false );
 activate_plugin( 'easy-digital-downloads/easy-digital-downloads.php' );
+activate_plugin( 'wordpress-seo/wp-seo.php' );
 xwp_install_edd();
 
 require __DIR__ . '/testcase.php';

tests/testcase.php

@@ -15,6 +15,13 @@ class WP_StreamTestCase extends \WP_Ajax_UnitTestCase {
 	 */
 	protected $action_prefix = 'wp_stream_test_';
 
+	/**
+	 * Holds the mocked class.
+	 *
+	 * @var MockBuilder
+	 */
+	protected $mock;
+
 	/**
 	 * PHP unit setup function
 	 *