From f36bea38013733931d5d271c282f98cefb0814f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= <piponazo@gmail.com>
Date: Tue, 28 Nov 2017 19:11:31 +0100
Subject: [PATCH] exiv2::getEnv throws std::out_of_range on unexpected EnVar

This change define explicitly the behavior that exiv2::getEnv should have
on response to unexpected inputs.

There are some other minor changes:

- Use _putenv_s for the unit tests on Windows
- Add todo comment
- Remove deprecated note about freeing memory
---
 include/exiv2/futils.hpp  |  5 ++---
 src/futils.cpp            | 11 +++++++++--
 unitTests/test_futils.cpp | 25 ++++++++++++++++++++-----
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/include/exiv2/futils.hpp b/include/exiv2/futils.hpp
index 2bd71239..957e3a27 100644
--- a/include/exiv2/futils.hpp
+++ b/include/exiv2/futils.hpp
@@ -56,6 +56,7 @@ namespace Exiv2 {
       @brief  Return the value of environmental variable.
       @param  var The name of environmental variable.
       @return the value of environmental variable. If it's empty, the default value is returned.
+      @throws std::out_of_range when an unexpected EnVar is given as input.
      */
     EXIV2API std::string getEnv(EnVar var);
     /*!
@@ -74,9 +75,7 @@ namespace Exiv2 {
       @brief Encode the input url.
       @param str The url needs encoding.
       @return the url-encoded version of str.
-
-      @note Be sure to free() the returned string after use
-            Source: http://www.geekhideout.com/urlcode.shtml
+      @note Source: http://www.geekhideout.com/urlcode.shtml
      */
     EXIV2API std::string urlencode(const char *str);
     /*!
diff --git a/src/futils.cpp b/src/futils.cpp
index 9b345fdf..00571aed 100644
--- a/src/futils.cpp
+++ b/src/futils.cpp
@@ -44,6 +44,7 @@
 #include <sstream>
 #include <cstring>
 #include <algorithm>
+#include <stdexcept>
 
 #if defined EXV_HAVE_STRERROR_R && !defined EXV_HAVE_DECL_STRERROR_R
 # ifdef EXV_STRERROR_R_CHAR_P
@@ -58,9 +59,13 @@ namespace Exiv2 {
     const char* ENVARKEY[] = {"EXIV2_HTTP_POST", "EXIV2_TIMEOUT"}; //!< @brief request keys for http exiv2 handler and time-out
 // *****************************************************************************
 // free functions
-    std::string getEnv(EnVar var) {
+    std::string getEnv(EnVar var)
+    {
+        if (var < envHTTPPOST || var > envTIMEOUT) {
+            throw std::out_of_range("Unexpected env variable");
+        }
         return getenv(ENVARKEY[var]) ? getenv(ENVARKEY[var]) : ENVARDEF[var];
-    } // getEnv
+    }
 
     char to_hex(char code) {
         static char hex[] = "0123456789abcdef";
@@ -73,6 +78,8 @@ namespace Exiv2 {
 
     std::string urlencode(const char* str) {
         const char* pstr = str;
+        // \todo try to use std::string for buf and avoid the creation of another string for just
+        // returning the final value
         char* buf  = new char[strlen(str) * 3 + 1];
         char* pbuf = buf;
         while (*pstr) {
diff --git a/unitTests/test_futils.cpp b/unitTests/test_futils.cpp
index 4279d112..7e751ebb 100644
--- a/unitTests/test_futils.cpp
+++ b/unitTests/test_futils.cpp
@@ -15,7 +15,13 @@ TEST(strError, returnSuccessAfterClosingFile)
     std::string tmpFile("tmp.dat");
     std::ofstream auxFile(tmpFile.c_str());
     auxFile.close();
-    ASSERT_STREQ("Success (errno = 0)", strError().c_str());
+#ifdef _WIN32
+    const char * expectedString = "No error (errno = 0)";
+#else
+    const char * expectedString = "Success (errno = 0)";
+#endif
+
+    ASSERT_STREQ(expectedString, strError().c_str());
     std::remove(tmpFile.c_str());
 }
 
@@ -28,7 +34,12 @@ TEST(strError, returnNoSuchFileOrDirectoryWhenTryingToOpenNonExistingFile)
 TEST(strError, doNotRecognizeUnknownError)
 {
     errno = 9999;
-    ASSERT_STREQ("Unknown error 9999 (errno = 9999)", strError().c_str());
+#ifdef _WIN32
+    const char * expectedString = "Unknown error (errno = 9999)";
+#else
+    const char * expectedString = "Unknown error 9999 (errno = 9999)";
+#endif
+    ASSERT_STREQ(expectedString, strError().c_str());
 }
 
 TEST(getEnv, getsDefaultValueWhenExpectedEnvVariableDoesNotExist)
@@ -40,18 +51,22 @@ TEST(getEnv, getsDefaultValueWhenExpectedEnvVariableDoesNotExist)
 TEST(getEnv, getsProperValuesWhenExpectedEnvVariableExists)
 {
     const char * expectedValue = "test";
+#ifdef _WIN32
+    ASSERT_EQ(0, _putenv_s("EXIV2_HTTP_POST", expectedValue));
+#else
     ASSERT_EQ(0, setenv("EXIV2_HTTP_POST", expectedValue, 1));
+#endif
 
     ASSERT_STREQ(expectedValue, getEnv(envHTTPPOST).c_str());
 
+#ifndef _WIN32
     ASSERT_EQ(0, unsetenv("EXIV2_HTTP_POST"));
+#endif
 }
 
 TEST(getEnv, throwsWhenKeyDoesNotExist)
 {
-    // This is just a characterisation test that was written to see how the current implementation
-    // works.
-    ASSERT_THROW(getEnv(static_cast<EnVar>(3)), std::exception);
+    ASSERT_THROW(getEnv(static_cast<EnVar>(3)), std::out_of_range);
 }
 
 TEST(urlencode, encodesGivenUrl)