Added additional formats for getHttpDate function and fixed undefined behavior upon error (#453) (#456)

This patch adds support for the RFC 850 and asctime format. If an error
occurs, we now return a date with the epoch value of -1 and warn instead of
triggering undefined behavior. This is checked by a new set of tests.

Co-authored-by: VayuDev <vayudev@protonmail.com>
Co-authored-by: antao <antao2002@gmail.com>

lib/inc/drogon/Cookie.h

@@ -13,8 +13,9 @@
  */
 #pragma once
 
-#include <string>
 #include <trantor/utils/Date.h>
+#include <string>
+#include <limits>
 
 namespace drogon
 {
@@ -235,7 +236,7 @@ class Cookie
     }
 
   private:
-    trantor::Date expiresDate_;
+    trantor::Date expiresDate_{(std::numeric_limits<int64_t>::max)()};
     bool httpOnly_{true};
     bool secure_{false};
     std::string domain_;

lib/inc/drogon/utils/Utilities.h

@@ -21,6 +21,7 @@
 #include <string>
 #include <vector>
 #include <set>
+#include <limits>
 #ifdef _WIN32
 #include <time.h>
 char *strptime(const char *s, const char *f, struct tm *tm);
@@ -125,6 +126,9 @@ std::string brotliDecompress(const char *data, const size_t ndata);
 char *getHttpFullDate(const trantor::Date &date = trantor::Date::now());
 
 /// Get the trantor::Date object according to the http full date string
+/**
+ * Returns trantor::Date(std::numeric_limits<int64_t>::max()) upon failure.
+ */
 trantor::Date getHttpDate(const std::string &httpFullDateString);
 
 /// Get a formatted string

lib/src/Cookie.cc

@@ -19,7 +19,9 @@ std::string Cookie::cookieString() const
 {
     std::string ret = "Set-Cookie: ";
     ret.append(key_).append("= ").append(value_).append("; ");
-    if (expiresDate_.microSecondsSinceEpoch() > 0)
+    if (expiresDate_.microSecondsSinceEpoch() !=
+            (std::numeric_limits<int64_t>::max)() &&
+        expiresDate_.microSecondsSinceEpoch() >= 0)
     {
         ret.append("Expires= ")
             .append(utils::getHttpFullDate(expiresDate_))

lib/src/Utilities.cc

@@ -33,6 +33,7 @@
 #include <string>
 #include <thread>
 #include <algorithm>
+#include <array>
 #include <cctype>
 #include <cstdlib>
 #include <stdio.h>
@@ -935,10 +936,27 @@ char *getHttpFullDate(const trantor::Date &date)
 }
 trantor::Date getHttpDate(const std::string &httpFullDateString)
 {
+    static const std::array<const char *, 4> formats = {
+        // RFC822 (default)
+        "%a, %d %b %Y %H:%M:%S",
+        // RFC 850 (deprecated)
+        "%a, %d-%b-%y %H:%M:%S",
+        // ansi asctime format
+        "%a %b %d %H:%M:%S %Y",
+        // weird RFC 850-hybrid thing that reddit uses
+        "%a, %d-%b-%Y %H:%M:%S",
+    };
     struct tm tmptm;
-    strptime(httpFullDateString.c_str(), "%a, %d %b %Y %H:%M:%S", &tmptm);
-    auto epoch = timegm(&tmptm);
-    return trantor::Date(epoch * MICRO_SECONDS_PRE_SEC);
+    for (const char *format : formats)
+    {
+        if (strptime(httpFullDateString.c_str(), format, &tmptm) != NULL)
+        {
+            auto epoch = timegm(&tmptm);
+            return trantor::Date(epoch * MICRO_SECONDS_PRE_SEC);
+        }
+    }
+    LOG_WARN << "invalid datetime format: '" << httpFullDateString << "'";
+    return trantor::Date((std::numeric_limits<int64_t>::max)());
 }
 std::string formattedString(const char *format, ...)
 {

unittest/CMakeLists.txt

@@ -6,6 +6,7 @@ add_executable(sha1_unittest SHA1Unittest.cpp ../lib/src/ssl_funcs/Sha1.cc)
 add_executable(ostringstream_unittest OStringStreamUnitttest.cpp)
 add_executable(base64_unittest Base64Unittest.cpp)
 add_executable(pubsubservice_unittest PubSubServiceUnittest.cpp)
+add_executable(httpdate_unittest HttpDateUnittest.cpp)
 if(Brotli_FOUND)
   add_executable(brotli_unittest BrotliUnittest.cpp)
 endif()
@@ -18,7 +19,8 @@ set(UNITTEST_TARGETS
     sha1_unittest
     ostringstream_unittest
     base64_unittest
-    pubsubservice_unittest)
+    pubsubservice_unittest
+    httpdate_unittest)
 if(Brotli_FOUND)
   set(UNITTEST_TARGETS ${UNITTEST_TARGETS} brotli_unittest)
 endif()

unittest/HttpDateUnittest.cpp

@@ -0,0 +1,35 @@
+#include <drogon/utils/Utilities.h>
+#include <gtest/gtest.h>
+using namespace drogon;
+
+TEST(HttpDate, rfc850)
+{
+    auto date = utils::getHttpDate("Fri, 05-Jun-20 09:19:38 GMT");
+    EXPECT_EQ(date.microSecondsSinceEpoch() / MICRO_SECONDS_PRE_SEC, 1591348778);
+}
+
+TEST(HttpDate, redditFormat)
+{
+    auto date = utils::getHttpDate("Fri, 05-Jun-2020 09:19:38 GMT");
+    EXPECT_EQ(date.microSecondsSinceEpoch() / MICRO_SECONDS_PRE_SEC, 1591348778);
+}
+
+TEST(HttpDate, invalidFormat)
+{
+    auto date = utils::getHttpDate("Fri, this format is invalid");
+    EXPECT_EQ(date.microSecondsSinceEpoch(), (std::numeric_limits<int64_t>::max)());
+}
+
+TEST(HttpDate, asctimeFormat)
+{
+    auto epoch = time(nullptr);
+    auto str = asctime(gmtime(&epoch));
+    auto date = utils::getHttpDate(str);
+    EXPECT_EQ(date.microSecondsSinceEpoch() / MICRO_SECONDS_PRE_SEC, epoch);
+}
+
+int main(int argc, char **argv)
+{
+    testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}