Fail time parsing if input is insufficient to supply all fields (#2523)

Co-authored-by: Matt Stephanson <[email protected]>
microsoft · Mar 28, 2022 · c42ad5c · c42ad5c
1 parent c302187
commit c42ad5c
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 34 deletions.
diff --git a/stl/inc/chrono b/stl/inc/chrono
@@ -3947,8 +3947,11 @@ namespace chrono {
             const _Ctype& _Ctype_fac{_STD use_facet<_Ctype>(_Iosbase.getloc())};
             constexpr _InIt _Last{};
 
-            while (*_Fmt != '\0' && _First != _Last && _State == ios_base::goodbit) {
-                if (*_Fmt == '%') {
+            while (*_Fmt != '\0' && (_State & ~ios_base::eofbit) == ios_base::goodbit) {
+                if (_First == _Last) {
+                    _State |= ios_base::failbit | ios_base::eofbit;
+                    break;
+                } else if (*_Fmt == '%') {
                     _First = _Parse_time_field(_First, _Iosbase, _State, *++_Fmt, '\0', 0, _Subsecond_precision);
                 } else if (_Ctype_fac.narrow(*_First++) != *_Fmt) {
                     _State |= ios_base::failbit;
@@ -4181,7 +4184,7 @@ namespace chrono {
 
             if (_Ok) {
                 _TRY_IO_BEGIN
-                for (; _FmtFirst != _FmtLast && _State == ios_base::goodbit; ++_FmtFirst) {
+                for (; _FmtFirst != _FmtLast && (_State & ~ios_base::eofbit) == ios_base::goodbit; ++_FmtFirst) {
                     if (_First == _Last) {
                         // EOF is not an error if the remaining flags can match zero characters.
                         for (; _FmtFirst != _FmtLast; ++_FmtFirst) {

diff --git a/tests/std/tests/P0355R7_calendars_and_time_zones_io/test.cpp b/tests/std/tests/P0355R7_calendars_and_time_zones_io/test.cpp
@@ -127,7 +127,7 @@ void test_duration_output() {
 
 
 template <class CharT, class CStringOrStdString, class Parsable>
-void test_parse(const CharT* str, const CStringOrStdString& fmt, Parsable& p,
+ios_base::iostate parse_state(const CharT* str, const CStringOrStdString& fmt, Parsable& p,
     type_identity_t<basic_string<CharT>*> abbrev = nullptr, minutes* offset = nullptr) {
     p = Parsable{};
     if (abbrev) {
@@ -157,41 +157,19 @@ void test_parse(const CharT* str, const CStringOrStdString& fmt, Parsable& p,
         }
     }
 
-    assert(sstr);
+    return sstr.rdstate();
 }
 
 template <class CharT, class CStringOrStdString, class Parsable>
-void fail_parse(const CharT* str, const CStringOrStdString& fmt, Parsable& p,
+void test_parse(const CharT* str, const CStringOrStdString& fmt, Parsable& p,
     type_identity_t<basic_string<CharT>*> abbrev = nullptr, minutes* offset = nullptr) {
-    p = Parsable{};
-    if (abbrev) {
-        if constexpr (is_same_v<CharT, char>) {
-            *abbrev = "!";
-        } else {
-            *abbrev = L"!";
-        }
-    }
-
-    if (offset) {
-        *offset = minutes::min();
-    }
-
-    basic_stringstream<CharT> sstr{str};
-    if (abbrev) {
-        if (offset) {
-            sstr >> parse(fmt, p, *abbrev, *offset);
-        } else {
-            sstr >> parse(fmt, p, *abbrev);
-        }
-    } else {
-        if (offset) {
-            sstr >> parse(fmt, p, *offset);
-        } else {
-            sstr >> parse(fmt, p);
-        }
-    }
+    assert((parse_state(str, fmt, p, abbrev, offset) & ~ios_base::eofbit) == ios_base::goodbit);
+}
 
-    assert(!sstr);
+template <class CharT, class CStringOrStdString, class Parsable>
+void fail_parse(const CharT* str, const CStringOrStdString& fmt, Parsable& p,
+    type_identity_t<basic_string<CharT>*> abbrev = nullptr, minutes* offset = nullptr) {
+    assert((parse_state(str, fmt, p, abbrev, offset) & ~ios_base::eofbit) != ios_base::goodbit);
 }
 
 template <class TimeType, class IntType = int>
@@ -878,6 +856,32 @@ void parse_other_week_date() {
     assert(ymd == 2022y / January / 1d);
 }
 
+void parse_incomplete() {
+    // Parsing should fail if the input is insufficient to supply all fields of the format string, even if the input is
+    // sufficient to supply all fields of the parsable.
+    // Check both explicit and shorthand format strings, since the code path is different.
+    year_month ym;
+    assert(parse_state("2021-01", "%Y-%m-%d", ym) == (ios_base::eofbit | ios_base::failbit));
+    assert(parse_state("2022-02", "%F", ym) == (ios_base::eofbit | ios_base::failbit));
+    assert(parse_state("2021-", "%Y-%m-%d", ym) == (ios_base::eofbit | ios_base::failbit));
+    assert(parse_state("2022-", "%F", ym) == (ios_base::eofbit | ios_base::failbit));
+
+    seconds time;
+    fail_parse("01:59", "%H:%M:%S", time);
+    fail_parse("03:23", "%T", time);
+    fail_parse("04", "%R", time);
+
+    // Check for parsing of whitespace fields after other fields.  More whitespace tests below.
+    test_parse("15:19", "%H:%M%t", time);
+    test_parse("15:19", "%R%t", time);
+    fail_parse("15:19", "%H:%M%n", time);
+    fail_parse("15:19", "%R%n", time);
+
+    // However, it is OK to omit seconds from the format when parsing a duration to seconds precision.
+    test_parse("05:24", "%H:%M", time);
+    test_parse("06:25", "%R", time);
+}
+
 void parse_whitespace() {
     seconds time;
     fail_parse("ab", "a%nb", time);
@@ -1213,6 +1217,7 @@ void test_parse() {
     parse_calendar_types_basic();
     parse_iso_week_date();
     parse_other_week_date();
+    parse_incomplete();
     parse_whitespace();
     parse_timepoints();
     test_io_manipulator<char, const char*>();