Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.4] Validation Rule: date_format bug #17642

Closed
SiNONiMiTY opened this issue Jan 29, 2017 · 21 comments
Closed

[5.4] Validation Rule: date_format bug #17642

SiNONiMiTY opened this issue Jan 29, 2017 · 21 comments

Comments

@SiNONiMiTY
Copy link

SiNONiMiTY commented Jan 29, 2017

As the title says, I may be missing something. The scenario is provided below.

Versions used

Laravel Version: 5.4.3
PHP Version: 7.0.15
Database Driver & Version: MariaDB 10.1.20

Problem

The Laravel 'date_format' validation rule on Requests fail on the date '2017-02'. It fails on non leap years when using the month '02' (February).

Test Data

Working on the following dates [Validation is successful]

2016-01
2016-02
2020-02
2024-02

Not working on the following dates [Validation does not match the format Y-m]

2013-02
2017-02
2021-02

Steps to Reproduce

Create a request with the following rule

return ['testDate' => ['bail', required, 'date_format:Y-m']];

Next steps

Create a sample input form, route, and controller that uses the request created.
Input the test data provided above.
Input test data '2017-02'. Expected: Validation Passed, Actual: Does not match the format Y-m

Other notes

The validation rule 'date' works properly even on non leap years when using the month '02' (February).
Tested on Laravel 5.3, same error.

@arjasco
Copy link
Contributor

arjasco commented Jan 31, 2017

I'm sure this is a PHP bug.

Any months with 31 days will format correctly.

I'm not a PHP internals expert here but It appears then when you define just the month it's using the last possible day out of all months (31) and therefore overflowing into the next month because e.g feb only has 28 days.

I wonder if we could do something like this that appears in Carbon
https://github.com/briannesbitt/Carbon/blob/master/src/Carbon/Carbon.php#L2218

@SiNONiMiTY
Copy link
Author

Hi, I tried running some quick tests on the validation rule 'date_format'

See this line here
https://github.com/laravel/framework/blob/5.4/src/Illuminate/Validation/Concerns/ValidatesAttributes.php#L353

It seems that you are right, it is a quirky behavior of the DateTime::createFromFormat method

I tested it using the input value '2017-02'

A quick call on dd($date) gave me

DateTime {#190 ▼
  +"date": "2017-03-03 14:30:01.000000"
  +"timezone_type": 3
  +"timezone": "UTC"
}

Next, I tried this dd($date->format($parameters[0])) and gave me

"2017-03"

Finally, dd($value)

"2017-02"

The return value looks like this
return $date && $date->format($parameters[0]) == $value

Translating it with the values

return true && ( "2017-03" == "2017-02" )
return true && false
return false

So it seems it is not a Laravel related bug. The suggestion you raised above on preparing a solution to handle overflows that is similar with the Carbon seems good.

@fernandobandeira
Copy link
Contributor

Then just add it to your own date format when you pass it to the validator.

Basically just change

return ['testDate' => ['bail', required, 'date_format:Y-m']];

to

return ['testDate' => ['bail', required, 'date_format:!Y-m']];

Should work...

@themsaid
Copy link
Member

themsaid commented Feb 1, 2017

@fernandobandeira thanks :)

@themsaid themsaid closed this as completed Feb 1, 2017
@themsaid
Copy link
Member

themsaid commented Feb 1, 2017

@MiSAKACHi please check the latest comments for a fix.

@arjasco
Copy link
Contributor

arjasco commented Feb 1, 2017

@themsaid @MiSAKACHi @fernandobandeira

Fun fact: As it is now 1st February this problem has now disappeared 😉

php -r "echo DateTime::createFromFormat('Y-m', '2016-02')->format('Y-m') . \"\n\";"

Roll your clock back to yesterday and run the above again.. hehe.

@SiNONiMiTY
Copy link
Author

@fernandobandeira

The rule return ['testDate' => ['bail', required, 'date_format:!Y-m']]; yields the error

The testDate does not match the format !Y-m.

@arjasco

Yes, it so weird 👍

...

I temporarily used the validation rule 'date' & limited the max length to 7 to resolve the issue.
Everyone, thank you for taking your time in looking into this... Here, get your medal 🥇 Cheers!

@arjasco
Copy link
Contributor

arjasco commented Feb 2, 2017

@MiSAKACHi Ahhh. Yes the validation will still fail because the same format is being used for the next comparison. hmm.

Glad you found a work around though. Still not correct behaviour, couldn't find anything on the PHP bugs site either.

@SiNONiMiTY
Copy link
Author

SiNONiMiTY commented Feb 2, 2017

@themsaid

Can we re-open this issue? Since this one has no clear solution and the proposed workaround isn't working when I tested it.

@arjasco
Copy link
Contributor

