Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Experiment] Additional field extensible sanitisation and validation handling #44463

Merged
merged 21 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: update

This is behind a feature flag.
18 changes: 13 additions & 5 deletions plugins/woocommerce/includes/class-wc-form-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -353,12 +353,20 @@ public static function save_account_details() {
$customer->save();
}

wc_add_notice( __( 'Account details changed successfully.', 'woocommerce' ) );

/**
* Hook: woocommerce_save_account_details.
*
* @since 3.6.0
* @param int $user_id User ID being saved.
*/
do_action( 'woocommerce_save_account_details', $user->ID );

wp_safe_redirect( wc_get_endpoint_url( 'edit-account', '', wc_get_page_permalink( 'myaccount' ) ) );
exit;
// Notices are checked here so that if something created a notice during the save hooks above, the redirect will not happen.
if ( 0 === wc_notice_count( 'error' ) ) {
wc_add_notice( __( 'Account details changed successfully.', 'woocommerce' ) );
wp_safe_redirect( wc_get_endpoint_url( 'edit-account', '', wc_get_page_permalink( 'myaccount' ) ) );
exit;
}
}
}

Expand Down Expand Up @@ -901,7 +909,7 @@ private static function add_to_cart_handler_variable( $product_id ) {
$quantity = empty( $_REQUEST['quantity'] ) ? 1 : wc_stock_amount( wp_unslash( $_REQUEST['quantity'] ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended
$variations = array();

$product = wc_get_product( $product_id );
$product = wc_get_product( $product_id );

foreach ( $_REQUEST as $key => $value ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended
if ( 'attribute_' !== substr( $key, 0, 10 ) ) {
Expand Down
197 changes: 127 additions & 70 deletions plugins/woocommerce/src/Blocks/Domain/Services/CheckoutFields.php
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
<?php

Check notice on line 1 in plugins/woocommerce/src/Blocks/Domain/Services/CheckoutFields.php

View workflow job for this annotation

GitHub Actions / Analyze Branch Changes

new filter found - woocommerce_blocks_sanitize_additional_field

\'woocommerce_blocks_sanitize_additional_field\' introduced in 8.7.0

Check notice on line 1 in plugins/woocommerce/src/Blocks/Domain/Services/CheckoutFields.php

View workflow job for this annotation

GitHub Actions / Analyze Branch Changes

updated filter found - woocommerce_blocks_validate_additional_field_

\'woocommerce_blocks_validate_additional_field_\' introduced in 8.7.0

Check notice on line 1 in plugins/woocommerce/src/Blocks/Domain/Services/CheckoutFields.php

View workflow job for this annotation

GitHub Actions / Analyze Branch Changes

new action found - woocommerce_blocks_validate_

\'woocommerce_blocks_validate_\' introduced in 8.7.0

namespace Automattic\WooCommerce\Blocks\Domain\Services;

use Automattic\WooCommerce\Blocks\Assets\AssetDataRegistry;
use WC_Customer;
use WC_Order;
use WP_Error;

/**
* Service class managing checkout fields and its related extensibility points.
Expand Down Expand Up @@ -253,7 +254,7 @@
*
* @param array $options The field options.
*
* @return \WP_Error|void True if the field was registered, a WP_Error otherwise.
* @return WP_Error|void True if the field was registered, a WP_Error otherwise.
*/
public function register_checkout_field( $options ) {
// Check the options and show warnings if they're not supplied. Return early if an error that would prevent registration is encountered.
Expand Down Expand Up @@ -537,73 +538,82 @@
}

/**
* Validate an additional field against any custom validation rules. The result should be a WP_Error or true.
* Sanitize an additional field against any custom sanitization rules.
*
* @param string $key The key of the field.
* @param mixed $field_value The value of the field.
* @param \WP_REST_Request $request The current API Request.
* @param string|null $address_type The type of address (billing, shipping, or null if the field is a contact/additional field).
* @since 8.7.0

* @param string $field_key The key of the field.
* @param mixed $field_value The value of the field.
* @return mixed
*/
public function sanitize_field( $field_key, $field_value ) {
try {
/**
* Allow custom sanitization of an additional field.
*
* @param mixed $field_value The value of the field being sanitized.
* @param string $field_key Key of the field being sanitized.
*
* @since 8.7.0
*/
return apply_filters( 'woocommerce_blocks_sanitize_additional_field', $field_value, $field_key );

} catch ( \Throwable $e ) {
// One of the filters errored so skip it. This allows the checkout process to continue.
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
trigger_error(
sprintf(
'The filter %s encountered an error. The field %s will not have any custom sanitization applied to it. %s',
'woocommerce_blocks_sanitize_additional_field',
esc_html( $field_key ),
esc_html( $e->getMessage() )
),
E_USER_WARNING
);
}

return $field_value;
}

/**
* Validate an additional field against any custom validation rules.
*
* @since 8.6.0
*
* @param string $field_key The key of the field.
* @param mixed $field_value The value of the field.
* @return WP_Error
*/
public function validate_field( $key, $field_value, $request, $address_type = null ) {

$error = new \WP_Error();
public function validate_field( $field_key, $field_value ) {
$errors = new WP_Error();
try {
/**
* Filter the result of validating an additional field.
* Pass an error object to allow validation of an additional field.
*
* @param \WP_Error $error A WP_Error that extensions may add errors to.
* @param mixed $field_value The value of the field.
* @param \WP_REST_Request $request The current API Request.
* @param string|null $address_type The type of address (billing, shipping, or null if the field is a contact/additional field).
* @param WP_Error $errors A WP_Error object that extensions may add errors to.
* @param mixed $field_value The value of the field being validated.
* @param string $field_key Key of the field being sanitized.
*
* @since 8.6.0
* @since 8.7.0
*/
$filtered_result = apply_filters( 'woocommerce_blocks_validate_additional_field_' . $key, $error, $field_value, $request, $address_type );

if ( $error !== $filtered_result ) {

// Different WP_Error was returned. This would remove errors from other filters. Skip filtering and allow the order to place without validating this field.
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
trigger_error(
sprintf(
'The filter %s encountered an error. One of the filters returned a new WP_Error. Filters should use the same WP_Error passed to the filter and use the WP_Error->add function to add errors. The field will not have any custom validation applied to it.',
'woocommerce_blocks_validate_additional_field_' . esc_html( $key ),
),
E_USER_WARNING
);
}
do_action( 'woocommerce_blocks_validate_additional_field', $errors, $field_value, $field_key );

} catch ( \Throwable $e ) {

// One of the filters errored so skip them and validate the field. This allows the checkout process to continue.
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
trigger_error(
sprintf(
'The filter %s encountered an error. The field will not have any custom validation applied to it. %s',
'woocommerce_blocks_validate_additional_field_' . esc_html( $key ),
'The action %s encountered an error. The field %s may not have any custom validation applied to it. %s',
'woocommerce_blocks_validate_additional_field',
esc_html( $field_key ),
esc_html( $e->getMessage() )
),
E_USER_WARNING
);

return new \WP_Error();
}

if ( is_wp_error( $filtered_result ) ) {
return $filtered_result;
}

// If the filters didn't return a valid value, ignore them and return an empty WP_Error. This allows the checkout process to continue.
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
trigger_error(
sprintf(
'The filter %s did not return a valid value. The field will not have any custom validation applied to it.',
'woocommerce_blocks_validate_additional_field_' . esc_html( $key )
),
E_USER_WARNING
);
return new \WP_Error();
return $errors;
}

/**
Expand Down Expand Up @@ -648,24 +658,6 @@
return $this->fields_locations['additional'];
}

/**
* Returns an array of fields definitions only meant for order.
*
* @return array An array of fields definitions.
*/
public function get_order_only_fields() {
// For now, all contact fields are order only fields, along with additional fields.
$order_fields_keys = array_merge( $this->get_contact_fields_keys(), $this->get_additional_fields_keys() );

return array_filter(
$this->get_additional_fields(),
function( $key ) use ( $order_fields_keys ) {
return in_array( $key, $order_fields_keys, true );
},
ARRAY_FILTER_USE_KEY
);
}

/**
* Returns an array of fields for a given group.
*
Expand All @@ -688,17 +680,60 @@
}

/**
* Validates a field value for a given group.
* Validates a set of fields for a given location against custom validation rules.
*
* @param array $fields Array of key value pairs of field values to validate.
* @param string $location The location being validated (address|contact|additional).
* @param string $group The group to get the field value for (shipping|billing|'') in which '' refers to the additional group.
* @return WP_Error
*/
public function validate_fields_for_location( $fields, $location, $group = '' ) {
$errors = new WP_Error();

try {
/**
* Pass an error object to allow validation of an additional field.
*
* @param WP_Error $errors A WP_Error object that extensions may add errors to.
* @param mixed $fields List of fields (key value pairs) in this location.
* @param string $group The group of this location (shipping|billing|'').
*
* @since 8.7.0
*/
do_action( 'woocommerce_blocks_validate_' . $location . '_fields', $errors, $fields, $group );
Copy link
Contributor

Choose a reason for hiding this comment

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

When the additional area is validated, the hook is:
woocommerce_blocks_validate_additional_fields which may be easily confused with woocommerce_blocks_validate_additional_field. Do you think this is an issue? Should we consider renaming one or the other? Maybe adding location to the location hook would work? woocommerce_blocks_validate_location_additional_fields

Copy link
Member Author

Choose a reason for hiding this comment

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

Added in b424149


} catch ( \Throwable $e ) {

// One of the filters errored so skip them. This allows the checkout process to continue.
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
trigger_error(
sprintf(
'The action %s encountered an error. The field location %s may not have any custom validation applied to it. %s',
esc_html( 'woocommerce_blocks_validate_' . $location . '_fields' ),
esc_html( $location ),
esc_html( $e->getMessage() )
),
E_USER_WARNING
);
}

return $errors;
}

/**
* Validates a field to check it belongs to the given location and is valid according to its registration.
*
* This does not apply any custom validation rules on the value.
*
* @param string $key The field key.
* @param mixed $value The field value.
* @param string $location The location to validate the field for (address|contact|additional).
*
* @return true|\WP_Error True if the field is valid, a WP_Error otherwise.
* @return true|WP_Error True if the field is valid, a WP_Error otherwise.
*/
public function validate_field_for_location( $key, $value, $location ) {
if ( ! $this->is_field( $key ) ) {
return new \WP_Error(
return new WP_Error(
'woocommerce_blocks_checkout_field_invalid',
\sprintf(
// translators: % is field key.
Expand All @@ -709,7 +744,7 @@
}

if ( ! in_array( $key, $this->fields_locations[ $location ], true ) ) {
return new \WP_Error(
return new WP_Error(
'woocommerce_blocks_checkout_field_invalid_location',
\sprintf(
// translators: %1$s is field key, %2$s location.
Expand All @@ -722,7 +757,7 @@

$field = $this->additional_fields[ $key ];
if ( ! empty( $field['required'] ) && empty( $value ) ) {
return new \WP_Error(
return new WP_Error(
'woocommerce_blocks_checkout_field_required',
\sprintf(
// translators: %s is field key.
Expand Down Expand Up @@ -986,6 +1021,28 @@
);
}

/**
* From a set of fields, returns only the ones for a given location.
*
* @param array $fields The fields to filter.
* @param string $location The location to validate the field for (address|contact|additional).
* @return array The filtered fields.
*/
public function filter_fields_for_location( $fields, $location ) {
return array_filter(
$fields,
function( $key ) use ( $location ) {
if ( 0 === strpos( $key, '/billing/' ) ) {
$key = str_replace( '/billing/', '', $key );
} elseif ( 0 === strpos( $key, '/shipping/' ) ) {
$key = str_replace( '/shipping/', '', $key );
}
return $this->is_field( $key ) && $this->get_field_location( $key ) === $location;
},
ARRAY_FILTER_USE_KEY
);
}

/**
* Filter fields for order confirmation.
*
Expand Down