Forms: Fix fatal error when field render value is an array#48032
Forms: Fix fatal error when field render value is an array#48032
Conversation
Guard against array values being used as array keys in the get_legacy_extra_values method. When a form field (e.g., email) receives array POST data, get_render_value() returns an array which cannot be used as a PHP array key, causing a TypeError fatal error. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
For field types that should always be strings (email, name, url, subject, textarea, ip), take the first element when array POST data is submitted. Also use first-element fallback in get_legacy_extra_values as a defensive guard for any pre-existing array values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Code Coverage SummaryCoverage changed in 1 file.
|
Use direct Feedback_Field construction with array value for a url-type field and v2 format storage, which works in isolation. The key is using 'submit' context — get_render_submit_value() returns the raw array, unlike 'default' which implodes it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Test every field type in $non_extra_fields (email, name, url, subject, textarea, ip) with array values across both submit and default contexts. Also verify checkbox-multiple arrays are preserved in submit context and imploded in default context. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@copilot review this PR. |
Assert that extra values contain expected field values (Comment text field in parameterized tests, checkbox-multiple array in preservation test), not just that the method returns an array. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review complete (standard depth, 152 lines, 1 project). Verdict: Minor issues — can merge after addressing. Two suggestions found, no blockers:
Everything else looks good: the two-layer fix (prevent bad state at ingestion + guard at consumption) is the right approach, test coverage is thorough across all |
…over both fixed sites Agent-Logs-Url: https://github.com/Automattic/jetpack/sessions/7b5b7414-2667-4535-9781-d5ef5ee74cac Co-authored-by: enejb <115071+enejb@users.noreply.github.com>
Legacy v1 format stores checkbox-multiple as JSON arrays with field type 'basic'. Verify the full array survives the round-trip and is not flattened by the reset() guard in get_legacy_extra_values(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dhasilva
left a comment
There was a problem hiding this comment.
LGTM. Tested the image select field with multiple images and it still works.
The type-aware unwrap logic added in e29b191 was broader than necessary. The minimal fix for the fatal is to handle arrays in sanitization; preserve arrays for all field types and let downstream code handle them.
Fixes FORMS-670
Proposed changes
Fixes
PHP Fatal error: Uncaught TypeError: Cannot access offset of type array on arrayinFeedback::get_legacy_extra_values().Root cause: When a form field like
email,name, orurlreceives array POST data (e.g. crafted requests sendingemail[]=a&email[]=b),get_field_value()stores the value as an array. In thesubmitcontext,get_render_submit_value()returns this raw array, which then gets used as an array key inget_legacy_extra_values()— causing a TypeError.Fix (two layers):
Source enforcement (
get_field_value()): For all field types exceptcheckbox-multiple(the only type that legitimately accepts arrays), flatten array POST data to the first element.fileandimage-selectalready have dedicated early returns.Defensive guard (
get_legacy_extra_values()): If a render value is still an array (e.g. from pre-existing stored data), usereset()to extract the first element before using it as an array key — instead of crashing.Tests added:
$non_extra_fieldstypes (email, name, url, subject, textarea, ip) with array values in bothsubmitanddefaultcontextscheckbox-multiplearrays are preserved throughget_legacy_extra_values()(v2 format with correct type)checkbox-multiplearrays (stored with typebasic) survive the round-trip and are not flattened by thereset()guardRelated product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
Automated:
Or run just the new tests: