fix(parser): write numeric/bool values unquoted for newly created config files

parser/helpers.go

@@ -35,6 +35,20 @@ var configMatchRegex = regexp.MustCompile(`{{\s?config\.([\w.-]+)\s?}}`)
 // noinspection RegExpRedundantEscape
 var xmlValueMatchRegex = regexp.MustCompile(`^\[([\w]+)='(.*)'\]$`)
 
+// jsonNumberRegex matches a canonical JSON number exactly as defined by the JSON
+// grammar. We deliberately use this instead of gjson.Parse/json.Valid, which
+// accept lax forms such as "007", "+5", "1.20.1" or "25565x". A value is only
+// written unquoted via sjson.SetRaw when it matches this pattern, guaranteeing
+// the result is valid JSON and the exact literal (including large integers) is
+// preserved.
+var jsonNumberRegex = regexp.MustCompile(`^-?(?:0|[1-9]\d*)(?:\.\d+)?(?:[eE][+-]?\d+)?$`)
+
+// isJSONNumber reports whether s is a canonical JSON number that is safe to write
+// unquoted into a document.
+func isJSONNumber(s string) bool {
+	return jsonNumberRegex.MatchString(s)
+}
+
 // Iterate over an unstructured JSON/YAML/etc. interface and set all of the required
 // key/value pairs for the configuration file.
 //
@@ -156,34 +170,39 @@ func (cfr *ConfigurationFileReplacement) setValueWithSjson(jsonStr string, path
 	}
 
 	var setValue interface{}
-	if cfr.ReplaceWith.Type() == jsonparser.Boolean {
+	switch {
+	case cfr.ReplaceWith.Type() == jsonparser.Boolean:
 		// Explicit boolean type declared in the egg definition.
-		v, err := strconv.ParseBool(value)
-		if err != nil {
-			log.WithFields(log.Fields{"value": value, "path": path, "match": cfr.Match}).Warn("cannot parse replacement as boolean, falling back to string value")
-			return sjson.Set(jsonStr, path, value)
-		}
-		setValue = v
-	} else {
-		// Mirror the type already present in the document so booleans and numbers
-		// survive template expansion (panel always sends values as JSON strings).
+		return cfr.setBool(jsonStr, path, value)
+	case cfr.ReplaceWith.Type() == jsonparser.Number:
+		// Explicit numeric type declared in the egg definition.
+		return cfr.setNumber(jsonStr, path, value)
+	default:
+		// The panel expands template variables and sends every value as a JSON
+		// string, so mirror the type already present in the document where possible
+		// to keep booleans and numbers unquoted.
 		existing := gjson.Get(jsonStr, path)
-		switch existing.Type {
-		case gjson.True, gjson.False:
-			v, err := strconv.ParseBool(value)
-			if err != nil {
-				log.WithFields(log.Fields{"value": value, "path": path, "match": cfr.Match}).Warn("cannot parse replacement as boolean, falling back to string value")
-				return sjson.Set(jsonStr, path, value)
-			}
-			setValue = v
-		case gjson.Number:
+		switch {
+		case existing.Type == gjson.True || existing.Type == gjson.False:
+			return cfr.setBool(jsonStr, path, value)
+		case existing.Type == gjson.Number && isJSONNumber(value):
 			// Write the numeric literal as-is via SetRaw to avoid float64 precision
-			// loss for large integers (> 2^53). Fall back to string if the incoming
-			// value is not a valid JSON number.
-			if gjson.Parse(value).Type == gjson.Number {
+			// loss for large integers (> 2^53).
+			return sjson.SetRaw(jsonStr, path, value)
+		case !existing.Exists():
+			// The key does not exist yet. This is the common case when a server is
+			// first created and its configuration file is generated from scratch:
+			// there is no existing type to mirror. Infer the natural type from the
+			// value itself so numeric values such as ports are written unquoted
+			// (e.g. `server-port: 25565` instead of `server-port: "25565"`).
+			switch {
+			case value == "true" || value == "false":
+				setValue = value == "true"
+			case isJSONNumber(value):
 				return sjson.SetRaw(jsonStr, path, value)
+			default:
+				setValue = value
 			}
-			setValue = value
 		default:
 			setValue = value
 		}
@@ -192,6 +211,31 @@ func (cfr *ConfigurationFileReplacement) setValueWithSjson(jsonStr string, path
 	return sjson.Set(jsonStr, path, setValue)
 }
 
+// setBool parses value as a boolean and writes it to path unquoted. If value is
+// not a valid boolean it logs a warning and falls back to writing the raw string,
+// keeping the explicit-type and mirror-existing-type boolean paths in sync.
+func (cfr *ConfigurationFileReplacement) setBool(jsonStr, path, value string) (string, error) {
+	v, err := strconv.ParseBool(value)
+	if err != nil {
+		log.WithFields(log.Fields{"value": value, "path": path, "match": cfr.Match}).Warn("cannot parse replacement as boolean, falling back to string value")
+		return sjson.Set(jsonStr, path, value)
+	}
+	return sjson.Set(jsonStr, path, v)
+}
+
+// setNumber writes a canonical JSON number to path unquoted via SetRaw to avoid
+// float64 precision loss for large integers (> 2^53). If value is not a canonical
+// JSON number it logs a warning and falls back to writing the raw string, so a
+// misconfigured egg that declares a numeric type for a non-numeric value is
+// diagnosable (mirroring setBool).
+func (cfr *ConfigurationFileReplacement) setNumber(jsonStr, path, value string) (string, error) {
+	if !isJSONNumber(value) {
+		log.WithFields(log.Fields{"value": value, "path": path, "match": cfr.Match}).Warn("cannot parse replacement as number, falling back to string value")
+		return sjson.Set(jsonStr, path, value)
+	}
+	return sjson.SetRaw(jsonStr, path, value)
+}
+
 // Looks up a configuration value on the Daemon given a dot-notated syntax.
 func (f *ConfigurationFile) LookupConfigurationValue(cfr ConfigurationFileReplacement) (result string, err error) {
 	// If this is not something that we can do a regex lookup on then just continue