woocommerce-blocks icon indicating copy to clipboard operation
woocommerce-blocks copied to clipboard

Fix error with usage of Cart::deep_sort_with_accents

Open nerrad opened this issue 2 years ago • 5 comments

A fatal error was reported in this forum thread:

Message Uncaught TypeError: html_entity_decode(): Argument #1 ($string) must be of type string, array given in /home/516558.cloudwaysapps.com/yucfgwgahw/public_html/wp-content/plugins/woo-gutenberg-products-block/src/BlockTypes/Cart.php:226

After some debugging isolated to potential uses of Automattic\WooCommerce\Blocks\BlockTypes\Cart::deep_sort_with_accents (which is where the fatal error is triggered), I discovered that the current implementation expects the incoming $array argument to either have the first element be an array, or all elements to be strings. In particular the logic in these lines here will only execute when the first element is an array on the first call to the recursive function:

https://github.com/woocommerce/woocommerce-blocks/blob/c5bdcfffc49b38c6e110d44c775290e9aaea300d/src/BlockTypes/Cart.php#L222-L224

Thus the error is happening on this user's site because an unexpected data shape is coming into this function on this user's site.

This code has been in place for a while, however, given the critical path of this code (for the Cart block), I decided to get a PR up for this.

This PR fixes the issue by ensuring that when the element is iterated over, we recursively call deep_sort_with_accents only on elements that are arrays.

I've also added a basic unit test to demonstrate the bug is fixed. I didn't cover all use-cases for this method, it might be a good idea to do that in a separate PR.

Accessibility

Does not apply

Other Checks

  • [ ] This PR adds/removes a feature flag & I've updated this doc.
  • [ ] This PR adds/removes an experimental interfaces and I've updated this doc.
  • [ ] I tagged two reviewers because this PR makes queries to the database or I think it might have some security impact.

Screenshots

Does not apply

Testing

Automated Tests

  • [x] Changes in this PR are covered by Automated Tests.
    • [x] Unit tests
    • [ ] E2E tests

User Facing Testing

  1. Configure multiple shipping countries.
  2. Setup the cart block for use on the site.
  3. Verify there are no errors when displaying the cart in the editor or on the frontend (especially around any shipping options).
  • [ ] Do not include in the Testing Notes

WooCommerce Visibility

  • [x] WooCommerce Core
  • [x] Feature plugin
  • [ ] Experimental

Performance Impact

There should be no performance impact.

Changelog

Fixes a fatal error with Cart Block usage in specific site configurations with multiple shipping countries.

nerrad avatar Aug 12 '22 16:08 nerrad

The release ZIP for this PR is accessible via:

https://wcblocks.wpcomstaging.com/wp-content/uploads/woocommerce-gutenberg-products-block-6896.zip

github-actions[bot] avatar Aug 12 '22 16:08 github-actions[bot]

Size Change: 0 B

Total Size: 962 kB

ℹ️ View Unchanged
Filename Size
build/active-filters-frontend.js 7.72 kB
build/active-filters-wrapper-frontend.js 6.01 kB
build/active-filters.js 7.44 kB
build/all-products-frontend.js 26.5 kB
build/all-products.js 33.6 kB
build/all-reviews.js 7.79 kB
build/attribute-filter-frontend.js 22.5 kB
build/attribute-filter-wrapper-frontend.js 7.04 kB
build/attribute-filter.js 12.4 kB
build/blocks-checkout.js 17.5 kB
build/cart-blocks/cart-accepted-payment-methods-frontend.js 1.38 kB
build/cart-blocks/cart-cross-sells-frontend.js 253 B
build/cart-blocks/cart-cross-sells-products--product-add-to-cart-frontend.js 5.63 kB
build/cart-blocks/cart-cross-sells-products-frontend.js 4.66 kB
build/cart-blocks/cart-express-payment--checkout-blocks/express-payment-frontend.js 5.23 kB
build/cart-blocks/cart-express-payment-frontend.js 786 B
build/cart-blocks/cart-items-frontend.js 298 B
build/cart-blocks/cart-line-items--mini-cart-contents-block/products-table-frontend.js 5.3 kB
build/cart-blocks/cart-line-items-frontend.js 1.07 kB
build/cart-blocks/cart-order-summary-frontend.js 1.11 kB
build/cart-blocks/cart-totals-frontend.js 320 B
build/cart-blocks/empty-cart-frontend.js 346 B
build/cart-blocks/filled-cart-frontend.js 785 B
build/cart-blocks/order-summary-coupon-form-frontend.js 2.73 kB
build/cart-blocks/order-summary-discount-frontend.js 2.16 kB
build/cart-blocks/order-summary-fee-frontend.js 273 B
build/cart-blocks/order-summary-heading-frontend.js 454 B
build/cart-blocks/order-summary-shipping--checkout-blocks/order-summary-shipping-frontend.js 6.61 kB
build/cart-blocks/order-summary-shipping-frontend.js 430 B
build/cart-blocks/order-summary-subtotal-frontend.js 274 B
build/cart-blocks/order-summary-taxes-frontend.js 435 B
build/cart-blocks/proceed-to-checkout-frontend.js 1.19 kB
build/cart-frontend.js 50 kB
build/cart.js 46.2 kB
build/checkout-blocks/actions-frontend.js 1.79 kB
build/checkout-blocks/billing-address--checkout-blocks/shipping-address-frontend.js 4.98 kB
build/checkout-blocks/billing-address-frontend.js 948 B
build/checkout-blocks/contact-information-frontend.js 3.03 kB
build/checkout-blocks/express-payment-frontend.js 1.16 kB
build/checkout-blocks/fields-frontend.js 344 B
build/checkout-blocks/order-note-frontend.js 1.14 kB
build/checkout-blocks/order-summary-cart-items-frontend.js 3.67 kB
build/checkout-blocks/order-summary-coupon-form-frontend.js 2.88 kB
build/checkout-blocks/order-summary-discount-frontend.js 2.28 kB
build/checkout-blocks/order-summary-fee-frontend.js 276 B
build/checkout-blocks/order-summary-frontend.js 1.11 kB
build/checkout-blocks/order-summary-shipping-frontend.js 603 B
build/checkout-blocks/order-summary-subtotal-frontend.js 273 B
build/checkout-blocks/order-summary-taxes-frontend.js 435 B
build/checkout-blocks/payment-frontend.js 8 kB
build/checkout-blocks/shipping-address-frontend.js 1.06 kB
build/checkout-blocks/shipping-methods-frontend.js 4.89 kB
build/checkout-blocks/terms-frontend.js 1.64 kB
build/checkout-blocks/totals-frontend.js 323 B
build/checkout-frontend.js 52.1 kB
build/checkout.js 40 kB
build/featured-category.js 13.2 kB
build/featured-product.js 13.4 kB
build/filter-wrapper-frontend.js 10.6 kB
build/filter-wrapper.js 1.86 kB
build/general-style-rtl.css 1.29 kB
build/general-style.css 1.29 kB
build/handpicked-products.js 7.27 kB
build/legacy-template.js 2.84 kB
build/mini-cart-component-frontend.js 16.8 kB
build/mini-cart-contents-block/empty-cart-frontend.js 367 B
build/mini-cart-contents-block/filled-cart-frontend.js 230 B
build/mini-cart-contents-block/footer-frontend.js 2.98 kB
build/mini-cart-contents-block/items-frontend.js 236 B
build/mini-cart-contents-block/products-table-frontend.js 590 B
build/mini-cart-contents-block/shopping-button-frontend.js 287 B
build/mini-cart-contents-block/title-frontend.js 367 B
build/mini-cart-contents.js 16.8 kB
build/mini-cart-frontend.js 1.71 kB
build/mini-cart.js 4.57 kB
build/price-filter-frontend.js 13.6 kB
build/price-filter-wrapper-frontend.js 6.95 kB
build/price-filter.js 8.47 kB
build/price-format.js 1.19 kB
build/product-add-to-cart--product-button--product-category-list--product-image--product-price--product-r--a0326d00.js 227 B
build/product-add-to-cart--product-button--product-image--product-title.js 2.66 kB
build/product-add-to-cart-frontend.js 1.25 kB
build/product-add-to-cart.js 6.47 kB
build/product-best-sellers.js 7.62 kB
build/product-button--product-category-list--product-image--product-price--product-rating--product-sale-b--e17c7c01.js 433 B
build/product-button--product-image--product-rating--product-sale-badge--product-title.js 302 B
build/product-button-frontend.js 1.89 kB
build/product-button.js 1.58 kB
build/product-categories.js 2.36 kB
build/product-category-list-frontend.js 882 B
build/product-category-list.js 502 B
build/product-category.js 8.61 kB
build/product-image-frontend.js 1.92 kB
build/product-image.js 1.62 kB
build/product-new.js 7.62 kB
build/product-on-sale.js 7.95 kB
build/product-price-frontend.js 1.92 kB
build/product-price.js 1.53 kB
build/product-query.js 648 B
build/product-rating-frontend.js 1.18 kB
build/product-rating.js 773 B
build/product-sale-badge-frontend.js 1.15 kB
build/product-sale-badge.js 819 B
build/product-search.js 2.61 kB
build/product-sku-frontend.js 379 B
build/product-sku.js 380 B
build/product-stock-indicator-frontend.js 997 B
build/product-stock-indicator.js 624 B
build/product-summary-frontend.js 1.29 kB
build/product-summary.js 920 B
build/product-tag-list-frontend.js 878 B
build/product-tag-list.js 496 B
build/product-tag.js 8 kB
build/product-title-frontend.js 1.34 kB
build/product-title.js 938 B
build/product-top-rated.js 7.86 kB
build/products-by-attribute.js 8.53 kB
build/rating-filter-frontend.js 7.39 kB
build/rating-filter.js 7.87 kB
build/reviews-by-category.js 11.2 kB
build/reviews-by-product.js 12.3 kB
build/reviews-frontend.js 7 kB
build/single-product-frontend.js 29.2 kB
build/single-product.js 10 kB
build/stock-filter-frontend.js 7.72 kB
build/stock-filter-wrapper-frontend.js 5.99 kB
build/stock-filter.js 6.65 kB
build/vendors--attribute-filter-wrapper--mini-cart-contents-block/footer-frontend.js 6.86 kB
build/vendors--attribute-filter-wrapper-frontend.js 8.21 kB
build/vendors--cart-blocks/cart-cross-sells-products--cart-blocks/cart-line-items--cart-blocks/cart-order--671ca56f-frontend.js 5.26 kB
build/vendors--cart-blocks/cart-cross-sells-products--cart-blocks/order-summary-shipping--checkout-blocks--18f9376a-frontend.js 19.1 kB
build/vendors--cart-blocks/cart-cross-sells-products--product-add-to-cart-frontend.js 7.53 kB
build/vendors--cart-blocks/cart-line-items--checkout-blocks/order-summary-cart-items--mini-cart-contents---233ab542-frontend.js 3.14 kB
build/vendors--cart-blocks/order-summary-shipping--checkout-blocks/billing-address--checkout-blocks/order--5b8feb0b-frontend.js 4.85 kB
build/wc-blocks-data.js 15.9 kB
build/wc-blocks-editor-style-rtl.css 5.24 kB
build/wc-blocks-editor-style.css 5.24 kB
build/wc-blocks-google-analytics.js 1.56 kB
build/wc-blocks-middleware.js 932 B
build/wc-blocks-registry.js 2.92 kB
build/wc-blocks-shared-context.js 1.52 kB
build/wc-blocks-shared-hocs.js 1.72 kB
build/wc-blocks-style-rtl.css 24.2 kB
build/wc-blocks-style.css 24.1 kB
build/wc-blocks-vendors-style-rtl.css 1.95 kB
build/wc-blocks-vendors-style.css 1.95 kB
build/wc-blocks-vendors.js 62.4 kB
build/wc-blocks.js 2.62 kB
build/wc-payment-method-bacs.js 816 B
build/wc-payment-method-cheque.js 811 B
build/wc-payment-method-cod.js 909 B
build/wc-payment-method-paypal.js 837 B
build/wc-settings.js 2.6 kB

