From d4d5adf88bd2bcf59d590d8085c79c1af142910b Mon Sep 17 00:00:00 2001 From: VayuDev <26022837+VayuDev@users.noreply.github.com> Date: Fri, 5 Jun 2020 14:57:36 +0200 Subject: [PATCH] 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 Co-authored-by: antao --- lib/inc/drogon/Cookie.h | 5 +++-- lib/inc/drogon/utils/Utilities.h | 4 ++++ lib/src/Cookie.cc | 4 +++- lib/src/Utilities.cc | 24 +++++++++++++++++++--- unittest/CMakeLists.txt | 4 +++- unittest/HttpDateUnittest.cpp | 35 ++++++++++++++++++++++++++++++++ 6 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 unittest/HttpDateUnittest.cpp diff --git a/lib/inc/drogon/Cookie.h b/lib/inc/drogon/Cookie.h index ffe24a5e..6f725564 100644 --- a/lib/inc/drogon/Cookie.h +++ b/lib/inc/drogon/Cookie.h @@ -13,8 +13,9 @@ */ #pragma once -#include #include +#include +#include namespace drogon { @@ -235,7 +236,7 @@ class Cookie } private: - trantor::Date expiresDate_; + trantor::Date expiresDate_{(std::numeric_limits::max)()}; bool httpOnly_{true}; bool secure_{false}; std::string domain_; diff --git a/lib/inc/drogon/utils/Utilities.h b/lib/inc/drogon/utils/Utilities.h index 608df6ce..ec1747d2 100644 --- a/lib/inc/drogon/utils/Utilities.h +++ b/lib/inc/drogon/utils/Utilities.h @@ -21,6 +21,7 @@ #include #include #include +#include #ifdef _WIN32 #include 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::max()) upon failure. + */ trantor::Date getHttpDate(const std::string &httpFullDateString); /// Get a formatted string diff --git a/lib/src/Cookie.cc b/lib/src/Cookie.cc index 7a58d7ad..cf771140 100644 --- a/lib/src/Cookie.cc +++ b/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::max)() && + expiresDate_.microSecondsSinceEpoch() >= 0) { ret.append("Expires= ") .append(utils::getHttpFullDate(expiresDate_)) diff --git a/lib/src/Utilities.cc b/lib/src/Utilities.cc index 07013825..18696358 100644 --- a/lib/src/Utilities.cc +++ b/lib/src/Utilities.cc @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -935,10 +936,27 @@ char *getHttpFullDate(const trantor::Date &date) } trantor::Date getHttpDate(const std::string &httpFullDateString) { + static const std::array 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::max)()); } std::string formattedString(const char *format, ...) { diff --git a/unittest/CMakeLists.txt b/unittest/CMakeLists.txt index ef4d8fda..55adb082 100644 --- a/unittest/CMakeLists.txt +++ b/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() diff --git a/unittest/HttpDateUnittest.cpp b/unittest/HttpDateUnittest.cpp new file mode 100644 index 00000000..02a383f1 --- /dev/null +++ b/unittest/HttpDateUnittest.cpp @@ -0,0 +1,35 @@ +#include +#include +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::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(); +} \ No newline at end of file