Skip to content

Conversation

@kawamuray
Copy link

Time::Piece->strptime() croaks if first argument STRING isn't matching second argument FORMAT. For example, following code will croaked:

Time::Piece->strptime("2014-01-01", '%Y/%m/%d'); #=> Error parsing time at ...

But there are cases that invalid date string is ignored and Time::Piece object is returned regardless to date string.

Case1: Not enough date string

Date string and format are compared from ahead and trailing format specifiers are ignored if date string reached end of string while parsing.

print Time::Piece->strptime("2014", '%Y-%m'), "\n";
#=> Wed Jan  1 00:00:00 2014
# What about month!?

Case2: Empty date string

Empty date string is ignored even if one or more format has been specified.

print Time::Piece->strptime("", '%Y-%m'), "\n";
#=> Thu Jan  1 00:00:00 1970

These behavior is differ from unix strptime(3). Following code can tell us what kind of date string should be treated as invalid:

#define _XOPEN_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define TM_FORMAT "%Y%m"
static const char *const examples[] = {
    "201401",  /* Valid */
    "foo",     /* Invalid */
    "2013",    /* Not enought data */
    "",        /* Empty string */
    "201301 ", /* Trailing whitespaces(allowed) */
    NULL
};

int main(void)
{
    const char *const *ptr = examples;
    struct tm tm;
    int valid;

    while (*ptr != NULL) {
        valid = strptime(*ptr, TM_FORMAT, &tm) != NULL;
        printf("'%s' is %s\n", *ptr, (valid ? "valid" : "invalid"));
        ptr++;
    }

    return 0;
}
gcc -Wall strptime-checkvalid.c -o strptime-checkvalid
./strptime-checkvalid
'201401' is valid
'foo' is invalid
'2013' is invalid
'' is invalid
'201301 ' is valid

We expect that Time::Piece->strptime() is respecting unix strptime(3) because you did wrote "see strptime man page." in the POD.
We also expect that every invalid format date string will be notified via croak.
So my opinion is that Time::Piece->strptime() should check if date string has completely satisfied format specifier and croak if not.

Regards.

This patch makes strptime() to be croak "Error parsing time" if:
- STRING is empty string but format is not (e.g, format: "%y" string: "")
- STRING is not enough to match with format (e.g, format: "%Y%m" string: "2014")
@scottchiefbaker
Copy link

I agree with the logic here... code looks decent to me, and includes unit tests.

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

Successfully merging this pull request may close these issues.

2 participants