fix: bugfix/pcp-issues-sept-2025 #1709
Conversation
arifulhoque7
added
needs: testing
needs: dev review
WalkthroughThis pull request systematically enhances security and WordPress standards compliance across the codebase. Changes include adding output escaping (esc_html, esc_url, esc_attr, esc_js) throughout to prevent XSS vulnerabilities, replacing inline script tags with WordPress script enqueuing, refactoring database queries to use prepared statements, simplifying HTTP calls via WordPress API, adding nonce verification, and improving localization with translator comments and placeholder patterns. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Despite the 30+ file count, the changes are predominantly systematic and repetitive (output escaping applied consistently across templates and rendering functions). The homogeneous nature of most edits (wrapping outputs with esc_* functions following predictable patterns) reduces complexity. Higher-complexity items include the reCaptcha script enqueuing refactor (Lib/recaptchalib.php), nonce verification additions, and database query simplifications (List_Table_Subscribers.php, Subscription.php), but these represent a smaller fraction of the total changes. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
includes/Integrations/Events_Calendar/Compatibility/TEC_V6_Compatibility.php (1)
153-159: Avoid gmdate for all-day end dates – breaks validation in non-UTC timezones.Switching to
gmdate()forces the computed end date into UTC. On sites whose timezone offset is > 0 (e.g., UTC+6),gmdate()turns a start date of2025-09-29 00:00:00into an end date of2025-09-28 23:59:59, sovalidate_orm_requirements()seesend_timestamp <= start_timestampand the event fails to save. Keep this conversion in the site timezone (wp_date()/date()) instead of UTC.- $end_date = gmdate( 'Y-m-d 23:59:59', strtotime( $start_date ) ); + $end_date = wp_date( 'Y-m-d 23:59:59', strtotime( $start_date ) );class/render-form.php (1)
1792-1795: Escaping breaks classic reCAPTCHA outputrecaptcha_get_html returns HTML that must not be escaped. Current esc_html will render markup as text and break the widget.
- <?php echo esc_html( recaptcha_get_html( wpuf_get_option( 'recaptcha_public', 'wpuf_general' ), $enable_no_captcha, null, is_ssl() ) ); ?> + <?php echo recaptcha_get_html( wpuf_get_option( 'recaptcha_public', 'wpuf_general' ), $enable_no_captcha, null, is_ssl() ); ?>includes/Render_Form.php (1)
1683-1686: Escaping breaks classic reCAPTCHA outputesc_html will break the widget; output the HTML unescaped.
- <?php echo esc_html( recaptcha_get_html( wpuf_get_option( 'recaptcha_public', 'wpuf_general' ), $enable_no_captcha, null, is_ssl() ) ); ?> + <?php echo recaptcha_get_html( wpuf_get_option( 'recaptcha_public', 'wpuf_general' ), $enable_no_captcha, null, is_ssl() ); ?>Lib/recaptchalib_noCaptcha.php (1)
131-141: Guard against transport/JSON failuresIf _submitHTTPGet returns false or JSON decoding fails, access to $answers['success'] will error. Add defensive checks and map to a failure response.
- $answers = json_decode($getResponse, true); + if ( false === $getResponse ) { + $recaptchaResponse = new WPUF_ReCaptchaResponse(); + $recaptchaResponse->success = false; + $recaptchaResponse->errorCodes = 'request-failed'; + return $recaptchaResponse; + } + $answers = json_decode( $getResponse, true ); + if ( ! is_array( $answers ) ) { + $recaptchaResponse = new WPUF_ReCaptchaResponse(); + $recaptchaResponse->success = false; + $recaptchaResponse->errorCodes = 'invalid-json'; + return $recaptchaResponse; + }admin/form-builder/views/post-form-settings.php (2)
331-337: Value loading bug: dynamic name value is overwritten.The else branch reassigns $value from $form_settings[$field_key], discarding earlier dynamic-name resolution. Breaks fields using nested keys.
Apply this diff:
- if ( $is_pro_preview ) { - $value = ! empty( $field['value'] ) ? $field['value'] : $value; - } else { - $value = isset( $form_settings[ $field_key ] ) ? $form_settings[ $field_key ] : $value; // checking with isset because saved value can be empty string - } + if ( $is_pro_preview ) { + $value = ! empty( $field['value'] ) ? $field['value'] : $value; + }
374-374: Use esc_url for image src attributes (URL context).esc_attr doesn’t perform URL sanitization; esc_url is the right choice for src/href.
Apply this diff:
- <img class="wpuf-ml-2" src="<?php echo esc_attr( $pro_badge ); ?>" alt=""> + <img class="wpuf-ml-2" src="<?php echo esc_url( $pro_badge ); ?>" alt="">- <img + <img class="wpuf-absolute wpuf-opacity-0 peer-checked:wpuf-opacity-100 wpuf-top-[7%] wpuf-right-[12%] wpuf-transition-all wpuf-duration-200 wpuf-ease-in-out" - src="<?php echo esc_attr( WPUF_ASSET_URI . '/images/checked-green.svg' ); ?>" alt=""> + src="<?php echo esc_url( WPUF_ASSET_URI . '/images/checked-green.svg' ); ?>" alt="">As per WordPress coding standards.
Also applies to: 512-512
Lib/Gateway/Paypal.php (2)
410-414: Rewrite rule target is malformed.'index.php?action=webhook_triggered=1' sets action to 'webhook_triggered=1'. It should be 'index.php?action=webhook_triggered'.
- 'index.php?action=webhook_triggered=1', + 'index.php?action=webhook_triggered',
435-476: Webhook condition never matches.You’re comparing the literal string 'action' instead of the query var value. This prevents the handler from running.
- if ( 'action' === get_query_var( 'action' ) && + if ( get_query_var( 'action' ) === 'webhook_triggered' && isset( $_SERVER['REQUEST_METHOD'] ) && 'POST' === $_SERVER['REQUEST_METHOD'] && isset( $_SERVER['HTTP_PAYPAL_TRANSMISSION_ID'] ) ) {
🧹 Nitpick comments (29)
includes/Admin/template-parts/modal-v4.1.php (1)
21-22: Ensure translator comment is parsed by GlotPress
// translators:comments are ignored by the WordPress i18n tools. Switch to the block comment form so the context actually reaches translators.- // translators: %s is the form type (e.g., 'Post', 'Registration') + /* translators: %s is the form type (e.g., 'Post', 'Registration') */includes/Integrations/Events_Calendar/Validators/Date_Validator.php (2)
104-108: Clarify translator comment for time field placeholder.
Line 105 lists “StartTime/EndTime”, but the runtime placeholder is the raw meta key (e.g.,EventStartHour). Please update the example so translators see the actual value they need to interpret.- // translators: %s is the field name (e.g., StartTime, EndTime) + // translators: %s is the field key (e.g., EventStartHour, EventEndMinute)
340-344: Align translator comment with emitted placeholder.
Line 341 claims the placeholder is “StartDate/EndDate”, but the code passes only “Start” or “End”. Let’s correct the guidance so translators don’t expect the removed “Date” suffix.- // translators: %s is the field name (e.g., StartDate, EndDate) + // translators: %s is the field label fragment (e.g., Start, End)class/render-form.php (3)
1764-1764: Good switch to wp_enqueue_script; also localize the reCAPTCHA languageUse the site’s locale instead of a hard-coded hl=en for better i18n.
- <?php wp_enqueue_script( 'wpuf-recaptcha-invisible', 'https://www.google.com/recaptcha/api.js?onload=wpufreCaptchaLoaded&render=explicit&hl=en', array(), null, true ); ?> + <?php + $recaptcha_src = add_query_arg( + array( + 'onload' => 'wpufreCaptchaLoaded', + 'render' => 'explicit', + 'hl' => substr( determine_locale(), 0, 2 ), + ), + 'https://www.google.com/recaptcha/api.js' + ); + wp_enqueue_script( 'wpuf-recaptcha-invisible', $recaptcha_src, array(), null, true ); + ?>
1767-1767: Escape for JS contextThis is inside a JS string literal; esc_js is the correct context.
- jQuery('[name="submit"]').removeClass('wpuf-submit-button').addClass('g-recaptcha').attr('data-sitekey', '<?php echo esc_html( wpuf_get_option( 'recaptcha_public', 'wpuf_general' ) ); ?>'); + jQuery('[name="submit"]').removeClass('wpuf-submit-button').addClass('g-recaptcha').attr('data-sitekey', '<?php echo esc_js( wpuf_get_option( 'recaptcha_public', 'wpuf_general' ) ); ?>');
1790-1791: Mismatch between data-callback and actual callbackThe div uses data-callback="onSubmit" but the code defines wpufRecaptchaCallback. Align them to avoid confusion.
- <div type="submit" id='recaptcha' class="g-recaptcha" data-sitekey=<?php echo esc_attr( wpuf_get_option( 'recaptcha_public', 'wpuf_general' ) ); ?> data-callback="onSubmit" data-size="invisible"></div> + <div type="submit" id='recaptcha' class="g-recaptcha" data-sitekey="<?php echo esc_attr( wpuf_get_option( 'recaptcha_public', 'wpuf_general' ) ); ?>" data-callback="wpufRecaptchaCallback" data-size="invisible"></div>Lib/invisible_recaptcha.php (2)
26-28: PHPCS indent warning + prefer wp_die for WP contextFix the reported indentation and consider wp_die for a nicer error in WP.
- die("To use reCAPTCHA you must get an API key from <a href='" - . esc_url( self::$_signupUrl ) . "'>" . esc_html( self::$_signupUrl ) . "</a>"); + wp_die( + sprintf( + /* translators: %s: URL to Google reCAPTCHA admin */ + esc_html__( 'To use reCAPTCHA you must get an API key from %s', 'wp-user-frontend' ), + '<a href="' . esc_url( self::$_signupUrl ) . '" target="_blank" rel="noopener noreferrer">' . esc_html( self::$_signupUrl ) . '</a>' + ) + );Also re-indent the concatenation to satisfy PHPCS as hinted. Based on static analysis hints
91-97: Avoid file_get_contents for HTTP; use WP HTTP API with timeoutsThis network call lacks timeout/error handling. Align with your noCaptcha lib update by switching to wp_remote_get.
- $url = 'https://www.google.com/recaptcha/api/siteverify?'.http_build_query($data); - $response = file_get_contents($url); - - return $response; + $url = 'https://www.google.com/recaptcha/api/siteverify?' . http_build_query($data, '', '&', PHP_QUERY_RFC3986); + $response = wp_remote_get( $url, array( + 'timeout' => 5, + // Leave sslverify default (true) for security + ) ); + if ( is_wp_error( $response ) ) { + return false; + } + return wp_remote_retrieve_body( $response );admin/form-builder/assets/js/components/form-column_field/template.php (1)
108-109: Use esc_url for URLs (not esc_attr)For src attributes, esc_url is the appropriate escaping.
- <img src="<?php echo esc_attr( WPUF_ASSET_URI . '/images/pro-badge.svg' ); ?>" alt=""> + <img src="<?php echo esc_url( WPUF_ASSET_URI . '/images/pro-badge.svg' ); ?>" alt="">admin/form-builder/assets/js/components/form-fields-v4-1/template.php (1)
75-77: Use esc_url for the image srcSwap esc_attr for esc_url for URL escaping.
- <img src="<?php echo esc_attr( WPUF_ASSET_URI . '/images/pro-badge.svg' ); ?>" alt=""> + <img src="<?php echo esc_url( WPUF_ASSET_URI . '/images/pro-badge.svg' ); ?>" alt="">includes/Render_Form.php (3)
1655-1655: Load reCAPTCHA with locale awarenessMatch the site language instead of hard-coded hl=en.
- <?php wp_enqueue_script( 'wpuf-recaptcha-invisible', 'https://www.google.com/recaptcha/api.js?onload=wpufreCaptchaLoaded&render=explicit&hl=en', array(), null, true ); ?> + <?php + $recaptcha_src = add_query_arg( + array( + 'onload' => 'wpufreCaptchaLoaded', + 'render' => 'explicit', + 'hl' => substr( determine_locale(), 0, 2 ), + ), + 'https://www.google.com/recaptcha/api.js' + ); + wp_enqueue_script( 'wpuf-recaptcha-invisible', $recaptcha_src, array(), null, true ); + ?>
1658-1659: Escape for JS contextUse esc_js for values embedded in JS string literals.
- jQuery('[name="submit"]').removeClass('wpuf-submit-button').addClass('g-recaptcha').attr('data-sitekey', '<?php echo esc_html( wpuf_get_option( 'recaptcha_public', 'wpuf_general' ) ); ?>'); + jQuery('[name="submit"]').removeClass('wpuf-submit-button').addClass('g-recaptcha').attr('data-sitekey', '<?php echo esc_js( wpuf_get_option( 'recaptcha_public', 'wpuf_general' ) ); ?>');
1681-1682: Align data-callback with actual callbackThe div declares onSubmit but the code defines wpufRecaptchaCallback.
- <div type="submit" id='recaptcha' class="g-recaptcha" data-sitekey=<?php echo esc_attr( wpuf_get_option( 'recaptcha_public', 'wpuf_general' ) ); ?> data-callback="onSubmit" data-size="invisible"></div> + <div type="submit" id='recaptcha' class="g-recaptcha" data-sitekey="<?php echo esc_attr( wpuf_get_option( 'recaptcha_public', 'wpuf_general' ) ); ?>" data-callback="wpufRecaptchaCallback" data-size="invisible"></div>admin/form-builder/assets/js/components/builder-stage-v4-1/template.php (1)
114-115: Use esc_url for image srcSwap esc_attr for esc_url for proper URL escaping.
- <img src="<?php echo esc_attr( WPUF_ASSET_URI . '/images/pro-badge.svg' ); ?>" alt=""> + <img src="<?php echo esc_url( WPUF_ASSET_URI . '/images/pro-badge.svg' ); ?>" alt="">admin/form-builder/views/post-form-settings.php (3)
64-65: Prefer esc_js for values embedded in inline Vue expressions.Safer for JS-string contexts than esc_attr.
Apply this diff:
- @click="switch_settings_menu('<?php echo esc_attr( $key ); ?>', '<?php echo esc_attr( $sub_key ); ?>')" - :class="active_settings_tab === '<?php echo esc_attr( $sub_key ); ?>' ? 'wpuf-bg-primary active_settings_tab' : ''" + @click="switch_settings_menu('<?php echo esc_js( $key ); ?>', '<?php echo esc_js( $sub_key ); ?>')" + :class="active_settings_tab === '<?php echo esc_js( $sub_key ); ?>' ? 'wpuf-bg-primary active_settings_tab' : ''" - :class="active_settings_tab === '<?php echo esc_attr( $sub_key ); ?>' ? 'wpuf-text-white' : 'wpuf-text-gray-600'" + :class="active_settings_tab === '<?php echo esc_js( $sub_key ); ?>' ? 'wpuf-text-white' : 'wpuf-text-gray-600'"Also applies to: 70-70
506-509: Radio inputs lack ids; label “for” mismatched (a11y).Associate each radio with a unique id and point the text label to it.
Apply this diff:
- <input - type="radio" - name="<?php echo esc_attr( $name ); ?>" - value="<?php echo esc_attr( $key ); ?>" - <?php echo esc_attr( checked( $value, $key, false ) ); ?> - class="wpuf-absolute wpuf-opacity-0 wpuf-peer"> + <input + type="radio" + id="<?php echo esc_attr( "{$field_key}_{$key}" ); ?>" + name="<?php echo esc_attr( $name ); ?>" + value="<?php echo esc_attr( $key ); ?>" + <?php echo esc_attr( checked( $value, $key, false ) ); ?> + class="wpuf-absolute wpuf-opacity-0 wpuf-peer"> ... - <label - for="<?php echo esc_attr( $field_key ); ?>" + <label + for="<?php echo esc_attr( "{$field_key}_{$key}" ); ?>"Also applies to: 519-521
569-571: Avoid double-escaping JSON in Vue prop binding.Use wp_json_encode with HEX flags and single quotes to safely embed the object literal.
Apply this diff:
- <submit-button-conditional-logics - :current-settings="<?php echo esc_attr( wp_json_encode( $submit_button_settings ) ); ?>"> + <submit-button-conditional-logics + :current-settings='<?php echo wp_json_encode( $submit_button_settings, JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT ); ?>'>includes/Integrations/TEC_Venue_Organizer_Fix.php (1)
59-60: Avoid esc_js for full inline scripts; use wp_add_inline_script (safer, cleaner).esc_js here over-escapes a complete script block and can make maintenance harder. Prefer registering a handle and injecting via wp_add_inline_script.
Apply this change within output_inline_script:
- echo '<script type="text/javascript">' . esc_js( $this->get_inline_script() ) . '</script>'; + // Register a dummy handle once, then attach inline script + if ( ! wp_script_is( 'wpuf-tec-venue-organizer-fix', 'registered' ) ) { + wp_register_script( 'wpuf-tec-venue-organizer-fix', false, [ 'jquery' ], null, true ); + } + wp_add_inline_script( 'wpuf-tec-venue-organizer-fix', $this->get_inline_script() ); + wp_enqueue_script( 'wpuf-tec-venue-organizer-fix' );If you prefer to keep direct output, drop esc_js:
- echo '<script type="text/javascript">' . esc_js( $this->get_inline_script() ) . '</script>'; + echo '<script type="text/javascript">' . $this->get_inline_script() . '</script>';admin/form-builder/assets/js/components/field-option-pro-feature-alert/template.php (1)
6-7: Nice escape; also add rel to new-tab link.Good use of esc_url on the icon. For target="_blank", add rel="noopener noreferrer".
- <a :href="pro_link" target="_blank"><img src="<?php echo esc_url( wpuf_get_pro_icon() ) ?>" alt="pro icon"></a> + <a :href="pro_link" target="_blank" rel="noopener noreferrer"><img src="<?php echo esc_url( wpuf_get_pro_icon() ) ?>" alt="pro icon"></a>Lib/Gateway/Bank.php (1)
127-129: Format the amount and fix grammar.Use a formatted amount for i18n and currency display, and remove the extra space before the period.
- // translators: %s is the payment amount - $msg .= sprintf( __( 'We have received your payment amount of %s through bank . ', 'wp-user-frontend' ), $transaction['cost'] ) . "\r\n\r\n"; + // translators: %s is the payment amount + $amount = isset( $transaction['cost'] ) ? (float) $transaction['cost'] : 0; + // Prefer wpuf_format_price if available; otherwise fallback to number_format_i18n + $formatted = function_exists( 'wpuf_format_price' ) ? wpuf_format_price( $amount ) : number_format_i18n( $amount, 2 ); + $msg .= sprintf( __( 'We have received your bank payment of %s.', 'wp-user-frontend' ), $formatted ) . "\r\n\r\n";includes/Admin/views/need-to-update.php (1)
7-9: Good URL escaping; add rel for external link.Escaping admin_url is correct. For the external link with target="_blank", add rel="noopener noreferrer".
- <a href="<?php echo esc_url( 'https://wedevs.com/blog/504745/wp-user-frontend-release-march-2025/' ); ?>" target="_blank" class="wpuf-btn-secondary">Learn more <span aria-hidden="true">→</span></a> + <a href="<?php echo esc_url( 'https://wedevs.com/blog/504745/wp-user-frontend-release-march-2025/' ); ?>" target="_blank" rel="noopener noreferrer" class="wpuf-btn-secondary">Learn more <span aria-hidden="true">→</span></a>includes/Ajax/Admin_Form_Builder_Ajax.php (1)
189-191: Escape placeholders in label and use esc_html__ for the string.$post_type and $tax->label should be escaped before output to admin HTML. Use esc_html__ to keep the string safe.
- // translators: %1$s is the post type, %2$s is the taxonomy label - $cat .= sprintf( __( 'Default %1$s %2$s', 'wp-user-frontend' ), $post_type, $tax->label ); + // translators: %1$s is the post type, %2$s is the taxonomy label + $cat .= sprintf( esc_html__( 'Default %1$s %2$s', 'wp-user-frontend' ), esc_html( $post_type ), esc_html( $tax->label ) );Lib/recaptchalib.php (1)
38-39: Optional: default to HTTPS for API server.RECAPTCHA_API_SERVER uses http; prefer https to avoid mixed content.
-define("RECAPTCHA_API_SERVER", "http://www.google.com/recaptcha/api"); +define("RECAPTCHA_API_SERVER", "https://www.google.com/recaptcha/api");Lib/Gateway/Paypal.php (3)
2075-2103: Duplicate webhook registration/handling.Bottom-of-file rewrite + handlers duplicate what the class already registers. This risks config drift and double flushing. Keep one implementation (prefer the class-based one) and remove the duplicate.
484-501: Header retrieval fallback may miss PayPal headers.array_change_key_case($SERVER) yields keys like http_paypal_transmission_id, not paypal-transmission-id. Consider normalizing HTTP prefixed keys to expected names to make verification robust on servers lacking getallheaders.
2069-2070: Avoid escaping when building exception messages.Escape at output, not at throw-site, to keep messages raw for logs/handlers.
- throw new \Exception( 'Error handling subscription activation: ' . esc_html( $e->getMessage() ) ); + throw new \Exception( 'Error handling subscription activation: ' . $e->getMessage() );includes/Fields/Form_Field_reCaptcha.php (3)
72-72: Localize reCAPTCHA language.Pass current locale to api.js to match site language.
- <?php wp_enqueue_script( 'wpuf-recaptcha-invisible', 'https://www.google.com/recaptcha/api.js?onload=wpufreCaptchaLoaded&render=explicit&hl=en', array(), null, true ); ?> + <?php wp_enqueue_script( + 'wpuf-recaptcha-invisible', + 'https://www.google.com/recaptcha/api.js?onload=wpufreCaptchaLoaded&render=explicit&hl=' . rawurlencode( substr( get_locale(), 0, 2 ) ), + array(), + null, + true + ); ?>
89-99: Ensure the onload callback is globally accessible.Define on window to guarantee availability when Google calls it.
- var wpufreCaptchaLoaded = function() { + window.wpufreCaptchaLoaded = function() {
105-106: Localize non-invisible reCAPTCHA too.Add hl param based on site locale.
- <?php wp_enqueue_script( 'wpuf-recaptcha', 'https://www.google.com/recaptcha/api.js', array(), null, true ); ?> + <?php wp_enqueue_script( + 'wpuf-recaptcha', + 'https://www.google.com/recaptcha/api.js?hl=' . rawurlencode( substr( get_locale(), 0, 2 ) ), + array(), + null, + true + ); ?>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
Lib/Appsero/License.php(1 hunks)Lib/Gateway/Bank.php(1 hunks)Lib/Gateway/Paypal.php(4 hunks)Lib/invisible_recaptcha.php(1 hunks)Lib/recaptchalib.php(1 hunks)Lib/recaptchalib_noCaptcha.php(2 hunks)admin/form-builder/assets/js/components/builder-stage-v4-1/template.php(3 hunks)admin/form-builder/assets/js/components/field-option-pro-feature-alert/template.php(1 hunks)admin/form-builder/assets/js/components/field-visibility/template.php(2 hunks)admin/form-builder/assets/js/components/form-column_field/template.php(1 hunks)admin/form-builder/assets/js/components/form-fields-v4-1/template.php(1 hunks)admin/form-builder/views/form-builder-v4.1.php(3 hunks)admin/form-builder/views/post-form-settings.php(18 hunks)assets/js-templates/form-components.php(8 hunks)class/render-form.php(1 hunks)class/subscription.php(1 hunks)includes/Admin/Forms/Admin_Form.php(1 hunks)includes/Admin/Forms/Admin_Form_Builder.php(1 hunks)includes/Admin/List_Table_Subscribers.php(5 hunks)includes/Admin/Subscription.php(4 hunks)includes/Admin/template-parts/modal-v4.1.php(2 hunks)includes/Admin/template-parts/modal-v4.2.php(1 hunks)includes/Admin/views/need-to-update.php(1 hunks)includes/Ajax/Admin_Form_Builder_Ajax.php(1 hunks)includes/Fields/Form_Field_Post_Tags.php(1 hunks)includes/Fields/Form_Field_reCaptcha.php(2 hunks)includes/Fields/Form_Pro_Upgrade_Fields.php(1 hunks)includes/Frontend/Payment.php(3 hunks)includes/Frontend_Render_Form.php(2 hunks)includes/Integrations/Events_Calendar/Compatibility/TEC_V6_Compatibility.php(2 hunks)includes/Integrations/Events_Calendar/Templates/Event_Form_Template.php(1 hunks)includes/Integrations/Events_Calendar/Validators/Date_Validator.php(4 hunks)includes/Integrations/Events_Calendar/Validators/Event_Validator.php(6 hunks)includes/Integrations/TEC_Venue_Organizer_Fix.php(1 hunks)includes/Render_Form.php(3 hunks)readme.txt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
Lib/Appsero/License.php (1)
Lib/Appsero/Client.php (1)
__trans(242-244)
admin/form-builder/views/post-form-settings.php (3)
class/render-form.php (1)
label(880-892)includes/Render_Form.php (1)
label(742-754)wpuf-functions.php (2)
wpuf_is_pro_active(4990-4992)wpuf_get_pro_icon(4979-4981)
includes/Admin/Subscription.php (1)
includes/User_Subscription.php (1)
current_pack(53-73)
admin/form-builder/views/form-builder-v4.1.php (1)
wpuf-functions.php (1)
get_wpuf_preview_page(4244-4286)
admin/form-builder/assets/js/components/field-option-pro-feature-alert/template.php (1)
wpuf-functions.php (1)
wpuf_get_pro_icon(4979-4981)
assets/js-templates/form-components.php (1)
wpuf-functions.php (1)
wpuf_get_pro_icon(4979-4981)
class/subscription.php (1)
includes/User_Subscription.php (1)
current_pack(53-73)
🪛 GitHub Check: Run PHPCS inspection
Lib/invisible_recaptcha.php
[failure] 27-27:
Multi-line function call not indented correctly; expected 12 spaces but found 16
admin/form-builder/views/form-builder-v4.1.php
[failure] 62-62:
Closing PHP tag must be on a line by itself
[failure] 61-61:
Opening PHP tag must be on a line by itself
[failure] 61-61:
Opening PHP tag must be on a line by itself
🔇 Additional comments (25)
includes/Fields/Form_Field_Post_Tags.php (1)
37-66: Escaping the query string is correctUsing
esc_js( $query_string )before embedding it in the inline script properly shields the AJAX URL from injection vectors. Looks good.includes/Admin/template-parts/modal-v4.2.php (1)
102-104: Translator context is clearThanks for adding the translators’ note—it accurately clarifies what
%srepresents, which will help avoid mistranslations.includes/Fields/Form_Pro_Upgrade_Fields.php (1)
95-100: Text domain alignment looks good.Switching the phone field label to the
wp-user-frontenddomain matches the surrounding field definitions and keeps translations consistent.includes/Integrations/Events_Calendar/Templates/Event_Form_Template.php (1)
294-303: Nice use of numbered placeholders for translators.Switching to
%1$s/%2$splus the translator note keeps the string safe for reordering without touching runtime behavior.includes/Integrations/Events_Calendar/Validators/Event_Validator.php (1)
74-333: Translator comments look good.The inline notes document each placeholder clearly while leaving the validation flow untouched.
class/subscription.php (1)
897-901: Good: escaped, translatable activation messages.Using esc_html_e/esc_html__ and a translator comment is correct and consistent with WP i18n/escaping practices.
includes/Admin/Subscription.php (2)
784-785: Good: removed redundant prepare and second query.Reusing the already-prepared $sql with a single get_row call is cleaner and avoids duplicate work.
931-935: Good: activation messages use proper escaping and translator note.printf with esc_html__ and escaping the pack title via esc_html is correct.
includes/Admin/Forms/Admin_Form.php (1)
396-409: i18n placeholder handling looks correctUsing sprintf with a translated format string and explicit translator comment is good.
includes/Render_Form.php (2)
869-871: Nice: correct JS escapingSwitching to esc_js for the AJAX query string is appropriate.
1501-1503: Nice: correct JS escapingConsistent esc_js usage for tag suggestions URL.
admin/form-builder/assets/js/components/builder-stage-v4-1/template.php (1)
3-3: LGTM: correct URL escapingesc_url for the image src is appropriate.
admin/form-builder/views/post-form-settings.php (2)
83-83: LGTM on esc_url for the PRO badge source.Correct URL escaping for the image src.
128-133: Good, consistent escaping for attributes and text.esc_attr for class/data-* and esc_html for labels/descriptions are appropriate.
readme.txt (1)
6-6: Confirm “Tested up to” regression (6.8 vs 6.8.2).This lowers the “Tested up to” version. If 6.8.2 is already verified, keep the highest tested patch version; otherwise confirm 6.8 is intentional.
Lib/Appsero/License.php (1)
275-276: LGTM: escape dynamic name in activation notice.Escaping the client name before printf is correct and consistent with wp_kses_post on the string.
includes/Frontend_Render_Form.php (1)
153-154: Likewise, print footer scripts via wp_footer().wp_enqueue_script('jquery') inside the head doesn’t output tags here. Use wp_footer() to flush footer enqueued scripts.
- <?php wp_enqueue_script( 'jquery' ); ?> + <?php wp_footer(); ?>Also ensure jQuery is enqueued before rendering (line 124 is fine).
Likely an incorrect or invalid review comment.
admin/form-builder/views/form-builder-v4.1.php (2)
8-8: LGTM on asset URL escaping.Using esc_url for the image source is correct.
110-112: LGTM on preview URL escaping.esc_url around get_wpuf_preview_page() is appropriate in a bound :href.
includes/Admin/Forms/Admin_Form_Builder.php (1)
335-339: LGTM: keep HTML outside the translatable string.This improves translator experience and avoids HTML in i18n strings.
Lib/Gateway/Paypal.php (1)
132-133: LGTM: escape nonce for JS context.esc_js around wp_create_nonce is correct.
assets/js-templates/form-components.php (1)
92-92: LGTM: esc_url for asset path.Correctly escaped image source.
includes/Frontend/Payment.php (3)
631-639: LGTM: clearer i18n with placeholders and translator comments.Good use of positional placeholders and translator notes to avoid ambiguity.
Also applies to: 642-651, 654-663, 666-675
827-836: LGTM: invoice item name translator notes.Helpful translator comments for placeholders.
860-863: LGTM: default subject with translator note.Appropriate fallback with site name.
| echo esc_html( $partially_filtered ); | ||
| } |
There was a problem hiding this comment.
Stop escaping the rendered role/subscription markup
esc_html() turns the already-sanitized markup into a literal string, so the <input> elements render as plain text and the visibility checkboxes/radios become unusable. We need the HTML to pass through exactly as filtered by wp_kses() (plus the reinstated Vue bindings).
- echo esc_html( $partially_filtered );
+ echo $partially_filtered;
...
- echo esc_html( $partially_filtered );
+ echo $partially_filtered;Also applies to: 132-133
🤖 Prompt for AI Agents
In admin/form-builder/assets/js/components/field-visibility/template.php around
lines 84-85 (also apply same fix at 132-133), the code calls esc_html() on
markup that was already sanitized with wp_kses(), which escapes HTML and renders
inputs as literal text; remove esc_html() and output the filtered markup
directly so the sanitized HTML (including inputs and Vue bindings) is rendered
as intended, ensuring you only echo the wp_kses()-filtered variable (or use echo
wp_kses_post()/wp_kses() result) without additional escaping.
|
16 errors still remains @arifulhoque7 vai |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
class/subscription.php (1)
1146-1164: Fix SQL placeholders for string status and reduce duplicationsubscribtion_status is a string; %d coerces to 0. Also branch-heavy. Use %s and prepare only when needed.
- if ( $pack_id && $status ) { - $rows = $wpdb->get_results( $wpdb->prepare( - 'SELECT user_id FROM ' . $wpdb->prefix . 'wpuf_subscribers WHERE subscribtion_id = %d AND subscribtion_status = %d', - $pack_id, $status - ) ); - } elseif ( $pack_id ) { - $rows = $wpdb->get_results( $wpdb->prepare( - 'SELECT user_id FROM ' . $wpdb->prefix . 'wpuf_subscribers WHERE subscribtion_id = %d', - $pack_id - ) ); - } elseif ( $status ) { - $rows = $wpdb->get_results( $wpdb->prepare( - 'SELECT user_id FROM ' . $wpdb->prefix . 'wpuf_subscribers WHERE subscribtion_status = %d', - $status - ) ); - } else { - $rows = $wpdb->get_results( 'SELECT user_id FROM ' . $wpdb->prefix . 'wpuf_subscribers' ); - } + $sql = 'SELECT user_id FROM ' . $wpdb->prefix . 'wpuf_subscribers'; + $wheres = []; + $vals = []; + if ( $pack_id ) { + $wheres[] = 'subscribtion_id = %d'; + $vals[] = (int) $pack_id; + } + if ( $status ) { + $wheres[] = 'subscribtion_status = %s'; + $vals[] = $status; + } + if ( $wheres ) { + $sql .= ' WHERE ' . implode( ' AND ', $wheres ); + } + $rows = $vals ? $wpdb->get_results( $wpdb->prepare( $sql, ...$vals ) ) : $wpdb->get_results( $sql );This mirrors the safer pattern used elsewhere in the PR.
includes/Admin/Subscription.php (3)
423-427: Critical: stray string in saved expiration_time" subscription.php" was accidentally concatenated into the meta value.
- $expiration_time = sanitize_text_field( wp_unslash( $post_data['post_expiration_settings']['expiration_time_value'] ) ) . ' subscription.php' . sanitize_text_field( wp_unslash( $post_data['post_expiration_settings']['expiration_time_type'] ) ); + $expiration_time = sanitize_text_field( wp_unslash( $post_data['post_expiration_settings']['expiration_time_value'] ) ) . ' ' . sanitize_text_field( wp_unslash( $post_data['post_expiration_settings']['expiration_time_type'] ) );
1377-1382: Inconsistent status capitalization (‘Free’ vs ‘free’)Elsewhere this codebase uses lower-case 'free' (and mixed-case 'Completed'/'Cancel' in admin views). Pick one canonical value and use it everywhere; otherwise filters and counts won’t align.
- 'subscribtion_status' => 'Free', - 'gateway' => 'Free', - 'transaction_id' => 'Free', + 'subscribtion_status' => 'free', + 'gateway' => 'free', + 'transaction_id' => 'free',If the canonical DB values are mixed-case (e.g., 'Completed','Cancel','Free'), align the rest of the code accordingly instead. Please standardize across inserts, updates, filters, and view links.
171-174: Include thewpuf_register_formnonce in subscription signup URLsThe
register_form()hook now early-returns unless$_GET['_wpnonce']is valid. Inclass/subscription.php(around lines 1003–1006), add'_wpnonce' => wp_create_nonce('wpuf_register_form')to the$query_argsarray so your “Sign Up”/“Buy Now” links append the required nonce.
🧹 Nitpick comments (4)
includes/Admin/List_Table_Subscribers.php (4)
18-28: Make nonce check optional and normalize with sanitize_keyAvoid treating missing nonce as a failure. Only verify when provided to reduce log noise; use sanitize_key for nonce normalization.
- private function verify_nonce() { - if ( ! isset( $_REQUEST['_wpnonce'] ) ) { - return false; - } - return wp_verify_nonce( sanitize_text_field( wp_unslash( $_REQUEST['_wpnonce'] ) ), 'wpuf_subscribers_list' ); - } + private function verify_nonce() { + if ( ! isset( $_REQUEST['_wpnonce'] ) ) { + // Nonce not provided; treat as pass to avoid log noise on read-only views + return true; + } + $nonce = sanitize_key( wp_unslash( $_REQUEST['_wpnonce'] ) ); + return (bool) wp_verify_nonce( $nonce, 'wpuf_subscribers_list' ); + }
134-140: Gate logs behind presence of _wpnonceLog only when a nonce was provided but failed verification.
- if ( ! $this->verify_nonce() ) { + if ( isset( $_REQUEST['_wpnonce'] ) && ! $this->verify_nonce() ) { if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { error_log( 'WPUF Subscribers: Nonce verification failed for column_cb function' ); } }
153-159: Same: avoid logging when nonce is absentApply the same gating here.
- if ( ! $this->verify_nonce() ) { + if ( isset( $_REQUEST['_wpnonce'] ) && ! $this->verify_nonce() ) { if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { error_log( 'WPUF Subscribers: Nonce verification failed for get_views function' ); } }
185-191: Same: avoid logging when nonce is absentApply the same gating here.
- if ( ! $this->verify_nonce() ) { + if ( isset( $_REQUEST['_wpnonce'] ) && ! $this->verify_nonce() ) { if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { error_log( 'WPUF Subscribers: Nonce verification failed for prepare_items function' ); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
class/subscription.php(3 hunks)includes/Admin/List_Table_Subscribers.php(5 hunks)includes/Admin/Subscription.php(5 hunks)includes/Frontend/Payment.php(3 hunks)includes/User_Subscription.php(1 hunks)includes/upgrades/upgrade-2.6.0.php(0 hunks)
💤 Files with no reviewable changes (1)
- includes/upgrades/upgrade-2.6.0.php
🚧 Files skipped from review as they are similar to previous changes (1)
- includes/Frontend/Payment.php
🧰 Additional context used
🧬 Code graph analysis (2)
class/subscription.php (1)
includes/User_Subscription.php (1)
current_pack(53-73)
includes/Admin/Subscription.php (1)
includes/User_Subscription.php (1)
current_pack(53-73)
🔇 Additional comments (10)
includes/User_Subscription.php (1)
224-227: Good: consolidated prepare + get_rowSafer single-call pattern; placeholders/types are correct.
class/subscription.php (3)
58-62: Good: simplified prepared get_rowConsolidation improves readability and safety.
741-750: Good: direct prepared queryReturning the prepared get_row directly is clear and safe.
883-887: LGTM: escaped output for active pack noticeUses esc_html/printf properly.
includes/Admin/Subscription.php (5)
62-66: Good: consolidated prepared get_rowSafer, clearer DB call.
772-776: Good: direct prepared queryClear and safe retrieval by order id.
846-852: Nonce verification should be optional (duplicate of earlier guidance)Don’t verify/log when nonce is absent; normalize with sanitize_key.
- // Verify nonce for security - if ( ! wp_verify_nonce( isset( $_GET['_wpnonce'] ) ? sanitize_text_field( wp_unslash( $_GET['_wpnonce'] ) ) : '', 'wpuf_subscription_packs' ) ) { - // If nonce verification fails, still allow the function to work but log the issue - if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { - error_log( 'WPUF Subscription: Nonce verification failed for subscription_packs function' ); - } - } + // Optional nonce check (only when provided) + if ( isset( $_GET['_wpnonce'] ) ) { + $nonce = sanitize_key( wp_unslash( $_GET['_wpnonce'] ) ); + if ( ! wp_verify_nonce( $nonce, 'wpuf_subscription_packs' ) && defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( 'WPUF Subscription: Nonce verification failed for subscription_packs function' ); + } + }
923-927: LGTM: safer, escaped messagesGood use of esc_html__/printf around UI strings.
1146-1164: Fix status placeholder type and reduce duplication (same issue as core class)subscribtion_status is a string; use %s and collapse branches.
- if ( $pack_id && $status ) { - $rows = $wpdb->get_results( $wpdb->prepare( - 'SELECT user_id FROM ' . $wpdb->prefix . 'wpuf_subscribers WHERE subscribtion_id = %d AND subscribtion_status = %d', - $pack_id, $status - ) ); - } elseif ( $pack_id ) { - $rows = $wpdb->get_results( $wpdb->prepare( - 'SELECT user_id FROM ' . $wpdb->prefix . 'wpuf_subscribers WHERE subscribtion_id = %d', - $pack_id - ) ); - } elseif ( $status ) { - $rows = $wpdb->get_results( $wpdb->prepare( - 'SELECT user_id FROM ' . $wpdb->prefix . 'wpuf_subscribers WHERE subscribtion_status = %d', - $status - ) ); - } else { - $rows = $wpdb->get_results( 'SELECT user_id FROM ' . $wpdb->prefix . 'wpuf_subscribers' ); - } + $sql = 'SELECT user_id FROM ' . $wpdb->prefix . 'wpuf_subscribers'; + $wheres = []; + $vals = []; + if ( $pack_id ) { + $wheres[] = 'subscribtion_id = %d'; + $vals[] = (int) $pack_id; + } + if ( $status ) { + $wheres[] = 'subscribtion_status = %s'; + $vals[] = $status; + } + if ( $wheres ) { + $sql .= ' WHERE ' . implode( ' AND ', $wheres ); + } + $rows = $vals ? $wpdb->get_results( $wpdb->prepare( $sql, ...$vals ) ) : $wpdb->get_results( $sql );includes/Admin/List_Table_Subscribers.php (1)
218-221: Restore case‑sensitive status filtering (prevents empty result sets)sanitize_key lowercases values (e.g., Completed → completed) and breaks DB matches that are mixed‑case.
- $status = ! empty( $_REQUEST['status'] ) ? sanitize_key( wp_unslash( $_REQUEST['status'] ) ) : ''; + $status = ! empty( $_REQUEST['status'] ) ? sanitize_text_field( wp_unslash( $_REQUEST['status'] ) ) : '';As flagged previously, keep case-preserving sanitization so it matches canonical DB values. Based on past review comments
| // Build the query with proper placeholders | ||
| $base_sql = 'SELECT * FROM ' . $wpdb->prefix . 'wpuf_subscribers'; | ||
| $prepare_values = []; | ||
|
|
||
| // Add conditional WHERE clauses if params exist | ||
| if ( ! empty( $_REQUEST['post_ID'] ) ) { | ||
| $where_clauses[] = 'subscribtion_id = %d'; | ||
| $prepare_values[] = intval( sanitize_text_field( wp_unslash( $_REQUEST['post_ID'] ) ) ); | ||
| } | ||
|
|
||
| if ( ! empty( $_REQUEST['status'] ) ) { | ||
| $where_clauses[] = 'subscribtion_status = %d'; | ||
| $prepare_values[] = sanitize_key( wp_unslash( $_REQUEST['status'] ) ); | ||
| } | ||
|
|
||
| // Combine WHERE clauses if any exist | ||
| if ( ! empty( $where_clauses ) ) { | ||
| $sql .= ' WHERE ' . implode( ' AND ', $where_clauses ); | ||
| $post_id = ! empty( $_REQUEST['post_ID'] ) ? intval( sanitize_text_field( wp_unslash( $_REQUEST['post_ID'] ) ) ) : ''; | ||
| $status = ! empty( $_REQUEST['status'] ) ? sanitize_key( wp_unslash( $_REQUEST['status'] ) ) : ''; | ||
|
|
||
| if ( $post_id && $status ) { | ||
| $sql = $base_sql . ' WHERE subscribtion_id = %d AND subscribtion_status = %s'; | ||
| $prepare_values = [ $post_id, $status ]; | ||
| } elseif ( $post_id ) { | ||
| $sql = $base_sql . ' WHERE subscribtion_id = %d'; | ||
| $prepare_values = [ $post_id ]; | ||
| } elseif ( $status ) { | ||
| $sql = $base_sql . ' WHERE subscribtion_status = %s'; | ||
| $prepare_values = [ $status ]; | ||
| } else { | ||
| $sql = $base_sql; | ||
| } | ||
|
|
||
| // Prepare and execute the query safely | ||
| $prepared_query = $wpdb->prepare( $sql, $prepare_values ); | ||
| $this->items = $wpdb->get_results( $prepared_query ); | ||
| if ( ! empty( $prepare_values ) ) { | ||
| $this->items = $wpdb->get_results( $wpdb->prepare( $sql, ...$prepare_values ) ); | ||
| } else { | ||
| // When no prepare values, the SQL is safe as it's built from constants | ||
| $this->items = $wpdb->get_results( $sql ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Apply pagination at SQL level and compute total count
Current query ignores LIMIT/OFFSET; it loads all rows and breaks pagination/perf.
// Build the query with proper placeholders
$base_sql = 'SELECT * FROM ' . $wpdb->prefix . 'wpuf_subscribers';
$prepare_values = [];
@@
- // Prepare and execute the query safely
- if ( ! empty( $prepare_values ) ) {
- $this->items = $wpdb->get_results( $wpdb->prepare( $sql, ...$prepare_values ) );
- } else {
- // When no prepare values, the SQL is safe as it's built from constants
- $this->items = $wpdb->get_results( $sql );
- }
+ // Sorting (whitelist)
+ $order_by = 'id';
+ $order = 'DESC';
+ if ( ! empty( $args['orderby'] ) ) {
+ $allowed_cols = [ 'id', 'user_id', 'subscribtion_id', 'subscribtion_status', 'starts_from', 'expire' ];
+ $candidate = sanitize_key( $args['orderby'] );
+ if ( in_array( $candidate, $allowed_cols, true ) ) {
+ $order_by = $candidate;
+ }
+ }
+ if ( ! empty( $args['order'] ) && in_array( strtoupper( $args['order'] ), [ 'ASC', 'DESC' ], true ) ) {
+ $order = strtoupper( $args['order'] );
+ }
+
+ // Apply pagination
+ $sql .= " ORDER BY {$order_by} {$order} LIMIT %d OFFSET %d";
+ $prepare_values[] = (int) $per_page;
+ $prepare_values[] = (int) $offset;
+
+ // Execute
+ $this->items = $wpdb->get_results( $wpdb->prepare( $sql, ...$prepare_values ) );Also update total count (outside the shown range) so pagination shows correct totals:
// Derive COUNT(*) with the same WHERE
$count_sql = str_replace( 'SELECT *', 'SELECT COUNT(*)', $base_sql );
if ( isset( $sql ) && false !== strpos( $sql, ' WHERE ' ) ) {
$count_sql .= substr( $sql, strpos( $sql, ' WHERE ' ), strpos( $sql, ' ORDER BY ' ) - strpos( $sql, ' WHERE ' ) );
}
$total_items = (int) ( ! empty( $prepare_values ) ? $wpdb->get_var( $wpdb->prepare( $count_sql, ...array_slice( $prepare_values, 0, count( $prepare_values ) - 2 ) ) ) : $wpdb->get_var( $count_sql ) );
$this->set_pagination_args( [
'total_items' => $total_items,
'per_page' => $per_page,
] );Also applies to: 242-246
@Rubaiyat-E-Mohammad vai , remaining errors will auto solve after build |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Lib/recaptchalib_noCaptcha.php (1)
139-156: Handle failed HTTP calls before decoding JSON.
_submitHttpGet()now returnsfalseon transport errors / non-200s, butverifyResponse()callsjson_decode()on that boolean and thentrim()the missingsuccesskey. On PHP 8 this throws a fatal TypeError, so captcha failures crash the request instead of failing gracefully. Bail out when$getResponse === false(and when decoding fails) before touching the response array.$getResponse = $this->_submitHttpGet( self::$_siteVerifyUrl, array ( 'secret' => $this->_secret, 'remoteip' => $remoteIp, 'v' => self::$_version, 'response' => $response ) ); - $answers = json_decode($getResponse, true); + if ( false === $getResponse ) { + $recaptchaResponse->success = false; + $recaptchaResponse->errorCodes = 'recaptcha-no-response'; + return $recaptchaResponse; + } + + $answers = json_decode( $getResponse, true ); + + if ( ! is_array( $answers ) ) { + $recaptchaResponse->success = false; + $recaptchaResponse->errorCodes = 'recaptcha-invalid-json'; + return $recaptchaResponse; + }includes/Admin/Subscription.php (1)
203-205: Nonce verification is broken inafter_registration()
wp_verify_nonce( '_wpnonce' )is verifying the literal string'_wpnonce', so the check always fails and the guard never fires. This effectively disables the nonce protection for this path, allowing the registration-side pack assignment logic to run even when the nonce is missing or forged. Please read the posted nonce, unslash/sanitize it, and compare against the correct action string, bailing out when the nonce is absent or invalid.- if ( isset( $_POST['_wpnonce'] ) && wp_verify_nonce( '_wpnonce' ) ) { - return; - } + $nonce = isset( $_POST['_wpnonce'] ) ? sanitize_text_field( wp_unslash( $_POST['_wpnonce'] ) ) : ''; + + if ( ! $nonce || ! wp_verify_nonce( $nonce, 'register' ) ) { // adjust the action to match the form that generated the nonce + return; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Lib/recaptchalib.php(1 hunks)Lib/recaptchalib_noCaptcha.php(2 hunks)admin/form-builder/views/form-builder-v4.1.php(3 hunks)admin/form-builder/views/post-form-settings.php(18 hunks)assets/js-templates/form-components.php(8 hunks)includes/Admin/List_Table_Subscribers.php(5 hunks)includes/Admin/Subscription.php(5 hunks)includes/Admin/template-parts/modal-v4.1.php(2 hunks)includes/Frontend_Render_Form.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- admin/form-builder/views/form-builder-v4.1.php
- includes/Frontend_Render_Form.php
- includes/Admin/List_Table_Subscribers.php
- assets/js-templates/form-components.php
- includes/Admin/template-parts/modal-v4.1.php
🧰 Additional context used
🧬 Code graph analysis (3)
admin/form-builder/views/post-form-settings.php (3)
class/render-form.php (1)
label(880-892)includes/Render_Form.php (1)
label(742-754)wpuf-functions.php (2)
wpuf_is_pro_active(4990-4992)wpuf_get_pro_icon(4979-4981)
Lib/recaptchalib_noCaptcha.php (2)
class/render-form.php (1)
url(1272-1290)includes/Render_Form.php (1)
url(1134-1152)
includes/Admin/Subscription.php (1)
includes/User_Subscription.php (1)
current_pack(53-73)
| id="<?php echo esc_attr( $inner_field_key ); ?>" | ||
| name="<?php echo esc_attr( $inner_field_key ); ?>" | ||
| data-value="<?php echo esc_attr( $value_str ); ?>" |
There was a problem hiding this comment.
Inline select won’t persist without the wpuf_settings[] name prefix
Here we now emit name="<?php echo esc_attr( $inner_field_key ); ?>". That drops the wpuf_settings[...] wrapper, so the select’s value never lands inside $_POST['wpuf_settings']. On save the option falls back to the default because nothing is written back into $form_settings. The adjacent inline text/date inputs still use the prefixed name, so this regression only hits the select variant. Please restore the prefixed name while keeping the escaping.
- name="<?php echo esc_attr( $inner_field_key ); ?>"
+ name="wpuf_settings[<?php echo esc_attr( $inner_field_key ); ?>]"🤖 Prompt for AI Agents
In admin/form-builder/views/post-form-settings.php around lines 655-657, the
select's name has been changed to the raw inner field key which removes the
required wpuf_settings[...] wrapper so the value isn't posted back; restore the
prefixed name to wpuf_settings[...] while keeping esc_attr escaping (i.e., wrap
the inner field key inside the wpuf_settings[...] name attribute) so the select
value lands in $_POST['wpuf_settings'] on save.
| $container_id = 'wpuf-recaptcha-legacy-' . uniqid(); | ||
| $return_var = '<div id="' . esc_attr($container_id) . '" class="wpuf-recaptcha-legacy-container"></div> | ||
| <script> | ||
| (function() { | ||
| var container = document.getElementById("' . esc_js($container_id) . '"); | ||
| if (container) { | ||
| var originalWrite = document.write; | ||
| document.write = function(content) { | ||
| container.innerHTML = content; | ||
| }; | ||
| // Reset document.write after a short delay to avoid interfering with other scripts | ||
| setTimeout(function() { | ||
| document.write = originalWrite; | ||
| }, 100); | ||
| } | ||
| })(); | ||
| </script>'; |
There was a problem hiding this comment.
Legacy challenge still renders in the footer and corrupts document.write.
The temporary document.write override drops after 100 ms, but the enqueued challenge.js often executes later, so it reverts to the original document.write and still writes the widget in the footer—regressing to the exact bug we’re trying to fix. With multiple legacy widgets, the chained overrides also leave document.write pointing at an earlier container, breaking unrelated scripts. Please restore inline loading (or keep the override active until the script fires a load event and then safely unwind it) so the v1 widget renders in place without clobbering document.write.
- wp_enqueue_script( 'wpuf-recaptcha-legacy', $server . '/challenge?k=' . $pubkey . $errorpart, array(), null, true );
- $container_id = 'wpuf-recaptcha-legacy-' . uniqid();
- $return_var = '<div id="' . esc_attr($container_id) . '" class="wpuf-recaptcha-legacy-container"></div>
-<script>
-(function() {
- var container = document.getElementById("' . esc_js($container_id) . '");
- if (container) {
- var originalWrite = document.write;
- document.write = function(content) {
- container.innerHTML = content;
- };
- // Reset document.write after a short delay to avoid interfering with other scripts
- setTimeout(function() {
- document.write = originalWrite;
- }, 100);
- }
-})();
-</script>';
+ $return_var = '<script src="' . esc_url( $server . '/challenge?k=' . rawurlencode( $pubkey ) . $errorpart ) . '"></script>';🤖 Prompt for AI Agents
In Lib/recaptchalib.php around lines 129 to 145, the temporary document.write
override currently restores after a fixed 100ms which often reverts before the
enqueued challenge.js runs and can leave document.write pointing at the wrong
container when multiple widgets exist; replace the fixed timeout approach by
creating and injecting the challenge script element inline, keep document.write
overridden until that specific script fires its load (or readyState) or an
explicit timeout fallback, then restore the original document.write in the
script's onload/onerror/onreadystatechange handlers so the widget is written
into its intended container and unrelated scripts are not broken; ensure each
widget uses its own uniquely-scoped override closure that captures and restores
the original document.write to avoid cross-widget interference.
Issues are still same @arifulhoque7 vai |
Rubaiyat-E-Mohammad
added
QA Approved
2 participants
Close issue
Fix PCP compatibility issues in WPUF 4.1.12 (free).
Summary by CodeRabbit
Bug Fixes
Chores