Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions paypal/controllers/FrmPayPalLiteHooksController.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ public static function load_admin_hooks() {
add_filter( 'frm_add_settings_section', 'FrmPayPalLiteSettingsController::add_settings_section', 99 );
add_action( 'frm_update_settings', 'FrmPayPalLiteSettingsController::process_form' );

// Hook into update_option to preserve test_mode when add-on saves
add_action( 'update_option_frm_paypal_options', 'FrmPayPalLiteSettingsController::preserve_test_mode_on_update', 10, 2 );

add_filter( 'frm_before_save_payment_action', 'FrmPayPalLiteActionsController::before_save_settings', 20, 2 );

// Hook into after payment type to render Product Name and Product Type fields.
Expand Down
24 changes: 24 additions & 0 deletions paypal/controllers/FrmPayPalLiteSettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,28 @@ public static function process_form() {
$settings->update( $_POST );
$settings->store();
}

/**
* Preserve test_mode when the PayPal add-on updates frm_paypal_options.
* This hooks into update_option to ensure test_mode isn't lost.
*
* @param mixed $old_value The old option value.
* @param mixed $new_value The new option value.
*
* @return void
*/
public static function preserve_test_mode_on_update( $old_value, $new_value ) {
if ( ! is_object( $old_value ) || ! is_object( $new_value ) ) {
return;
}

// If old value had test_mode but new value doesn't, preserve it
if ( ! isset( $old_value->test_mode ) || isset( $new_value->test_mode ) ) {
return;
}
Comment on lines +68 to +78

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify current re-entrant pattern:
# 1) Callback is attached to update_option_frm_paypal_options
# 2) Callback itself calls update_option('frm_paypal_options', ...)
rg -n --type=php -C3 "update_option_frm_paypal_options|preserve_test_mode_on_update|update_option\s*\(\s*'frm_paypal_options'" paypal/controllers

Repository: Strategy11/formidable-forms

Length of output: 1961


Avoid re-writing the same option inside its update_option_* callback.

Line 80 triggers a second update_option() from inside the update_option_frm_paypal_options action. This causes an extra write and re-fires option-update hooks, which can duplicate downstream side effects.

🔧 Suggested fix (move preservation to pre-update filter)
diff --git a/paypal/controllers/FrmPayPalLiteSettingsController.php b/paypal/controllers/FrmPayPalLiteSettingsController.php
@@
-	/**
-	 * Preserve test_mode when the PayPal add-on updates frm_paypal_options.
-	 * This hooks into update_option to ensure test_mode isn't lost.
-	 *
-	 * `@param` mixed $old_value The old option value.
-	 * `@param` mixed $new_value The new option value.
-	 *
-	 * `@return` void
-	 */
-	public static function preserve_test_mode_on_update( $old_value, $new_value ) {
-		if ( ! is_object( $old_value ) || ! is_object( $new_value ) ) {
-			return;
-		}
-
-		// If old value had test_mode but new value doesn't, preserve it
-		if ( isset( $old_value->test_mode ) && ! isset( $new_value->test_mode ) ) {
-			$new_value->test_mode = $old_value->test_mode;
-			// Update the option with the preserved test_mode
-			update_option( 'frm_paypal_options', $new_value );
-		}
-	}
+	/**
+	 * Preserve test_mode before frm_paypal_options is persisted.
+	 *
+	 * `@param` mixed $new_value The incoming option value.
+	 * `@param` mixed $old_value The currently stored option value.
+	 *
+	 * `@return` mixed
+	 */
+	public static function preserve_test_mode_on_update( $new_value, $old_value ) {
+		if ( is_object( $old_value ) && is_object( $new_value ) && isset( $old_value->test_mode ) && ! isset( $new_value->test_mode ) ) {
+			$new_value->test_mode = $old_value->test_mode;
+		}
+
+		return $new_value;
+	}
diff --git a/paypal/controllers/FrmPayPalLiteHooksController.php b/paypal/controllers/FrmPayPalLiteHooksController.php
@@
-		add_action( 'update_option_frm_paypal_options', 'FrmPayPalLiteSettingsController::preserve_test_mode_on_update', 10, 2 );
+		add_filter( 'pre_update_option_frm_paypal_options', 'FrmPayPalLiteSettingsController::preserve_test_mode_on_update', 10, 2 );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@paypal/controllers/FrmPayPalLiteSettingsController.php` around lines 68 - 78,
The preserve_test_mode_on_update function is currently hooked to the
update_option_frm_paypal_options action and calls update_option inside the
callback, which causes the hook to fire again and creates duplicate writes. Move
this logic to use the pre_update_option_frm_paypal_options filter instead, which
executes before the option is written. Remove the update_option call since the
filter can directly modify and return the new_value parameter, allowing the
preservation to happen in a single update operation without re-triggering the
hook.


$new_value->test_mode = $old_value->test_mode;
// Update the option with the preserved test_mode
update_option( 'frm_paypal_options', $new_value );
}
}
7 changes: 4 additions & 3 deletions paypal/models/FrmPayPalLiteSettings.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,15 @@ public function get_options() {
*/
public function update( $params ) {
$settings = $this->default_options();
$param = $this->param();

foreach ( $settings as $setting => $default ) {
if ( isset( $params[ 'frm_' . $this->param() . '_' . $setting ] ) ) {
$this->settings->{$setting} = sanitize_text_field( $params[ 'frm_' . $this->param() . '_' . $setting ] );
if ( isset( $params[ 'frm_' . $param . '_' . $setting ] ) ) {
$this->settings->{$setting} = sanitize_text_field( $params[ 'frm_' . $param . '_' . $setting ] );
}
}

$this->settings->test_mode = isset( $params[ 'frm_' . $this->param() . '_test_mode' ] ) ? absint( $params[ 'frm_' . $this->param() . '_test_mode' ] ) : 0;
$this->settings->test_mode = isset( $params[ 'frm_' . $param . '_test_mode' ] ) ? absint( $params[ 'frm_' . $param . '_test_mode' ] ) : 0;
}

/**
Expand Down
1 change: 1 addition & 0 deletions paypal/views/settings/connect-settings-container.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
</td>
<td>
<label>
<input type="hidden" name="frm_paypal_test_mode" value="0" />
<input type="checkbox" name="frm_paypal_test_mode" id="frm_paypal_test_mode" value="1" <?php checked( $settings->settings->test_mode, 1 ); ?> />
<?php esc_html_e( 'Use the PayPal test mode', 'formidable' ); ?>
</label>
Expand Down
Loading