Skip to content
Permalink
Browse files
There's no guard against unparsed characters at the end of the date s… 
…tring, any extra characters are just ignored

Fixes #7244 - Datepicker: parseDate() does not throw an exception for long years
  • Loading branch information
@jzaefferer
jzaefferer committed May 18, 2011
1 parent ba6dd5c commit 92b0f6702a9408f4bd7d71ccca7e0e851d0efc6b
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 0 deletions.
9 tests/unit/datepicker/datepicker_tickets.js
View file
@@ -29,4 +29,13 @@ test('Ticket 6827: formatDate day of year calculation is wrong during day lights
equals(time, "089");
});

test('Ticket #7244: date parser does not fail when too many numbers are passed into the date function', function() {
expect(1);
try{
var date = $.datepicker.parseDate('dd/mm/yy', '18/04/19881');
}catch(e){
ok("invalid date detected");
}
});

})(jQuery);
3 ui/jquery.ui.datepicker.js
View file
@@ -1082,6 +1082,9 @@ $.extend(Datepicker.prototype, {
checkLiteral();
}
}
if (iValue < value.length){
throw "Extra/unparsed characters found in date: " + value.substring(iValue);
}
if (year == -1)
year = new Date().getFullYear();
else if (year < 100)

3 comments on commit 92b0f67

@pgraham
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit breaks an existing timepicker extension to the datepicker control (https://github.com/trentrichardson/jQuery-Timepicker-Addon) which appends additional text to the end of the datepicker's input field. This can be avoided while still passing the test by ensuring that any additional text is separated from the date portion of the string by some white space:

if (iValue < value.length){
    var extra = value.substr(iValue);
    if (!/^\s+/.test(extra)) {
        throw "Extra/unparsed characters found in date: " + value.substring(iValue);
    } 
}

@gnarf
Copy link
Member

@gnarf gnarf commented on 92b0f67 Jun 27, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, just because datepicker doesn't currently conform to http://wiki.jqueryui.com/w/page/12137737/Coding-standards doesn't mean that patches against it shouldn't be adhering to the code standards.

@jzaefferer
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pgraham: I've reopened the ticket - could you provide a patch as a pull request, with an updated test? You should also reuse the extra variable instead of calling substring two times.

@gnarf37: We're going to rewrite datepicker from scratch.

Please sign in to comment.