Fix #340: Parse ASN1_TIME to struct tm #3378

tmshort · 2017-05-03T21:01:16Z

This works with ASN1_UTCTIME and ASN1_GENERALIZED_TIME

Checklist

documentation is added or updated
tests are added or updated

tmshort · 2017-05-03T21:01:41Z

Fixes #340

mattcaswell · 2017-05-04T15:27:53Z

crypto/asn1/a_time.c

@@ -130,7 +130,7 @@ int ASN1_TIME_set_string(ASN1_TIME *s, const char *str)
    return 1;
 }

-static int asn1_time_to_tm(struct tm *tm, const ASN1_TIME *t)
+int ASN1_TIME_to_tm(struct tm *tm, const ASN1_TIME *t)


I'm wondering what order the params should be in. Recently we have tended to always put the primary object first with the function name also starting with the primary object name. But here the ASN1_TIME type comes second.

i agree it should be flipped.

Yup; I just un-static-fied the exiting function, but I agree, the arguments should be reversed.

I will note that ASN1_TIME_print() is an anomaly (not suggesting this changes the need to reverse the arguments).

mattcaswell · 2017-05-04T15:28:15Z

doc/man3/ASN1_TIME_set.pod

@@ -13,6 +13,7 @@ ASN1_TIME_print, ASN1_TIME_diff - ASN.1 Time functions
 int ASN1_TIME_set_string(ASN1_TIME *s, const char *str);
 int ASN1_TIME_check(const ASN1_TIME *t);
 int ASN1_TIME_print(BIO *b, const ASN1_TIME *s);
+ int ASN1_TIME_to_tm(struct tm *tm, const ASN1_TIME *t);


Needs to be added to the NAME section too.

find-doc-nits is your friend. :)

mattcaswell · 2017-05-04T15:28:46Z

doc/man3/ASN1_TIME_set.pod

@@ -123,6 +129,9 @@ otherwise.
 ASN1_TIME_print() returns 1 if the time is successfully printed out and 0 if
 an error occurred (I/O error or invalid time format).

+ASN1_TIME_to_tm() returns 1 if the time is successfully parsed and 0 if an
+error occured (invalid time format).
+
 ASN1_TIME_diff() returns 1 for success and 0 for failure. It can fail if the
 pass ASN1_TIME structure has invalid syntax for example.



Please can we add a HISTORY section and update the COPYRIGHT date.

richsalz · 2017-05-04T20:26:22Z

crypto/asn1/a_time.c

@@ -130,7 +130,7 @@ int ASN1_TIME_set_string(ASN1_TIME *s, const char *str)
    return 1;
 }

-static int asn1_time_to_tm(struct tm *tm, const ASN1_TIME *t)
+int ASN1_TIME_to_tm(struct tm *tm, const ASN1_TIME *t)


i agree it should be flipped.

richsalz · 2017-05-04T20:26:42Z

crypto/asn1/a_time.c

@@ -152,9 +152,9 @@ int ASN1_TIME_diff(int *pday, int *psec,
                   const ASN1_TIME *from, const ASN1_TIME *to)
 {
    struct tm tm_from, tm_to;
-    if (!asn1_time_to_tm(&tm_from, from))
+    if (!ASN1_TIME_to_tm(&tm_from, from))


since you're here, blank line before the declaration.

richsalz · 2017-05-04T20:26:58Z

doc/man3/ASN1_TIME_set.pod

@@ -13,6 +13,7 @@ ASN1_TIME_print, ASN1_TIME_diff - ASN.1 Time functions
 int ASN1_TIME_set_string(ASN1_TIME *s, const char *str);
 int ASN1_TIME_check(const ASN1_TIME *t);
 int ASN1_TIME_print(BIO *b, const ASN1_TIME *s);
+ int ASN1_TIME_to_tm(struct tm *tm, const ASN1_TIME *t);


find-doc-nits is your friend. :)

richsalz · 2017-05-04T20:27:30Z

doc/man3/ASN1_TIME_set.pod

@@ -42,6 +43,11 @@ format. It will be of the format MMM DD HH:MM:SS YYYY [GMT], for example
 structure has invalid format it prints out "Bad time value" and returns
 an error.

+ASN1_TIME_to_tm() converts the time B<s> to the standard B<tm> structure.


prose here says as but the prototype above says t

Changed ASN1_TIME_to_tm() everywhere to s to make it consistent with other ASN1_TIME functions.

tmshort · 2017-05-08T14:36:48Z

Fixed everything, I believe.

tmshort · 2017-05-22T15:19:36Z

rebase ping

mattcaswell

Approved subject to the minor nit being fixed.

