system_get_date_types() returns invalid array keys if no `$type` argument is used

backdrop / backdrop-issues

Issue tracker for Backdrop core.

144 stars 38 forks source link

system_get_date_types() returns invalid array keys if no `$type` argument is used #6556

Open anemirovsky opened 2 weeks ago

anemirovsky commented 2 weeks ago

Description of the bug

I'm using the workflow and wokflow_node modules and have hit a bug in https://github.com/backdrop-contrib/workflow/blob/1.x-2.x/workflow_node/workflownode.tokens.inc#L73. That code calls system_get_date_types to generate some tokens based on the available date formats. Currently, it will throw a TypeError: Cannot access offset of type string on string in workflownode_token_info(). Looking into system_get_date_types(), it looks to me like the conditional logic for checking for whether we're looking for a specific $type or not is flipped and so additional array keys with empty values are getting added to the $formats array if no $type argument is used.

Steps To Reproduce

Enable the workflow and workflow_node modules and the error should be immediately triggered.

I'll be submitting PR shortly for review.

klonos commented 2 weeks ago

Thanks @anemirovsky for raising this issue and for providing details and a PR 🙏🏼

I have done this quick test with a demo sandbox:

installed and enabled the devel module
enabled all errors in admin/config/development/logging
used the "Execute PHP code" feature provided by the devel module and simply run this:
```
$date_types = system_get_date_types();
dpm($date_types);
```
Executed the code

I got the results as expected (I think? ...there's an array with info about all date formats on the site), but also these two warnings:

Warning: Undefined array key "name" in system_get_date_types() (line 2571 of /var/lib/tugboat/backdrop/core/includes/drupal.inc).
Warning: Undefined array key "label" in system_get_date_types() (line 2572 of /var/lib/tugboat/backdrop/core/includes/drupal.inc).

I'll need to also test things with adding custom date formats (other then those provided OOTB with core) + also compare the same with the changes proposed in the PR.

Thanks again for the PR @anemirovsky. I'll loop back to this to do proper review/testing as soon as possible.

herbdool commented 2 weeks ago

system_date_get_types() is deprecated so I feel uncomfortable trying to fix things there. It's better to first try convert your code to use https://docs.backdropcms.org/api/backdrop/core%21modules%21system%21system.module/function/system_get_date_formats/1 and then if you still have errors we can figure out if it's a core bug.