Fix Tracks events rejected due to bracket notation in array properties #61729

chihsuan · 2025-10-31T06:59:55Z

Submission Review Guidelines:

I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have reviewed my code for security best practices.
Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

Fixes a bug in WooCommerce Tracks where array event properties are incorrectly serialized, causing events to be rejected due to invalid property names.

Problem:

When arrays are passed as event properties to WC_Tracks::record_event(), the http_build_query() function converts them into bracket notation.

For example: additional_blocks_on_checkout_page[0]=value1&additional_blocks_on_checkout_page[1]=value2. These property names with brackets ([0], [1]) violate the required property naming convention (/^[a-z_][a-z0-9_]*$/), causing Tracks events to be rejected.

Solution:

This PR implements automatic array value sanitization in WC_Tracks_Event::validate_and_sanitize():

Empty arrays → empty string ''
Indexed arrays with scalar values (e.g., ['a', 'b', 'c']) → URL-encoded comma-separated string 'a,b,c'
Associative arrays (e.g., ['key' => 'val']) → JSON string '{"key":"val"}'
Nested/complex arrays → JSON string

Changes:

Added sanitize_property_values() method to handle array conversion
Integrated sanitization into validate_and_sanitize() to ensure consistent return type (object)
Updated PHPDoc in WC_Tracks::record_event() with examples and documentation
Added 8 comprehensive unit tests covering all array scenarios

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide:

Automated Testing

Run the WC Tracks unit tests:

pnpm --filter @woocommerce/plugin-woocommerce run test:php:env -- --filter WC_Tracks_Test

Verify all 8 new array-related tests pass.

Manual Testing

Add this code to your theme's functions.php or a custom plugin:

   add_action('admin_init', function() {       
       // Test indexed array
       WC_Tracks::record_event('test_array', [
           'blocks' => ['woocommerce/cart-items', 'woocommerce/checkout-totals']
       ]);
       
       // Test associative array
       WC_Tracks::record_event('test_array', [
           'settings' => ['enabled' => true, 'count' => 5]
       ]);
       
       // Test nested array
       WC_Tracks::record_event('test_array', [
           'items' => [
               ['id' => 1, 'name' => 'Item 1'],
               ['id' => 2, 'name' => 'Item 2']
           ]
       ]);
   });


function log_remote_pixels( $preempt, $parsed_args, $url ) {
	if ( strpos( $url, WC_Tracks_Client::PIXEL ) === 0  ) {
		$parsed_url = wp_parse_url( $url );
		error_log( 'PIXEL URL: ' . $url );
	}

	return $preempt;
}

add_action( 'pre_http_request', 'log_remote_pixels', 10, 3 );

Enable WooCommerce tracking in Settings → Advanced → WooCommerce.com
Load any admin page to trigger the test events
Check the wp-content/debug.log
Verify that URLs contain:
- blocks=woocommerce%2Fcart-items%2Cwoocommerce%2Fcheckout-totals (comma-separated)
- settings=%7B%22enabled%22%3Atrue%2C%22count%22%3A5%7D (JSON encoded)
- items=%5B%7B%22id%22%3A1... (JSON encoded)
Verify URLs do NOT contain bracket notation in property names like:
- blocks[0]=value or blocks%5B0%5D=value
- items[0][id]=1 or similar
Optionally, verify that events are recorded in the "Tracks Live View" and confirm the events are not rejected.

Testing that has already taken place:

Unit tests: All 8 new unit tests pass, verifying array conversion logic for all edge cases
Analysis: Verified that the fix prevents bracket notation in property names while allowing brackets in JSON-encoded values

Changelog entry

Automatically create a changelog entry from the details below.

Changelog Entry Details

Significance

Patch

Type

Fix - Fixes an existing bug

Message

Fix Tracks events being rejected when array properties are passed by automatically converting arrays to valid formats

When arrays are passed to WC_Tracks::record_event(), http_build_query() creates bracket notation in property names (e.g., prop[0], prop[1]) which violates the required naming pattern and causes event rejection. This fix implements automatic array sanitization in validate_and_sanitize(): - Empty arrays → empty string - Indexed arrays with scalars → comma-separated string (URL-encoded) - Associative/nested arrays → JSON string Aligns with Jetpack consumer-side fix: Automattic/jetpack#45544 Changes: - Add sanitize_property_values() method (PHP 7.4+ compatible) - Integrate into validate_and_sanitize() preserving object return type - Document array handling in WC_Tracks::record_event() PHPDoc - Add 8 comprehensive unit tests for all array scenarios 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

coderabbitai · 2025-10-31T07:05:28Z

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Adds a private sanitizer to WC_Tracks_Event that normalizes array-valued event properties (empty arrays → "", indexed scalar arrays → comma-delimited strings, associative/nested arrays → JSON strings), applies it in validate_and_sanitize (docblock updated), adds unit tests, and adds a changelog entry.

Changes

Cohort / File(s)	Summary
Event sanitization implementation `plugins/woocommerce/includes/tracks/class-wc-tracks-event.php`	Adds `private static function sanitize_property_values($properties)` to normalize array properties (empty → `""`, indexed scalar arrays → comma-delimited strings, associative/nested arrays → JSON). Integrates this sanitizer into `validate_and_sanitize()` and updates its docblock return annotation to `object
Documentation (docblock only) `plugins/woocommerce/includes/tracks/class-wc-tracks.php`	Expands `record_event()` docblock to document automatic conversion rules for array property values and adds usage examples; no runtime code changes.
Unit tests `plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php`	Adds seven tests covering array serialization and URL encoding: `test_array_property_indexed_array`, `test_array_property_associative_array`, `test_array_property_empty_array`, `test_array_property_nested_array`, `test_array_property_with_commas`, `test_array_property_names_remain_valid`, `test_multiple_array_properties`.
Changelog `plugins/woocommerce/changelog/61729-fix-tracks-array-serialization`	Adds changelog entry describing the fix for Tracks array property serialization.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Caller as Event Caller
    participant WCEvent as WC_Tracks_Event
    participant Validator as validate_and_sanitize()
    participant Sanitizer as sanitize_property_values()
    participant Pixel as build_pixel_url()
    Note over Sanitizer: (new) normalizes array properties:
    Note over Sanitizer: - empty arrays -> ""\n- indexed scalar arrays -> comma string\n- associative/nested -> JSON string

    Caller->>WCEvent: new WC_Tracks_Event(props)
    WCEvent->>Validator: validate_and_sanitize()
    Validator->>Sanitizer: sanitize_property_values(props)
    Sanitizer-->>Validator: normalized props (strings / JSON)
    Validator-->>WCEvent: validated & sanitized event (object or WP_Error)
    WCEvent->>Pixel: build_pixel_url()
    Pixel-->>Caller: pixel URL (array props serialized, no bracketed params)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Verify correct detection of indexed vs associative arrays and preservation of input type (object vs array) in sanitize_property_values().
Check for accidental duplication or placement issues where the helper was inserted.
Confirm URL construction/encoding avoids bracketed query keys and properly encodes commas and JSON strings.
Review new unit tests for coverage of nested/non-scalar edge cases and correct assertions.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "Fix Tracks events rejected due to bracket notation in array properties" directly and clearly describes the main objective of the changeset. The PR specifically addresses a bug where array properties are serialized with bracket notation that violates Tracks naming conventions, causing events to be rejected. The title is concise, specific enough for quick understanding when scanning history, and accurately reflects the primary change across all modified files including the new sanitization method, integration into validation logic, documentation updates, and comprehensive test coverage.
Description Check	✅ Passed	The description is highly relevant to the changeset, providing a thorough explanation of the problem (array properties being serialized with bracket notation), the implemented solution (automatic conversion of arrays to valid formats with specific rules for empty, indexed, associative, and nested arrays), and the supporting changes including documentation updates and eight new unit tests. The description includes testing instructions, examples, and context linking to related issues, all of which demonstrate clear connection to the actual code changes without being off-topic or vague.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/tracks-array-serialization

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e1bd2c5 and fd5db95.

📒 Files selected for processing (3)

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php (3 hunks)
plugins/woocommerce/includes/tracks/class-wc-tracks.php (1 hunks)
plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.{php,js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/code-quality.mdc)

**/*.{php,js,jsx,ts,tsx}: Guard against unexpected inputs
Sanitize and validate any potentially dangerous inputs
Ensure code is backwards compatible
Write code that is readable and intuitive
Ensure code has unit or E2E tests where applicable

Files:

plugins/woocommerce/includes/tracks/class-wc-tracks.php
plugins/woocommerce/includes/tracks/class-wc-tracks-event.php
plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php

**/*.{php,js,jsx,tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/avoid-regex.mdc)

**/*.{php,js,jsx,tsx,ts}: Avoid using regular expressions unless absolutely necessary to favor readability and maintainability
Only consider regex when no built-in language alternative (string/array APIs) fits the need
Only use regex when performance is critical and faster than alternatives, with measurements to justify
Use regex for complex pattern matching only if the pattern is well-documented and thoroughly tested
Allow regex when maintaining legacy code where an existing, correct pattern is being modified
If regex is necessary, document the pattern extensively to explain what it matches
If regex is necessary, add comprehensive tests covering edge cases and potential security issues
Use named capture groups in regex to improve readability when supported
Validate input before applying regex to ensure it is safe
Assess and mitigate security risks when using regex, including ReDoS and injection vulnerabilities
Avoid regex patterns that can cause catastrophic backtracking (ReDoS)
Do not construct regex from untrusted input to prevent injection attacks
Ensure regex patterns do not overmatch and unintentionally capture unexpected inputs

Files:

plugins/woocommerce/includes/tracks/class-wc-tracks.php
plugins/woocommerce/includes/tracks/class-wc-tracks-event.php
plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php

**/*.{php,js,ts,jsx,tsx}

⚙️ CodeRabbit configuration file

**/*.{php,js,ts,jsx,tsx}: Don't trust that extension developers will follow the best practices, make sure the code:

Guards against unexpected inputs.

Sanitizes and validates any potentially dangerous inputs.

Is backwards compatible.

Is readable and intuitive.

Has unit or E2E tests where applicable.
When making any changes to code that deletes or modifies orders/products/customer data, make sure that there are
sufficient checks in place to prevent accidental data loss. As an example, if deleting a draft order, check that
the order status is indeed draft or checkout-draft. Also think about whether race conditions could occur and
delete orders that don't belong to the current customer. When in doubt, ask for clarification in the PR comments.

Files:

plugins/woocommerce/includes/tracks/class-wc-tracks.php
plugins/woocommerce/includes/tracks/class-wc-tracks-event.php
plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php

plugins/woocommerce/tests/**/*.php

📄 CodeRabbit inference engine (.cursor/rules/woo-phpunit.mdc)

plugins/woocommerce/tests/**/*.php: Ensure test classes define public setUp() and tearDown() methods (not protected)
All PHPUnit test methods must be public, not protected
Add declare(strict_types=1); at the top of each test file
Test classes should extend WC_Unit_Test_Case

Files:

plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php

🧠 Learnings (7)

📓 Common learnings

Learnt from: joshuatf
Repo: woocommerce/woocommerce PR: 60324
File: packages/js/product-editor/src/components/content-preview/content-preview.tsx:83-88
Timestamp: 2025-08-29T09:31:00.392Z
Learning: In WooCommerce sanitization patterns, always defensively coerce inputs to strings before passing to sanitizeHTML using `String( input ?? '' )` to guard against null, undefined, or non-string values that could cause runtime errors.

Learnt from: joshuatf
Repo: woocommerce/woocommerce PR: 60324
File: packages/js/product-editor/src/blocks/generic/toggle/edit.tsx:74-75
Timestamp: 2025-08-29T09:30:44.987Z
Learning: In the WooCommerce woocommerce/sanitize package migration, the design goal is to centralize sanitization with secure defaults. Usage sites should call sanitizeHTML() without explicit allowlist configuration, as the package already provides DEFAULT_ALLOWED_TAGS and DEFAULT_ALLOWED_ATTR with appropriate security boundaries. Adding explicit configuration at every call site defeats the purpose of the centralized approach.

Learnt from: senadir
Repo: woocommerce/woocommerce PR: 59426
File: plugins/woocommerce/client/legacy/js/frontend/a8c-address-autocomplete-service.js:355-355
Timestamp: 2025-07-08T09:14:47.894Z
Learning: In the WooCommerce address autocomplete system, input sanitization happens upstream in address-autocomplete.js before data reaches a8c-address-autocomplete-service.js. The trim() operation in the service file is for optimization purposes (to skip API requests when only spaces are added), not for security sanitization.

📚 Learning: 2025-08-29T09:30:44.987Z

Learnt from: joshuatf
Repo: woocommerce/woocommerce PR: 60324
File: packages/js/product-editor/src/blocks/generic/toggle/edit.tsx:74-75
Timestamp: 2025-08-29T09:30:44.987Z
Learning: In the WooCommerce woocommerce/sanitize package migration, the design goal is to centralize sanitization with secure defaults. Usage sites should call sanitizeHTML() without explicit allowlist configuration, as the package already provides DEFAULT_ALLOWED_TAGS and DEFAULT_ALLOWED_ATTR with appropriate security boundaries. Adding explicit configuration at every call site defeats the purpose of the centralized approach.

Applied to files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

📚 Learning: 2025-08-29T09:31:00.392Z

Learnt from: joshuatf
Repo: woocommerce/woocommerce PR: 60324
File: packages/js/product-editor/src/components/content-preview/content-preview.tsx:83-88
Timestamp: 2025-08-29T09:31:00.392Z
Learning: In WooCommerce sanitization patterns, always defensively coerce inputs to strings before passing to sanitizeHTML using `String( input ?? '' )` to guard against null, undefined, or non-string values that could cause runtime errors.

Applied to files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

📚 Learning: 2025-08-18T06:11:48.768Z

Learnt from: CR
Repo: woocommerce/woocommerce PR: 0
File: .cursor/rules/woo-phpunit.mdc:0-0
Timestamp: 2025-08-18T06:11:48.768Z
Learning: Applies to plugins/woocommerce/tests/**/*.php : Add declare(strict_types=1); at the top of each test file

Applied to files:

plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php

📚 Learning: 2025-08-18T06:11:48.768Z

Learnt from: CR
Repo: woocommerce/woocommerce PR: 0
File: .cursor/rules/woo-phpunit.mdc:0-0
Timestamp: 2025-08-18T06:11:48.768Z
Learning: Applies to plugins/woocommerce/tests/**/*.php : Test classes should extend WC_Unit_Test_Case

Applied to files:

plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php

📚 Learning: 2025-08-18T06:11:48.768Z

Learnt from: CR
Repo: woocommerce/woocommerce PR: 0
File: .cursor/rules/woo-phpunit.mdc:0-0
Timestamp: 2025-08-18T06:11:48.768Z
Learning: Applies to plugins/woocommerce/tests/**/*.php : All PHPUnit test methods must be public, not protected

Applied to files:

plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php

📚 Learning: 2025-09-03T11:50:02.208Z

Learnt from: tpaksu
Repo: woocommerce/woocommerce PR: 60735
File: plugins/woocommerce/tests/php/includes/rest-api/Controllers/Version4/Fulfillments/class-wc-rest-fulfillments-v4-controller-tests.php:10-10
Timestamp: 2025-09-03T11:50:02.208Z
Learning: For REST API controller tests in plugins/woocommerce/tests/**/*.php, test classes should extend WC_REST_Unit_Test_Case instead of WC_Unit_Test_Case because REST routes need to be registered before tests run.

Applied to files:

plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php

🧬 Code graph analysis (1)

plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php (1)

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php (1)

build_pixel_url (118-135)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (32)

GitHub Check: Blocks e2e tests 6/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 3/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 8/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests - Legacy MiniCart 1/3 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 1/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 9/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Core e2e tests 6/6 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 4/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 7/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Metrics - @woocommerce/plugin-woocommerce [performance]
GitHub Check: Blocks e2e tests 10/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 5/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 2/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Core e2e tests - Legacy MiniCart - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: PHP: 8.4 WP: latest (HPOS:off) [WP latest] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: Core e2e tests 5/6 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Core API tests - @woocommerce/plugin-woocommerce [api]
GitHub Check: PHP: 8.4 WP: latest (HPOS:off) [WP latest] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: Core e2e tests 4/6 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Core e2e tests 3/6 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: PHP: 8.4 WP: latest - Legacy MiniCart [WP latest] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: Core e2e tests 1/6 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: PHP: 7.4 WP: pre-release [WP 6.9-beta2] 1/2 - @woocommerce/plugin-woocommerce [unit:php] (optional)
GitHub Check: PHP: 7.4 WP: pre-release [WP 6.9-beta2] 2/2 - @woocommerce/plugin-woocommerce [unit:php] (optional)
GitHub Check: PHP: 7.4 WP: latest - 1 [WP 6.7.4] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: PHP: 8.4 WP: latest - Legacy MiniCart [WP latest] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: Core e2e tests 2/6 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: PHP: 8.4 WP: latest [WP latest] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: PHP: 8.4 WP: latest [WP latest] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: PHP: 7.4 WP: latest - 1 [WP 6.7.4] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: Lint - @woocommerce/plugin-woocommerce
GitHub Check: build

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php

…mmerce

github-actions · 2025-10-31T07:07:09Z

Test using WordPress Playground

The changes in this pull request can be previewed and tested using a WordPress Playground instance.
WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Test this pull request with WordPress Playground.

Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit.

Updated the handling of indexed arrays in WC_Tracks_Event to directly use implode for creating comma-separated strings, removing unnecessary URL encoding. Adjusted unit tests to verify the expected string outputs without checking for bracket notation or JSON encoding, ensuring cleaner assertions for URL parameters. Changes: - Simplified value assignment for indexed arrays - Updated assertions in unit tests to check for string containment instead of exact matches This change aligns with the recent improvements in array handling for event tracking.

chihsuan · 2025-10-31T07:43:38Z

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

 	 *
 	 * @param  array $event Event arguments.
-	 * @return bool|WP_Error True on success, WP_Error on failure.
+	 * @return object|WP_Error Event object on success, WP_Error on failure.


Fixed the type hint for the $event parameter to reflect that it returns either an object or a WP_Error, not a boolean.

github-actions · 2025-10-31T07:44:39Z

Testing Guidelines

Hi @woocommerce/ventures,

Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed.

Reminder: PR reviewers are required to document testing performed. This includes:

🖼️ Screenshots or screen recordings.
📝 List of functionality tested / steps followed.
🌐 Site details (environment attributes such as hosting type, plugins, theme, store size, store age, and relevant settings).
🔍 Any analysis performed, such as assessing potential impacts on environment attributes and other plugins, conducting performance profiling, or using LLM/AI-based analysis.

⚠️ Within the testing details you provide, please ensure that no sensitive information (such as API keys, passwords, user data, etc.) is included in this public issue.

Copilot

Pull Request Overview

This PR fixes an issue where WooCommerce Tracks events were being rejected when array properties were passed. The solution automatically converts array properties to valid formats: indexed arrays become comma-separated strings, associative arrays become JSON strings, and empty arrays become empty strings. This prevents http_build_query() from generating bracket notation (e.g., prop[0]) that violates the property name regex.

Adds automatic array property conversion in sanitize_property_values() method
Updates documentation to explain the array handling behavior with examples
Adds comprehensive test coverage for various array scenarios (indexed, associative, nested, empty, with commas, and multiple array properties)

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
`plugins/woocommerce/includes/tracks/class-wc-tracks-event.php`	Adds `sanitize_property_values()` method to convert arrays before serialization and updates return type documentation
`plugins/woocommerce/includes/tracks/class-wc-tracks.php`	Adds comprehensive documentation explaining array conversion behavior with examples
`plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php`	Adds 7 new test cases covering various array property scenarios
`plugins/woocommerce/changelog/61729-fix-tracks-array-serialization`	Adds changelog entry for the fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

chihsuan · 2025-10-31T07:50:21Z

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

+
+			if ( $is_indexed_array && $has_scalar_only ) {
+				// Indexed arrays with scalar values: join as comma string.
+				$props[ $key ] = implode( ',', array_map( 'strval', $value ) );


This maintains the same behavior as when tracking events in JavaScript.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php (1)
147-149: Consider adding defensive input validation.

While this private method is currently only called with validated objects, adding a type check would guard against future refactoring risks and align with defensive coding practices.

Apply this diff to add input validation:
 private static function sanitize_property_values( $properties ) {
+	if ( ! is_object( $properties ) && ! is_array( $properties ) ) {
+		return $properties;
+	}
 	$is_object = is_object( $properties );
 	$props     = $is_object ? get_object_vars( $properties ) : $properties;
As per coding guidelines

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 472bd49 and c9e187b.

📒 Files selected for processing (1)

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php (3 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{php,js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/code-quality.mdc)

**/*.{php,js,jsx,ts,tsx}: Guard against unexpected inputs
Sanitize and validate any potentially dangerous inputs
Ensure code is backwards compatible
Write code that is readable and intuitive
Ensure code has unit or E2E tests where applicable

Files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

**/*.{php,js,jsx,tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/avoid-regex.mdc)

**/*.{php,js,jsx,tsx,ts}: Avoid using regular expressions unless absolutely necessary to favor readability and maintainability
Only consider regex when no built-in language alternative (string/array APIs) fits the need
Only use regex when performance is critical and faster than alternatives, with measurements to justify
Use regex for complex pattern matching only if the pattern is well-documented and thoroughly tested
Allow regex when maintaining legacy code where an existing, correct pattern is being modified
If regex is necessary, document the pattern extensively to explain what it matches
If regex is necessary, add comprehensive tests covering edge cases and potential security issues
Use named capture groups in regex to improve readability when supported
Validate input before applying regex to ensure it is safe
Assess and mitigate security risks when using regex, including ReDoS and injection vulnerabilities
Avoid regex patterns that can cause catastrophic backtracking (ReDoS)
Do not construct regex from untrusted input to prevent injection attacks
Ensure regex patterns do not overmatch and unintentionally capture unexpected inputs

Files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

**/*.{php,js,ts,jsx,tsx}

⚙️ CodeRabbit configuration file

**/*.{php,js,ts,jsx,tsx}: Don't trust that extension developers will follow the best practices, make sure the code:

Guards against unexpected inputs.

Sanitizes and validates any potentially dangerous inputs.

Is backwards compatible.

Is readable and intuitive.

Has unit or E2E tests where applicable.
When making any changes to code that deletes or modifies orders/products/customer data, make sure that there are
sufficient checks in place to prevent accidental data loss. As an example, if deleting a draft order, check that
the order status is indeed draft or checkout-draft. Also think about whether race conditions could occur and
delete orders that don't belong to the current customer. When in doubt, ask for clarification in the PR comments.

Files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

🧠 Learnings (6)

📓 Common learnings

Learnt from: joshuatf
Repo: woocommerce/woocommerce PR: 60324
File: packages/js/product-editor/src/blocks/generic/toggle/edit.tsx:74-75
Timestamp: 2025-08-29T09:30:44.987Z
Learning: In the WooCommerce woocommerce/sanitize package migration, the design goal is to centralize sanitization with secure defaults. Usage sites should call sanitizeHTML() without explicit allowlist configuration, as the package already provides DEFAULT_ALLOWED_TAGS and DEFAULT_ALLOWED_ATTR with appropriate security boundaries. Adding explicit configuration at every call site defeats the purpose of the centralized approach.

📚 Learning: 2025-06-17T07:07:53.443Z

Learnt from: samueljseay
Repo: woocommerce/woocommerce PR: 58716
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/iapi-frontend.ts:83-101
Timestamp: 2025-06-17T07:07:53.443Z
Learning: In WooCommerce blocks, when porting existing code patterns that have known issues (like parseInt truncating decimal money values), maintain consistency with existing implementation rather than making isolated fixes. The preference is for systematic refactoring approaches (like broader Dinero adoption) over piecemeal changes.

Applied to files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

📚 Learning: 2025-08-29T09:31:00.392Z

Learnt from: joshuatf
Repo: woocommerce/woocommerce PR: 60324
File: packages/js/product-editor/src/components/content-preview/content-preview.tsx:83-88
Timestamp: 2025-08-29T09:31:00.392Z
Learning: In WooCommerce sanitization patterns, always defensively coerce inputs to strings before passing to sanitizeHTML using `String( input ?? '' )` to guard against null, undefined, or non-string values that could cause runtime errors.

Applied to files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

📚 Learning: 2025-08-29T09:30:44.987Z

Learnt from: joshuatf
Repo: woocommerce/woocommerce PR: 60324
File: packages/js/product-editor/src/blocks/generic/toggle/edit.tsx:74-75
Timestamp: 2025-08-29T09:30:44.987Z
Learning: In the WooCommerce woocommerce/sanitize package migration, the design goal is to centralize sanitization with secure defaults. Usage sites should call sanitizeHTML() without explicit allowlist configuration, as the package already provides DEFAULT_ALLOWED_TAGS and DEFAULT_ALLOWED_ATTR with appropriate security boundaries. Adding explicit configuration at every call site defeats the purpose of the centralized approach.

Applied to files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

📚 Learning: 2025-07-08T09:14:47.894Z

Learnt from: senadir
Repo: woocommerce/woocommerce PR: 59426
File: plugins/woocommerce/client/legacy/js/frontend/a8c-address-autocomplete-service.js:355-355
Timestamp: 2025-07-08T09:14:47.894Z
Learning: In the WooCommerce address autocomplete system, input sanitization happens upstream in address-autocomplete.js before data reaches a8c-address-autocomplete-service.js. The trim() operation in the service file is for optimization purposes (to skip API requests when only spaces are added), not for security sanitization.

Applied to files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

📚 Learning: 2025-07-18T14:54:36.232Z

Learnt from: CR
Repo: woocommerce/woocommerce PR: 0
File: .cursor/rules/code-quality.mdc:0-0
Timestamp: 2025-07-18T14:54:36.232Z
Learning: Applies to **/*.{php,js,jsx,ts,tsx} : Sanitize and validate any potentially dangerous inputs

Applied to files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)

GitHub Check: Blocks e2e tests 3/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests - Legacy MiniCart 1/3 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 9/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 7/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 6/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 8/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 10/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 4/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: PHP: 8.4 WP: latest (HPOS:off) [WP latest] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: PHP: 8.4 WP: latest (HPOS:off) [WP latest] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: Core e2e tests 6/6 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Metrics - @woocommerce/plugin-woocommerce [performance]
GitHub Check: Core e2e tests 2/6 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Blocks e2e tests 2/10 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Core API tests - @woocommerce/plugin-woocommerce [api]
GitHub Check: Core e2e tests - Legacy MiniCart - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Core e2e tests 4/6 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: Core e2e tests 5/6 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: PHP: 7.4 WP: latest - 1 [WP 6.7.4] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: PHP: 7.4 WP: pre-release [WP 6.9-beta2] 2/2 - @woocommerce/plugin-woocommerce [unit:php] (optional)
GitHub Check: PHP: 7.4 WP: pre-release [WP 6.9-beta2] 1/2 - @woocommerce/plugin-woocommerce [unit:php] (optional)
GitHub Check: Core e2e tests 1/6 - @woocommerce/plugin-woocommerce [e2e]
GitHub Check: PHP: 7.4 WP: latest - 1 [WP 6.7.4] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: PHP: 8.4 WP: latest [WP latest] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: PHP: 8.4 WP: latest - Legacy MiniCart [WP latest] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: PHP: 8.4 WP: latest [WP latest] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
GitHub Check: Lint - @woocommerce/plugin-woocommerce
GitHub Check: build

🔇 Additional comments (4)

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php (4)

69-69: LGTM!

The updated return type documentation accurately reflects that the method returns an event object on success or WP_Error on failure.

106-108: LGTM!

The sanitization is correctly placed after all validation checks, ensuring only valid events are processed. The method preserves the object type, maintaining consistency with the return type.

177-179: Excellent error handling for JSON encoding.

The use of JSON_HEX_TAG provides XSS protection by escaping HTML-sensitive characters, and the fallback to an empty string on encoding failure prevents events from being rejected entirely due to malformed UTF-8 or other encoding issues.

182-182: LGTM!

The method correctly preserves the input type (object or array), ensuring consistent behavior and maintaining the API contract described in the docblock.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php (1)
147-147: Consider adding type hints for better type safety.

Adding explicit type hints would make the method contract clearer and enable static analysis tools to catch potential type mismatches.

Consider this optional enhancement:
-	private static function sanitize_property_values( $properties ) {
+	private static function sanitize_property_values( object|array $properties ): object|array {
Note: This requires PHP 8.0+. If supporting earlier PHP versions, this can be deferred.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c9e187b and 01e2f38.

📒 Files selected for processing (1)

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php (3 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{php,js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/code-quality.mdc)

**/*.{php,js,jsx,ts,tsx}: Guard against unexpected inputs
Sanitize and validate any potentially dangerous inputs
Ensure code is backwards compatible
Write code that is readable and intuitive
Ensure code has unit or E2E tests where applicable

Files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

**/*.{php,js,jsx,tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/avoid-regex.mdc)

**/*.{php,js,jsx,tsx,ts}: Avoid using regular expressions unless absolutely necessary to favor readability and maintainability
Only consider regex when no built-in language alternative (string/array APIs) fits the need
Only use regex when performance is critical and faster than alternatives, with measurements to justify
Use regex for complex pattern matching only if the pattern is well-documented and thoroughly tested
Allow regex when maintaining legacy code where an existing, correct pattern is being modified
If regex is necessary, document the pattern extensively to explain what it matches
If regex is necessary, add comprehensive tests covering edge cases and potential security issues
Use named capture groups in regex to improve readability when supported
Validate input before applying regex to ensure it is safe
Assess and mitigate security risks when using regex, including ReDoS and injection vulnerabilities
Avoid regex patterns that can cause catastrophic backtracking (ReDoS)
Do not construct regex from untrusted input to prevent injection attacks
Ensure regex patterns do not overmatch and unintentionally capture unexpected inputs

Files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

**/*.{php,js,ts,jsx,tsx}

⚙️ CodeRabbit configuration file

**/*.{php,js,ts,jsx,tsx}: Don't trust that extension developers will follow the best practices, make sure the code:

Guards against unexpected inputs.

Sanitizes and validates any potentially dangerous inputs.

Is backwards compatible.

Is readable and intuitive.

Has unit or E2E tests where applicable.
When making any changes to code that deletes or modifies orders/products/customer data, make sure that there are
sufficient checks in place to prevent accidental data loss. As an example, if deleting a draft order, check that
the order status is indeed draft or checkout-draft. Also think about whether race conditions could occur and
delete orders that don't belong to the current customer. When in doubt, ask for clarification in the PR comments.

Files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

🧠 Learnings (6)

📓 Common learnings

Learnt from: joshuatf
Repo: woocommerce/woocommerce PR: 60324
File: packages/js/product-editor/src/blocks/generic/toggle/edit.tsx:74-75
Timestamp: 2025-08-29T09:30:44.987Z
Learning: In the WooCommerce woocommerce/sanitize package migration, the design goal is to centralize sanitization with secure defaults. Usage sites should call sanitizeHTML() without explicit allowlist configuration, as the package already provides DEFAULT_ALLOWED_TAGS and DEFAULT_ALLOWED_ATTR with appropriate security boundaries. Adding explicit configuration at every call site defeats the purpose of the centralized approach.

📚 Learning: 2025-08-29T09:30:44.987Z

Learnt from: joshuatf
Repo: woocommerce/woocommerce PR: 60324
File: packages/js/product-editor/src/blocks/generic/toggle/edit.tsx:74-75
Timestamp: 2025-08-29T09:30:44.987Z
Learning: In the WooCommerce woocommerce/sanitize package migration, the design goal is to centralize sanitization with secure defaults. Usage sites should call sanitizeHTML() without explicit allowlist configuration, as the package already provides DEFAULT_ALLOWED_TAGS and DEFAULT_ALLOWED_ATTR with appropriate security boundaries. Adding explicit configuration at every call site defeats the purpose of the centralized approach.

Applied to files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

📚 Learning: 2025-06-17T07:07:53.443Z

Learnt from: samueljseay
Repo: woocommerce/woocommerce PR: 58716
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/iapi-frontend.ts:83-101
Timestamp: 2025-06-17T07:07:53.443Z
Learning: In WooCommerce blocks, when porting existing code patterns that have known issues (like parseInt truncating decimal money values), maintain consistency with existing implementation rather than making isolated fixes. The preference is for systematic refactoring approaches (like broader Dinero adoption) over piecemeal changes.

Applied to files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

📚 Learning: 2025-08-29T09:31:00.392Z

Learnt from: joshuatf
Repo: woocommerce/woocommerce PR: 60324
File: packages/js/product-editor/src/components/content-preview/content-preview.tsx:83-88
Timestamp: 2025-08-29T09:31:00.392Z
Learning: In WooCommerce sanitization patterns, always defensively coerce inputs to strings before passing to sanitizeHTML using `String( input ?? '' )` to guard against null, undefined, or non-string values that could cause runtime errors.

Applied to files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

📚 Learning: 2025-07-08T09:14:47.894Z

Learnt from: senadir
Repo: woocommerce/woocommerce PR: 59426
File: plugins/woocommerce/client/legacy/js/frontend/a8c-address-autocomplete-service.js:355-355
Timestamp: 2025-07-08T09:14:47.894Z
Learning: In the WooCommerce address autocomplete system, input sanitization happens upstream in address-autocomplete.js before data reaches a8c-address-autocomplete-service.js. The trim() operation in the service file is for optimization purposes (to skip API requests when only spaces are added), not for security sanitization.

Applied to files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

📚 Learning: 2025-07-18T14:54:36.232Z

Learnt from: CR
Repo: woocommerce/woocommerce PR: 0
File: .cursor/rules/code-quality.mdc:0-0
Timestamp: 2025-07-18T14:54:36.232Z
Learning: Applies to **/*.{php,js,jsx,ts,tsx} : Sanitize and validate any potentially dangerous inputs

Applied to files:

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

🔇 Additional comments (3)

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php (3)

69-69: LGTM: Accurate return type documentation.

The updated docblock correctly reflects that the method returns either an object or a WP_Error, not a boolean.

106-107: LGTM: Proper sanitization placement.

Calling sanitize_property_values() after property name validation ensures that array values are normalized before serialization, preventing bracket notation in the generated URLs.

147-183: LGTM: Robust array sanitization with proper error handling.

The implementation correctly handles all specified array conversion cases:

Empty arrays → empty string

Indexed scalar arrays → comma-separated string (without double-encoding)

Associative/nested arrays → JSON string with safe encoding flags

The logic is defensive, preserves input type (object vs array), and properly checks for JSON encoding failures.

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Oct 31, 2025

chihsuan self-assigned this Oct 31, 2025

coderabbitai bot reviewed Oct 31, 2025

View reviewed changes

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php Outdated Show resolved Hide resolved

plugins/woocommerce/tests/php/includes/class-wc-tracks-test.php Outdated Show resolved Hide resolved

Add changefile(s) from automation for the following project(s): wooco…

0def031

…mmerce

chihsuan commented Oct 31, 2025

View reviewed changes

chihsuan requested a review from Copilot October 31, 2025 07:44

Copilot AI reviewed Oct 31, 2025

View reviewed changes

plugins/woocommerce/includes/tracks/class-wc-tracks-event.php Outdated Show resolved Hide resolved

Update plugins/woocommerce/includes/tracks/class-wc-tracks-event.php

c9e187b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

chihsuan commented Oct 31, 2025

View reviewed changes

coderabbitai bot reviewed Oct 31, 2025

View reviewed changes

Fix lint

01e2f38

chihsuan requested a review from a team October 31, 2025 07:52

coderabbitai bot reviewed Oct 31, 2025

View reviewed changes

Fix Tracks events rejected due to bracket notation in array properties #61729

Are you sure you want to change the base?

Fix Tracks events rejected due to bracket notation in array properties #61729

Conversation

chihsuan commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Submission Review Guidelines:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Automated Testing

Manual Testing

Testing that has already taken place:

Changelog entry

Significance

Type

Message

Uh oh!

coderabbitai bot commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Other AI code review bot(s) detected

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test using WordPress Playground

Uh oh!

chihsuan Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing Guidelines

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

chihsuan Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chihsuan commented Oct 31, 2025 •

edited

Loading

coderabbitai bot commented Oct 31, 2025 •

edited

Loading

github-actions bot commented Oct 31, 2025 •

edited

Loading

github-actions bot commented Oct 31, 2025 •

edited

Loading