function date_validate($form) {
  if (!checkdate($form['#value']['month'], $form['#value']['day'], $form['#value']['year'])) {
    form_error($form, t('The specified date is invalid.'));
  }
}

checkdate() requires integer parameters, otherwise it fails with something like

checkdate() expects parameter 1 to be long, string given

I don't know how he did it, but a user has managed to produce this error on a D6 site by submitting user/register. So we should probably use is_int() on all three values before passing them on to checkdate().

CommentFileSizeAuthor
#12 drupal-typecast-checkdate-480366-12.patch611 bytesjoelstein
#2 date_validate.patch1.2 KBcasey
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

casey’s picture

casey CreditAttribution: casey commented
Status: Active » Needs review
FileSize
date_validate.patch1.2 KB

I think you have strict error reporting enabled. A cast to (int) would do I think.

casey’s picture

casey CreditAttribution: casey commented

sorry patch contains also 2 whitespace@endofline removals

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented

I think you have strict error reporting enabled.

No, this showed up in the watchdog log on a production site with a normal distribution tarball installed. If you run

echo checkdate('a', 2, 3);

in Devel's Execute PHP block, you'll get that result.

Casting to int does avoid the error, at least on my system, which is running PHP 5.2.6. But will it work for all PHP versions and everything that gets thrown at it? The PHP doc (http://ch2.php.net/manual/en/language.types.integer.php#language.types.i...) is very vague about this, but it explicitely warns against trying to convert unknown types to int:

The behaviour of converting to integer is undefined for other types. Do not rely on any observed behaviour, as it can change without notice.

Rather than trying to cast to an int, would it be unreasonable to require ints, i.e. to use is_int() on the parameters and to reject anything else?

Dries’s picture

Dries CreditAttribution: Dries commented

The patch fixes the symptoms, but might not fix the real cause. I'd like to see if we can identify the cause first.

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented

I have no idea about the cause. As I wrote in the OP I found it in a watchdog entry for the user/register page. It may have been a hacking attempt, but it was an isolated occurrence.

casey’s picture

casey CreditAttribution: casey commented

I think it's just because form input is not strictly checked ('12' == 12, instead of '12' === 12), so _form_validate() passes (uses isset() which also doesn't check strictly).

checkdate() however seems to check its arguments strictly by type.

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented

No,

echo checkdate('1', 2, 3);

works just fine and returns 1, at least on my installation.

MichaelCole’s picture

MichaelCole CreditAttribution: MichaelCole commented

#2: date_validate.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, date_validate.patch, failed testing.

xaris.tsimpouris’s picture

xaris.tsimpouris CreditAttribution: xaris.tsimpouris commented

Why not using "@" with type-casting? No errors, job done.. not everybody happy?

joelstein’s picture

joelstein CreditAttribution: joelstein as a volunteer commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
drupal-typecast-checkdate-480366-12.patch611 bytes

We saw this issue in Webform (#2831514: Warning: checkdate() expects parameter 3 to be long, string given in webform_validate_date()) and I think we should cast the variables to int to prevent this error. checkdate() will still return FALSE if an invalid date value like "n/a" is given, but it will do so quietly. Here's an updated patch. Does this also need a test?