Merge remote-tracking branch 'origin/topic/jsiwek/improve-command-line-option-redefs'

* origin/topic/jsiwek/improve-command-line-option-redefs:
  Integrate review feedback: improve command-line option redef parsing
  Fix several issues with command-line option redefs

CHANGES

@@ -1,4 +1,25 @@
 
+3.2.0-dev.813 | 2020-06-26 16:25:34 +0000
+
+  * Fix several issues with command-line option redefs
+
+    * Variables of `string` type can now be set to an empty string
+
+    * Trying to set a variable with non-`string` type to an empty value
+      now emits an error instead of silently doing nothing
+
+    * Providing an invalid identifier now emits an "unknown identifier"
+      error instead of silently doing nothing (Jon Siwek, Corelight)
+
+  * Fix "possibly-truncated" compiler warning in BuildJSON snprintf() (Jon Siwek, Corelight)
+
+  * GH-1025: allow copying/cloning of `opaque of Broker::Store`
+
+    Implemented simply as a reference count increment of the
+    data store handle. (Jon Siwek, Corelight)
+
+  * Fix shadowed variable that breaks lookup_hostname(). (Jon Siwek, Corelight)
+
 3.2.0-dev.804 | 2020-06-25 23:58:59 -0700
 
   * Extend dns_request, dns_reject, and dns_query_reply events with original_query param (Ryan Victory)

VERSION

@@ -1 +1 @@
-3.2.0-dev.804
+3.2.0-dev.813

src/scan.l

@@ -978,40 +978,54 @@ int yywrap()
 		return 0;
 
 	// Add redef statements for any X=Y command line parameters.
-	if ( params.size() > 0 )
+	if ( ! params.empty() )
 		{
 		std::string policy;
 
-		for ( unsigned int i = 0; i < params.size(); ++i )
+		for ( const auto& pi : params )
 			{
-			char* param = copy_string(params[i].c_str());
-			char* eq = strchr(param, '=');
-			char* val = eq + 1;
+			auto p = pi.data();
 
-			*eq = '\0';
+			while ( isalnum(*p) || *p == '_' || *p == ':' ) ++p;
 
-			if ( strlen(val) == 0 )
+			auto first_non_id_char = p - pi.data();
+			auto eq_idx = pi.find('=', first_non_id_char);
+			// Omit the '=' from op just to make fmt string below clearer.
+			auto op = pi.substr(first_non_id_char, eq_idx - first_non_id_char);
+			auto id_str = pi.substr(0, first_non_id_char);
+			auto val_str = pi.substr(eq_idx + 1);
+			const auto& id = zeek::id::find(id_str);
+
+			if ( ! id )
 				{
-				delete [] param;
+				reporter->Error("unknown identifier '%s' in command-line options",
+				                id_str.data());
 				continue;
 				}
 
-			// Try to find the type of the param, and interpret
-			// the value intelligently for that type.  (So far,
-			// that just means quoting the value if it's a
-			// string type.)  If no type is found, the value
-			// is left unchanged.
-			std::string opt_quote;	// no optional quote by default
-			const auto& param_id = lookup_ID(param, GLOBAL_MODULE_NAME);
-			Val* v = param_id ? param_id->GetVal().get() : nullptr;
+			// Interpret the value based on the identifier's type.
+			// So far, that just means quoting the value for string types.
+			const auto& type = id->GetType();
 
-			if ( v && v->GetType() && v->GetType()->Tag() == zeek::TYPE_STRING )
-				opt_quote = "\"";	// use quotes
+			if ( ! type )
+				{
+				reporter->Error("can't set value of '%s' in command-line "
+				                "options: unknown type", id_str.data());
+				continue;
+				}
 
-			policy += std::string("redef ") + param + "="
-					+ opt_quote + val + opt_quote + ";";
+			if ( val_str.empty() && ! zeek::IsString(type->Tag()) )
+				{
+				reporter->Error("must assign non-empty value to '%s' in "
+				                "command-line options", id_str.data());
+				continue;
+				}
 
-			delete [] param;
+			auto use_quotes = zeek::IsString(type->Tag());
+			auto fmt_str = use_quotes ? "redef %s %s= \"%s\";"
+			                          : "redef %s %s= %s;";
+
+			policy += fmt(fmt_str, id_str.data(), op.data(), val_str.data());
 			}
 
 		params.clear();

testing/btest/Baseline/core.command-line-option-redefs/1.out

@@ -0,0 +1,4 @@
+mystr, 
+mynum, 0
+mytable, {"one":"1"}
+MyMod::str, Good

testing/btest/Baseline/core.command-line-option-redefs/2.out

@@ -0,0 +1,4 @@
+mystr, default
+mynum, 0
+mytable, {"zero":"0","one":"1"}
+MyMod::str, def

testing/btest/Baseline/core.command-line-option-redefs/3.out

@@ -0,0 +1 @@
+error: unknown identifier 'no_such_var' in command-line options

testing/btest/Baseline/core.command-line-option-redefs/4.out

@@ -0,0 +1,2 @@
+error: must assign non-empty value to 'mynum' in command-line options
+error: must assign non-empty value to 'mytable' in command-line options

testing/btest/core/command-line-option-redefs.zeek

@@ -0,0 +1,27 @@
+# @TEST-EXEC: zeek -b %INPUT mynum=0 mytable='{["one"] = "1"}' mystr="" MyMod::str="Good" >1.out
+# @TEST-EXEC: zeek -b %INPUT mynum=0 mytable+='{["one"] = "1"}' >2.out
+
+# @TEST-EXEC-FAIL: zeek -b %INPUT no_such_var=13 >3.out 2>&1
+# @TEST-EXEC-FAIL: zeek -b %INPUT mynum="" mytable="" >4.out 2>&1
+
+# @TEST-EXEC: btest-diff 1.out
+# @TEST-EXEC: btest-diff 2.out
+# @TEST-EXEC: btest-diff 3.out
+# @TEST-EXEC: btest-diff 4.out
+
+const mynum: count &redef;
+const mytable: table[string] of string = {["zero"] = "0"} &redef;
+option mystr="default";
+
+module MyMod;
+export { option str="def"; }
+module GLOBAL;
+
+event zeek_init()
+	{
+	print "mystr", mystr;
+	print "mynum", mynum;
+	print "mytable", to_json(mytable);
+	print "MyMod::str", MyMod::str;
+	}
+