mattcaswell · 2017-05-22T15:21:26Z

doc/man3/ASN1_TIME_set.pod

@@ -42,6 +43,11 @@ format. It will be of the format MMM DD HH:MM:SS YYYY [GMT], for example
 structure has invalid format it prints out "Bad time value" and returns
 an error.

+ASN1_TIME_to_tm() converts the time B<s> to the standard B<tm> structure.
+If B<s> is NULL, the the current time is converted. The output time is GMT.


nit: then the

tmshort · 2017-05-22T15:36:45Z

Committed a doc tweak, which can be squashed.

tmshort · 2017-06-06T20:33:08Z

ping?

mattcaswell · 2017-06-07T08:15:36Z

@richsalz this needs your plus one

richsalz

should we memset the 'struct tm' since there might be fields we don't know about? but approved.

tmshort · 2017-06-07T13:24:29Z

should we memset the 'struct tm' since there might be fields we don't know about? but approved.

The documentation explicitly states what fields are filled in, but yeah, I think it's a good idea.

richsalz

@mattcaswell you can just merge if you approve of this.

This works with ASN1_UTCTIME and ASN1_GENERALIZED_TIME

tmshort · 2017-06-08T01:04:21Z

Fixed the merge conflict...

mattcaswell · 2017-06-08T12:24:16Z

Pushed. Thanks.

This works with ASN1_UTCTIME and ASN1_GENERALIZED_TIME Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from #3378)

mattcaswell requested changes May 4, 2017

View reviewed changes

richsalz suggested changes May 4, 2017

View reviewed changes

tmshort force-pushed the master-get-tm branch from fcd6399 to 14ce28e Compare May 8, 2017 13:37

tmshort force-pushed the master-get-tm branch from 14ce28e to ce18c91 Compare May 22, 2017 15:03

mattcaswell approved these changes May 22, 2017

View reviewed changes

tmshort force-pushed the master-get-tm branch from 6aed317 to 7e72c8d Compare June 6, 2017 20:33

richsalz approved these changes Jun 7, 2017

View reviewed changes

Fix openssl#340: Parse ASN1_TIME to struct tm

04d789c

This works with ASN1_UTCTIME and ASN1_GENERALIZED_TIME

tmshort force-pushed the master-get-tm branch from 7d65d15 to 04d789c Compare June 8, 2017 01:04

InfoHunter mentioned this pull request Jun 8, 2017

RFC5280 strict time string check #3566

Closed

2 tasks

mattcaswell approved these changes Jun 8, 2017

View reviewed changes

mattcaswell closed this Jun 8, 2017

levitte pushed a commit that referenced this pull request Jun 8, 2017

Fix #340: Parse ASN1_TIME to struct tm

1c036c6

This works with ASN1_UTCTIME and ASN1_GENERALIZED_TIME Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from #3378)

tmshort deleted the master-get-tm branch June 8, 2017 12:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #340: Parse ASN1_TIME to struct tm #3378

Fix #340: Parse ASN1_TIME to struct tm #3378

tmshort commented May 3, 2017 •

edited

tmshort commented May 3, 2017

mattcaswell May 4, 2017

richsalz May 4, 2017

tmshort May 8, 2017

tmshort May 8, 2017

mattcaswell May 4, 2017

richsalz May 4, 2017

mattcaswell May 4, 2017

richsalz May 4, 2017

richsalz May 4, 2017

richsalz May 4, 2017

richsalz May 4, 2017

tmshort May 8, 2017

tmshort commented May 8, 2017

tmshort commented May 22, 2017

mattcaswell left a comment

mattcaswell May 22, 2017

tmshort commented May 22, 2017

tmshort commented Jun 6, 2017

mattcaswell commented Jun 7, 2017

richsalz left a comment

tmshort commented Jun 7, 2017

richsalz left a comment

tmshort commented Jun 8, 2017

mattcaswell commented Jun 8, 2017

Fix #340: Parse ASN1_TIME to struct tm #3378

Fix #340: Parse ASN1_TIME to struct tm #3378

Conversation

tmshort commented May 3, 2017 • edited

Checklist

tmshort commented May 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmshort commented May 8, 2017

tmshort commented May 22, 2017

mattcaswell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmshort commented May 22, 2017

tmshort commented Jun 6, 2017

mattcaswell commented Jun 7, 2017

richsalz left a comment

Choose a reason for hiding this comment

tmshort commented Jun 7, 2017

richsalz left a comment

Choose a reason for hiding this comment

tmshort commented Jun 8, 2017

mattcaswell commented Jun 8, 2017

tmshort commented May 3, 2017 •

edited