Hi, I'm using "rules_data_type_date" Class to handle rules with dates objects, but the rules_data_type_date::check_value() returns a valid date even if it's input value is wrong.

The main reason rely on "rules_gmstrtotime($time)" function that, if the given $time parameter is an invalid string, it returns "1970-01-01 00:00:00".
This is caused by the unchecked return value of php strtotime() function invocation: it returns FALSE if the given time is not valid.

IMHO rules_gmstrtotime() should return NULL if the given parameter is invalid, and then rules_data_type_date::check_value() must check if NULL is returned.

Snippet from *new* rules_gmstrtotime() function

    // If the time string is not in plain old date format (e.g. 'now', '+1 day'
    // etc.), we need to convert it first.
    $value = strtotime($time);
    if ($value === FALSE)
      return NULL;
    $date = gmdate('Y-m-d H:i:s', $value);
    return strtotime($date .' UTC');
  } 

Suggested *new* rules_data_type_date::check_value() implementation

    function check_value($info, $value) {
      if (is_numeric($value)) {
        $value = gmdate('Y-m-d H:i:s', $value);
      }
      else if (is_string($value) && ($time = rules_gmstrtotime($value)) != NULL) {
        $value = gmdate('Y-m-d H:i:s', $time);
      }
      if (is_string($value) && preg_match(RULES_DATE_REGEX_LOOSE, $value)) {
        return $value;
      }
    }

Regards

Comments

thepanz’s picture

Title: Wrong rules_gmstrtotime conversions » Wrong rules_gmstrtotime() value checks
fago’s picture

Status: Needs review » Needs work

Please create a proper patch, see here.

thepanz’s picture

Pathc attached

mitchell’s picture

Component: Rules Engine » Rules Core
Status: Needs work » Needs review
tr’s picture

Title: Wrong rules_gmstrtotime() value checks » Wrong gmstrtotime() value checks
Version: 6.x-1.x-dev » 7.x-2.x-dev
Category: Bug report » Feature request
Priority: Major » Normal
Status: Needs review » Needs work

This code still exists in D7 Rules (but not in D8), so I'm moving the version and changing this into a feature request - it doesn't seem to have caused problems for anyone over the years ... (no other issues mention this).

The patch was for D6, so obviously doesn't apply anymore. Also, there are dpm() calls left in there that should not be in there.

If someone wants to work on this for D7, please supply a patch and preferably also a test case.