From: Mostyn Bramley-Moore <mostyn@antipode.se>
Date: Fri, 23 Jun 2017 18:20:54 +0000 (+0200)
Subject: Fail if boolean env vars are set with common negative strings
X-Git-Tag: v3.4~27
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e7906f4682f4a56e1cfca7581845a7e0b5b7ce65;p=thirdparty%2Fccache.git

Fail if boolean env vars are set with common negative strings

Boolean environment variables have strange semantics: if the variable
is set then the configuration setting is considered "true".  This means
users can easily be mistaken that eg CCACHE_DISABLE=false means that
the "disable" setting is set to false.

To avoid too much backwards-incompatibility, we now exit with an error if
some common negative-sounding (case-insensitive) values:
"0", "false", "disable", "no".

All other values (including the empty string) are still considered to mean
"true".

Resolves https://github.com/ccache/ccache/issues/182
---

diff --git a/MANUAL.txt b/MANUAL.txt
index 6205667d0..01dafae2f 100644
--- a/MANUAL.txt
+++ b/MANUAL.txt
@@ -222,11 +222,12 @@ Boolean values
 Some settings are boolean values (i.e. truth values). In a configuration file,
 such values must be set to the string *true* or *false*. For the corresponding
 environment variables, the semantics are a bit different: a set environment
-variable means ``true'' regardless of the value (even if set to the empty
-string), and an unset environment variable means ``false''. Each boolean
-environment variable also has a negated form starting with *CCACHE_NO*. For
-example, *CCACHE_COMPRESS* can be set to force compression and
-*CCACHE_NOCOMPRESS* can be set to force no compression.
+variable means ``true'' (even if set to the empty string), the following
+case-insensitive negative values are considered an error (rather than surprise
+the user): "0", "false", "disable" and "no", and an unset environment variable
+means ``false''. Each boolean environment variable also has a negated form
+starting with *CCACHE_NO*. For example, *CCACHE_COMPRESS* can be set to force
+compression and *CCACHE_NOCOMPRESS* can be set to force no compression.
 
 
 Configuration settings
diff --git a/NEWS.txt b/NEWS.txt
index 1b94f119a..c46d55543 100644
--- a/NEWS.txt
+++ b/NEWS.txt
@@ -22,6 +22,11 @@ New features and improvements
 - Added support for nvcc compiler options `--compiler-bindir/-ccbin`,
   `--output-directory/-odir` and `--libdevice-directory/-ldir`.
 
+- Boolean configuration settings no longer accept the following
+  (case-insensitive) values: "0", "false", "disable" and "no". All other values
+  are accepted and taken to mean "true". This is intended to avoid users
+  setting eg CCACHE_DISABLE=0 and expecting the cache to be used.
+
 
 Bug fixes
 ~~~~~~~~~
diff --git a/conf.c b/conf.c
index cfa2874af..6a0a9e66f 100644
--- a/conf.c
+++ b/conf.c
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2016 Joel Rosdahl
+// Copyright (C) 2011-2018 Joel Rosdahl
 //
 // This program is free software; you can redistribute it and/or modify it
 // under the terms of the GNU General Public License as published by the Free
@@ -241,8 +241,19 @@ handle_conf_setting(struct conf *conf, const char *key, const char *value,
 	}
 
 	if (from_env_variable && item->parser == parse_bool) {
-		// Special rule for boolean settings from the environment: any value means
-		// true.
+		// Special rule for boolean settings from the environment:
+		// "0", "false", "disable" and "no" (case insensitive) are invalid,
+		// and all other values mean true.
+		//
+		// Previously any value meant true, but this was surprising to
+		// users, who might do something like CCACHE_DISABLE=0 and expect
+		// ccache to be enabled.
+
+		if (!strcmp(value, "0") || !strcasecmp(value, "false") ||
+				!strcasecmp(value, "disable") || !strcasecmp(value, "no")) {
+			fatal("invalid boolean environment variable value \"%s\"", value);
+		}
+
 		bool *value = (bool *)((char *)conf + item->offset);
 		*value = !negate_boolean;
 		goto out;
diff --git a/test/suites/base.bash b/test/suites/base.bash
index 3c836dd15..06eac9179 100644
--- a/test/suites/base.bash
+++ b/test/suites/base.bash
@@ -909,6 +909,20 @@ EOF
     if [ "$stderr" != "2Pu1Cc" ]; then
         test_failed "Unexpected stderr: $stderr != 2Pu1Cc"
     fi
+
+    # -------------------------------------------------------------------------
+    TEST "Invalid boolean environment configuration options"
+
+    for invalid_val in 0 false FALSE disable no ; do
+        CCACHE_DISABLE=$invalid_val $CCACHE $COMPILER --version > /dev/null 2>&1
+        if [ $? -eq 0 ] ; then
+            test_failed "'$invalid_val' should be rejected for boolean env vars"
+        fi
+        CCACHE_NODISABLE=$invalid_val $CCACHE $COMPILER --version > /dev/null 2>&1
+        if [ $? -eq 0 ] ; then
+            test_failed "'$invalid_val' should be rejected for boolean env vars"
+        fi
+    done
 }
 
 # =============================================================================