backdrop-issues icon indicating copy to clipboard operation
backdrop-issues copied to clipboard

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

Open anemirovsky opened this issue 1 year ago • 2 comments

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.

anemirovsky avatar May 17 '24 17:05 anemirovsky

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

I have done this quick test with a demo sandbox:

  1. installed and enabled the devel module
  2. enabled all errors in admin/config/development/logging
  3. used the "Execute PHP code" feature provided by the devel module and simply run this:
    $date_types = system_get_date_types();
    dpm($date_types);
    
  4. 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.

klonos avatar May 17 '24 21:05 klonos

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.

herbdool avatar May 18 '24 00:05 herbdool