This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Prevent cart from breaking when item_data contains an array #8440
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
opr
added
status: needs review
type: bug
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
|
Size Change: 0 B Total Size: 1.1 MB ℹ️ View Unchanged
|
mikejolley
reviewed
Feb 15, 2023
mikejolley
reviewed
Feb 15, 2023
senadir
reviewed
Feb 15, 2023
senadir
reviewed
Feb 15, 2023
Comment on lines
461
to
468
| $item_data = apply_filters( 'woocommerce_get_item_data', array(), $cart_item ); | ||
| $formatted_item_data = array_map( [ $this, 'format_item_data_element' ], $item_data ); | ||
|
|
||
| // Remove empty arrays from the data. | ||
| $valid_item_data = array_filter( | ||
| $formatted_item_data, | ||
| function ( $item ) { | ||
| return ! empty( $item ); |
There was a problem hiding this comment.
Any chance we can just filter once?
There was a problem hiding this comment.
Or just have it all on the format_item_data_element function.
There was a problem hiding this comment.
What if we don't use these functions, and use a plain old foreach?
$clean_item_data = [];
foreach ( $item_data as $data ) {
// We will check each piece of data in the item data element to ensure it is scalar. Extensions could add arrays
// to this, which would cause a fatal in wp_strip_all_tags. If it is not scalar, we will return an empty array,
// which will be filtered out in get_item_data (after this function has run).
foreach ( $data as $data_value ) {
if ( ! is_scalar( $data_value ) ) {
continue 2;
}
}
$clean_item_data[] = $this->format_item_data_element( $data );
}
return $clean_item_data;
ralucaStan
added
the
status: blocker
senadir
approved these changes
Feb 16, 2023
github-actions
bot
added
status: ready to merge
and removed
status: needs review
labels
|
Thanks Nadir, the current behaviour is to show the name but not the value if it is falsy, so we will keep it. |
opr
force-pushed
the
fix/arrays-item-data
branch
from
e57c103 to
7cac874
Compare
tarhi-saad
pushed a commit
that referenced
this pull request
Feb 17, 2023
* Ensure array item data is removed * Remove unused key * Clean up code and add comments * Check for null instead of empty * Use plain foreach to filter and map arrays
tarhi-saad
added a commit
that referenced
this pull request
Feb 17, 2023
* Empty commit for release pull request * Add changelog to readme.txt * Unset default customer state if it doesn't match country (#8460) * Unset default state * add controller for customers * rename validation file * explain fix inline * address feedback * revert back state logic * Update src/StoreApi/Utilities/ValidationUtils.php Co-authored-by: Mike Jolley <mike.jolley@me.com> --------- Co-authored-by: Mike Jolley <mike.jolley@me.com> * Update readme.txt * Add testing notes * Update testing notes * Fix Customer account sidebar link incorrect margin in WP 6.2 (#8437) * Fix Customer account sidebar link incorrect margin in WP 6.2 * Update class name to match the guidelines * Prevent cart from breaking when item_data contains an array (#8440) * Ensure array item data is removed * Remove unused key * Clean up code and add comments * Check for null instead of empty * Use plain foreach to filter and map arrays * Add minimum height to Mini Cart Contents block in the Style Book (#8458) * Update testing notes zip file * Update testing notes * Update testing notes file * Bumping version strings to new version. --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Saad Tarhi <saad.trh@gmail.com> Co-authored-by: Seghir Nadir <nadir.seghir@gmail.com> Co-authored-by: Mike Jolley <mike.jolley@me.com> Co-authored-by: Albert Juhé Lluveras <contact@albertjuhe.com> Co-authored-by: Thomas Roberts <5656702+opr@users.noreply.github.com>
gigitux
added a commit
that referenced
this pull request
Feb 22, 2023
* Empty commit for release pull request * Add changelog to readme.txt * Unset default customer state if it doesn't match country (#8460) * Unset default state * add controller for customers * rename validation file * explain fix inline * address feedback * revert back state logic * Update src/StoreApi/Utilities/ValidationUtils.php Co-authored-by: Mike Jolley <mike.jolley@me.com> --------- Co-authored-by: Mike Jolley <mike.jolley@me.com> * Update readme.txt * Add testing notes * Update testing notes * Fix Customer account sidebar link incorrect margin in WP 6.2 (#8437) * Fix Customer account sidebar link incorrect margin in WP 6.2 * Update class name to match the guidelines * Prevent cart from breaking when item_data contains an array (#8440) * Ensure array item data is removed * Remove unused key * Clean up code and add comments * Check for null instead of empty * Use plain foreach to filter and map arrays * Add minimum height to Mini Cart Contents block in the Style Book (#8458) * Update testing notes zip file * Update testing notes * Update testing notes file * Bumping version strings to new version. * Empty commit for release pull request * Empty commit for release pull request * disable compatibilty layer (#8507) * update changelog and testing instructions * add zip link * Bumping version strings to new version. --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Saad Tarhi <saad.trh@gmail.com> Co-authored-by: Seghir Nadir <nadir.seghir@gmail.com> Co-authored-by: Mike Jolley <mike.jolley@me.com> Co-authored-by: Albert Juhé Lluveras <contact@albertjuhe.com> Co-authored-by: Thomas Roberts <5656702+opr@users.noreply.github.com> Co-authored-by: Luigi Teschio <gigitux@gmail.com>
nielslange
added a commit
that referenced
this pull request
Mar 3, 2023
* Empty commit for release pull request * Add changelog to readme.txt * Unset default customer state if it doesn't match country (#8460) * Unset default state * add controller for customers * rename validation file * explain fix inline * address feedback * revert back state logic * Update src/StoreApi/Utilities/ValidationUtils.php Co-authored-by: Mike Jolley <mike.jolley@me.com> --------- Co-authored-by: Mike Jolley <mike.jolley@me.com> * Update readme.txt * Add testing notes * Update testing notes * Fix Customer account sidebar link incorrect margin in WP 6.2 (#8437) * Fix Customer account sidebar link incorrect margin in WP 6.2 * Update class name to match the guidelines * Prevent cart from breaking when item_data contains an array (#8440) * Ensure array item data is removed * Remove unused key * Clean up code and add comments * Check for null instead of empty * Use plain foreach to filter and map arrays * Add minimum height to Mini Cart Contents block in the Style Book (#8458) * Update testing notes zip file * Update testing notes * Update testing notes file * Bumping version strings to new version. * Empty commit for release pull request * Empty commit for release pull request * disable compatibilty layer (#8507) * update changelog and testing instructions * add zip link * Bumping version strings to new version. * Empty commit for release pull request * Empty commit for release pull request * Add changelog in readme.txt * Add testing notes * Remove change from testing notes This requires AvaTax credentials for testing. So, we'll test for regressions instead * Check for null session before going forward (#8537) * Fix Payment Options settings crash in the editor (#8535) * Ensure express payment buttons are visible next to each other (#8548) * Update ZIP file * Bumping version strings to new version. * Empty commit for release pull request * Empty commit for release pull request --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Saad Tarhi <saad.trh@gmail.com> Co-authored-by: Seghir Nadir <nadir.seghir@gmail.com> Co-authored-by: Mike Jolley <mike.jolley@me.com> Co-authored-by: Albert Juhé Lluveras <contact@albertjuhe.com> Co-authored-by: Thomas Roberts <5656702+opr@users.noreply.github.com> Co-authored-by: Luigi Teschio <gigitux@gmail.com> Co-authored-by: Niels Lange <info@nielslange.de>
gigitux
added a commit
that referenced
this pull request
Mar 17, 2023
* Empty commit for release pull request * Add changelog to readme.txt * Unset default customer state if it doesn't match country (#8460) * Unset default state * add controller for customers * rename validation file * explain fix inline * address feedback * revert back state logic * Update src/StoreApi/Utilities/ValidationUtils.php Co-authored-by: Mike Jolley <mike.jolley@me.com> --------- Co-authored-by: Mike Jolley <mike.jolley@me.com> * Update readme.txt * Add testing notes * Update testing notes * Fix Customer account sidebar link incorrect margin in WP 6.2 (#8437) * Fix Customer account sidebar link incorrect margin in WP 6.2 * Update class name to match the guidelines * Prevent cart from breaking when item_data contains an array (#8440) * Ensure array item data is removed * Remove unused key * Clean up code and add comments * Check for null instead of empty * Use plain foreach to filter and map arrays * Add minimum height to Mini Cart Contents block in the Style Book (#8458) * Update testing notes zip file * Update testing notes * Update testing notes file * Bumping version strings to new version. * Empty commit for release pull request * Empty commit for release pull request * disable compatibilty layer (#8507) * update changelog and testing instructions * add zip link * Bumping version strings to new version. * Empty commit for release pull request * Empty commit for release pull request * Add changelog in readme.txt * Add testing notes * Remove change from testing notes This requires AvaTax credentials for testing. So, we'll test for regressions instead * Check for null session before going forward (#8537) * Fix Payment Options settings crash in the editor (#8535) * Ensure express payment buttons are visible next to each other (#8548) * Update ZIP file * Bumping version strings to new version. * Empty commit for release pull request * Empty commit for release pull request * Update readme.txt * Show three Express Payments buttons in-line (#8601) * Add testing notes * Bumping version strings to new version. * Empty commit for release pull request * Empty commit for release pull request * Add changelog in readme.txt * Undo dirty prop removal on error (#8633) Co-authored-by: Saad Tarhi <saad.trh@gmail.com> * Add testing notes * fix 404 error (#8445) * Empty commit for release pull request * add testing instruction * update zip link * fix version --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Saad Tarhi <saad.trh@gmail.com> Co-authored-by: Seghir Nadir <nadir.seghir@gmail.com> Co-authored-by: Mike Jolley <mike.jolley@me.com> Co-authored-by: Albert Juhé Lluveras <contact@albertjuhe.com> Co-authored-by: Thomas Roberts <5656702+opr@users.noreply.github.com> Co-authored-by: Luigi Teschio <gigitux@gmail.com> Co-authored-by: Niels Lange <info@nielslange.de>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It was reported in p1676459035348189-slack-C01DT6U03HC that some users were seeing fatal errors relating to the wrong type being passed to
wp_strip_all_tags, this can occur when an extension adds item data, but one of the values is an array, rather than a scalar.Other Checks
wp_strip_all_tagsI would like us to be super sure this change has no security impact. Note, I don't think it does because we are simply returning an empty array, so the untrusted data is entirely removed OR passed throughwp_strip_all_tags.Testing
Automated Tests
User Facing Testing
Other data: Should showon the cart items. Ensure the script is not rendered or executed!WooCommerce Visibility
Performance Impact
Changelog