compressed-size-action

github-actions[bot] avatar Aug 12 '22 16:08 github-actions[bot]

@senadir offered to take over this PR from me.

Nadir I was unable to get PHPunit tests running locally in the time I had devoted to this so the PR still has some bugs with the tests that I would have hopefully caught before pushing. The latest fails seem to be related to an incorrect signature for the mock class, once that's fixed the tests should hopefully work.

nerrad avatar Aug 15 '22 11:08 nerrad

This PR has been marked as stale because it has not seen any activity within the past 60 days. Our team uses this tool to help surface pull requests that have slipped through review.

If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label - otherwise it will automatically be closed after 10 days.

github-actions[bot] avatar Sep 17 '22 09:09 github-actions[bot]

Hey @senadir, is this PR still valid or was there some additional investigation that reduced its priority and/or validity? Just checking, because if it's something that does not need to be addressed anymore maybe it could be closed.

nerrad avatar Sep 21 '22 12:09 nerrad

Hey, yes this is still valid and I discussed it again with Raluca yesterday, I will give it a go today or Monday and merge it.

senadir avatar Sep 23 '22 08:09 senadir

This PR has been marked as stale because it has not seen any activity within the past 60 days. Our team uses this tool to help surface pull requests that have slipped through review.

If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label - otherwise it will automatically be closed after 10 days.

github-actions[bot] avatar Sep 30 '22 09:09 github-actions[bot]