arjasco commented Feb 2, 2017

@MiSAKACHi I've left a comment in my pull request to Taylor. My commit does fix the problem using the '!' internally, it's when passing it through the validation instance with date_format: that this will still not work.

Very minor change but because of the inconsistency with it not working depending on what the current month it would be nice to be fixed. It means one day as far as you're concerned your production app works, next day it's potentially not validating correctly anymore. Anyone unaware of the date time issue could spend ages trying to figure out this problem because their code is correct..

@pjebs
Copy link

pjebs commented Mar 4, 2017

I'm doing it client side for now:

    		// First check for the pattern
    		var regex_date = /^\d{4}\-\d{1,2}\-\d{1,2}$/;

		    if(!regex_date.test(value))
		    {
		        this.setState({valid: false});
				return;
		    }

		    // Parse the date parts to integers
		    var parts   = value.split("-");
		    var day     = parseInt(parts[2], 10);
		    var month   = parseInt(parts[1], 10);
		    var year    = parseInt(parts[0], 10);

		    // Check the ranges of month and year
		    if(year < 1000 || year > 3000 || month == 0 || month > 12)
		    {
		        this.setState({valid: false});
				return;
		    }

   			var monthLength = [ 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 ];

		    // Adjust for leap years
		    if(year % 400 == 0 || (year % 100 != 0 && year % 4 == 0))
		    {
		        monthLength[1] = 29;
		    }

    		// Check the range of the day
    		if (day > 0 && day <= monthLength[month - 1]) {
    			//Tests passed
				this.setState({valid: true});
				this.props.onUpdate(this.props.type, value);
				return;
    		} else {
    			this.setState({valid: false});
				return;
    		}

@ghost
Copy link

ghost commented May 30, 2017

I have the same problem with the date_format February

@decadence
Copy link
Contributor

decadence commented May 30, 2017

Why is this closed if it's still the issue? @themsaid

Issue with DateTime::createFromFormat that sets missing date components to current values.
"!" workaround doesn't work and I will explain: when you have "!" in format in indeed create date with first day and that's correct.
dd(DateTime::createFromFormat("Y-m", "2017-02")); // 2017-03-02 10:13:28.000000, date overflow because today is 30 day.
dd(DateTime::createFromFormat("!Y-m", "2017-02")); // 2017-02-01 00:00:00.000000, correct.

But validateDateFormat check $date->format($parameters[0]) == $value fails because we have "!" in format.
And therefore "2017-02" != "!2017-02"

dd(validator([
       "date" => "2017-02",
], [
   "date" => "date_format:!Y-m"
])->passes()); // false

@themsaid themsaid reopened this May 30, 2017
@arjasco
Copy link
Contributor

arjasco commented May 31, 2017

@decadence My quick one liner here (#17693) as reference above did fix this, but didn't progress any further.

Reseting the day to be the 1st if no day is present is another fix.

Discussion is also taking place on the Carbon library.

@reinink
Copy link
Contributor

reinink commented May 31, 2017

Just ran into this myself when trying to use date_format:Y-m as a validation rule. 😭

For those interested, I ended up just using the date validation rule instead.

@mpmurph
Copy link

mpmurph commented Jul 31, 2017

I seem to be having a similar problem. Today is July 31, 2017 and am using date_format:m to validate the month dropdown of a birthday input. At this moment, if I enter February, April, June, September or November, validation fails and returns: "Invalid month entered for your birthday." Any entry for the months that have 31 days - January, March, May, July, August, October and December - validates with no problem. Likely will create a custom validation rule as a workaround for the time being.

@hopeseekr
Copy link

It still fails when you have date_format:m. Hit my team today.

@decadence
Copy link
Contributor

@hopeseekr raise another issue because Laravel team don't look on closed ones.

@themsaid
Copy link
Member

@decadence a PR is better because we're basically trying to trick PHP into this, opening a new issue won't help, but if you have a solution that's much better.

@hopeseekr if I were you I'd just do between:1,12

@Valkyrie00
Copy link

Valkyrie00 commented Oct 31, 2017

Same problem here, 'dob_m' => 'required|date_format:"m"' goes in error with these months:
02, 04, 06, 09, 11

UPDATE:
My solutions are:
1 - Change your validation rules like this:
'your_field' => 'date_format:"!m"'
2 - Or remove date_format and add:
'your_field' => 'between:1,12'

I THINK that one solution is:
In:

/Illuminate/Validation/Concerns/ValidatesAttributes.php:361

Change 361 line from:

$format = $parameters[0] == 'Y-m' ? '!Y-m' : $parameters[0];

To:

$format = $parameters[0];
if (in_array($parameters[0], array("Y", "m", "d", "Y-m"))) {
    $format = "!".$format;
}

@SiNONiMiTY
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants