From 717c39f2ef26cf81909690cefb5fa115e168ca1d Mon Sep 17 00:00:00 2001
From: Matthieu Gautier <mgautier@kymeria.fr>
Date: Tue, 26 Apr 2022 14:41:54 +0200
Subject: [PATCH] Better ExtractFromString

- Throw a exception if we cannot extract from string.
  (We throw the same exception as `std::sto*`)
- Add a specialization to extract string from string
- Add some unit test
---
 src/tools/stringTools.cpp |  5 +++++
 src/tools/stringTools.h   |  7 +++++++
 test/stringTools.cpp      | 23 ++++++++++++++++++++++-
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/src/tools/stringTools.cpp b/src/tools/stringTools.cpp
index 6d6e1d39e..e7cb851ec 100644
--- a/src/tools/stringTools.cpp
+++ b/src/tools/stringTools.cpp
@@ -405,3 +405,8 @@ std::vector<std::string> kiwix::getTitleVariants(const std::string& title) {
   variants.push_back(kiwix::toTitle(title));
   return variants;
 }
+
+template<>
+std::string kiwix::extractFromString(const std::string& str) {
+  return str;
+}
diff --git a/src/tools/stringTools.h b/src/tools/stringTools.h
index 9b5cf3622..337548578 100644
--- a/src/tools/stringTools.h
+++ b/src/tools/stringTools.h
@@ -25,6 +25,7 @@
 #include <string>
 #include <vector>
 #include <sstream>
+#include <stdexcept>
 
 namespace kiwix
 {
@@ -65,9 +66,15 @@ T extractFromString(const std::string& str) {
     std::istringstream iss(str);
     T ret;
     iss >> ret;
+    if(iss.fail() || !iss.eof()) {
+        throw std::invalid_argument("no conversion");
+    }
     return ret;
 }
 
+template<>
+std::string extractFromString(const std::string& str);
+
 bool startsWith(const std::string& base, const std::string& start);
 
 std::vector<std::string> getTitleVariants(const std::string& title);
diff --git a/test/stringTools.cpp b/test/stringTools.cpp
index af9c25bdf..d91670933 100644
--- a/test/stringTools.cpp
+++ b/test/stringTools.cpp
@@ -18,11 +18,11 @@
  */
 
 #include "gtest/gtest.h"
+#include "../src/tools/stringTools.h"
 #include <string>
 #include <vector>
 
 namespace kiwix {
-std::string join(const std::vector<std::string>& list, const std::string& sep);
 std::vector<std::string> split(const std::string&  base, const std::string& sep, bool trimEmpty, bool keepDelim);
 };
 
@@ -58,4 +58,25 @@ TEST(stringTools, split)
   ASSERT_EQ(split(";a;b=;c=d;", ";=", false, true), list6);
 }
 
+TEST(stringTools, extractFromString)
+{
+  ASSERT_EQ(extractFromString<int>("55"), 55);
+  ASSERT_EQ(extractFromString<int>("-55"), -55);
+  ASSERT_EQ(extractFromString<float>("-55.0"), -55.0);
+  ASSERT_EQ(extractFromString<bool>("1"), true);
+  ASSERT_EQ(extractFromString<bool>("0"), false);
+  ASSERT_EQ(extractFromString<std::string>("55"), "55");
+  ASSERT_EQ(extractFromString<std::string>("foo"), "foo");
+  ASSERT_EQ(extractFromString<std::string>("foo bar"), "foo bar");
+
+// While spec says that >> operator should set the value to std::numeric_limits<uint>::max()
+// and set the failbit of the stream (and so detect the error), the gcc implementation (?)
+// set the value to (std::numeric_limits<uint>::max()-55) and doesn't set the failbit.
+//  ASSERT_THROW(extractFromString<uint>("-55"), std::invalid_argument);
+
+  ASSERT_THROW(extractFromString<int>("-55.0"), std::invalid_argument);
+  ASSERT_THROW(extractFromString<int>("55 foo"), std::invalid_argument);
+  ASSERT_THROW(extractFromString<float>("3.14.5"), std::invalid_argument);
+}
+
 };