Skip to content

Commit 89b5212

Browse files
authored
fix: parser correctness fixes (integer precision, nil safety, multi-variable lookup, type coercion, IfValue, XML duplicates) (#179)
* fix: preserve integer precision in YAML parser (UseNumber + normalizeYamlTypes) * fix: multiple parser bugs - UnmarshalJSON: guard against nil pointer panic on missing 'file'/'parser' keys - LookupConfigurationValue: resolve each {{ config.x }} placeholder independently so composite values like '{{ config.a }}:{{ config.b }}' expand correctly - setValueWithSjson: drop implicit string→int coercion in default case to preserve the original value type - parseTextFile: honour if_value condition (was silently ignored before) - parseXmlFile: remove existing attribute before CreateAttr to prevent duplicates on repeated parses * fix: address Copilot review comments
1 parent a57491d commit 89b5212

2 files changed

Lines changed: 89 additions & 34 deletions

File tree

parser/helpers.go

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,7 @@ func (cfr *ConfigurationFileReplacement) setValueWithSjson(jsonStr string, path
185185
}
186186
setValue = value
187187
default:
188-
if v, err := strconv.Atoi(value); err == nil {
189-
setValue = v
190-
} else {
191-
setValue = value
192-
}
188+
setValue = value
193189
}
194190
}
195191

@@ -208,29 +204,30 @@ func (f *ConfigurationFile) LookupConfigurationValue(cfr ConfigurationFileReplac
208204
// If there is a match, lookup the value in the configuration for the Daemon. If no key
209205
// is found, just return the string representation, otherwise use the value from the
210206
// daemon configuration here.
211-
huntPath := configMatchRegex.ReplaceAllString(
212-
configMatchRegex.FindString(cfr.ReplaceWith.String()), "$1",
213-
)
214-
215-
var path []string
216-
for _, value := range strings.Split(huntPath, ".") {
217-
path = append(path, strcase.ToSnake(value))
218-
}
219-
220-
// Look for the key in the configuration file, and if found return that value to the
221-
// calling function.
222-
match, _, _, err := jsonparser.Get(f.configuration, path...)
223-
if err != nil {
224-
if err != jsonparser.KeyPathNotFoundError {
225-
return string(match), err
207+
var lookupErr error
208+
result := configMatchRegex.ReplaceAllStringFunc(cfr.ReplaceWith.String(), func(placeholder string) string {
209+
if lookupErr != nil {
210+
return placeholder
226211
}
212+
keyPath := configMatchRegex.ReplaceAllString(placeholder, "$1")
227213

228-
log.WithFields(log.Fields{"path": path, "filename": f.FileName}).Debug("attempted to load a configuration value that does not exist")
214+
var path []string
215+
for _, part := range strings.Split(keyPath, ".") {
216+
path = append(path, strcase.ToSnake(part))
217+
}
229218

230-
// If there is no key, keep the original value intact, that way it is obvious there
231-
// is a replace issue at play.
232-
return string(match), nil
233-
} else {
234-
return configMatchRegex.ReplaceAllString(cfr.ReplaceWith.String(), string(match)), nil
235-
}
219+
// Look for the key in the Wings configuration and substitute the placeholder.
220+
match, _, _, err := jsonparser.Get(f.configuration, path...)
221+
if err != nil {
222+
if err != jsonparser.KeyPathNotFoundError {
223+
lookupErr = err
224+
return placeholder
225+
}
226+
log.WithFields(log.Fields{"path": path, "filename": f.FileName}).Debug("attempted to load a configuration value that does not exist")
227+
// Leave placeholder intact so the misconfiguration is visible.
228+
return placeholder
229+
}
230+
return string(match)
231+
})
232+
return result, lookupErr
236233
}

parser/parser.go

Lines changed: 65 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,18 +128,27 @@ func (f *ConfigurationFile) UnmarshalJSON(data []byte) error {
128128
return err
129129
}
130130

131-
if err := json.Unmarshal(*m["file"], &f.FileName); err != nil {
131+
fileRaw, ok := m["file"]
132+
if !ok || fileRaw == nil {
133+
return errors.New("parser: configuration file missing required 'file' key")
134+
}
135+
if err := json.Unmarshal(*fileRaw, &f.FileName); err != nil {
132136
return err
133137
}
134138

135-
if err := json.Unmarshal(*m["parser"], &f.Parser); err != nil {
139+
parserRaw, ok := m["parser"]
140+
if !ok || parserRaw == nil {
141+
return errors.New("parser: configuration file missing required 'parser' key")
142+
}
143+
if err := json.Unmarshal(*parserRaw, &f.Parser); err != nil {
136144
return err
137145
}
138146

139-
if err := json.Unmarshal(*m["replace"], &f.Replace); err != nil {
140-
log.WithField("file", f.FileName).WithField("error", err).Warn("failed to unmarshal configuration file replacement")
141-
142-
f.Replace = []ConfigurationFileReplacement{}
147+
f.Replace = []ConfigurationFileReplacement{}
148+
if replaceRaw, ok := m["replace"]; ok && replaceRaw != nil {
149+
if err := json.Unmarshal(*replaceRaw, &f.Replace); err != nil {
150+
log.WithField("file", f.FileName).WithField("error", err).Warn("failed to unmarshal configuration file replacement")
151+
}
143152
}
144153

145154
// test if "create_file" exists, if not just assume true
@@ -292,6 +301,7 @@ func (f *ConfigurationFile) parseXmlFile(file ufs.File) error {
292301
k := xmlValueMatchRegex.ReplaceAllString(value, "$1")
293302
v := xmlValueMatchRegex.ReplaceAllString(value, "$2")
294303

304+
element.RemoveAttr(k)
295305
element.CreateAttr(k, v)
296306
} else {
297307
element.SetText(value)
@@ -462,9 +472,12 @@ func (f *ConfigurationFile) parseYamlFile(file ufs.File) error {
462472
}
463473

464474
var jsonData interface{}
465-
if err := json.Unmarshal(data, &jsonData); err != nil {
475+
yamlDecoder := json.NewDecoder(bytes.NewReader(data))
476+
yamlDecoder.UseNumber()
477+
if err := yamlDecoder.Decode(&jsonData); err != nil {
466478
return err
467479
}
480+
jsonData = normalizeYamlTypes(jsonData)
468481

469482
marshaled, err := yaml.Marshal(jsonData)
470483
if err != nil {
@@ -536,6 +549,43 @@ func (f *ConfigurationFile) parseTomlFile(file ufs.File) error {
536549
return nil
537550
}
538551

552+
// normalizeYamlTypes converts json.Number values (produced by UseNumber()) into
553+
// proper Go numeric types so that yaml.Marshal writes integers as plain integers
554+
// instead of float64 scientific notation (e.g. 1.5e+18 for large Snowflake IDs).
555+
func normalizeYamlTypes(value interface{}) interface{} {
556+
switch typed := value.(type) {
557+
case map[string]interface{}:
558+
for key, item := range typed {
559+
typed[key] = normalizeYamlTypes(item)
560+
}
561+
return typed
562+
case []interface{}:
563+
for i := range typed {
564+
typed[i] = normalizeYamlTypes(typed[i])
565+
}
566+
return typed
567+
case json.Number:
568+
s := typed.String()
569+
// Preserve float representation: if the number contains '.', 'e' or 'E'
570+
// it was originally a float and must not be coerced to int64.
571+
if strings.ContainsAny(s, ".eE") {
572+
if floatVal, err := typed.Float64(); err == nil {
573+
return floatVal
574+
}
575+
return s
576+
}
577+
if intVal, err := typed.Int64(); err == nil {
578+
return intVal
579+
}
580+
if floatVal, err := typed.Float64(); err == nil {
581+
return floatVal
582+
}
583+
return s
584+
default:
585+
return value
586+
}
587+
}
588+
539589
func normalizeTomlTypes(value interface{}) interface{} {
540590
switch typed := value.(type) {
541591
case map[string]interface{}:
@@ -585,6 +635,14 @@ func (f *ConfigurationFile) parseTextFile(file ufs.File) error {
585635
if !bytes.HasPrefix(line, []byte(replace.Match)) {
586636
continue
587637
}
638+
// If an if_value is set, only replace when the remainder of the line matches.
639+
// Trim trailing \r\n so Windows line endings do not break the comparison.
640+
if replace.IfValue != "" {
641+
remainder := bytes.TrimRight(bytes.TrimPrefix(line, []byte(replace.Match)), "\r\n")
642+
if string(remainder) != replace.IfValue {
643+
continue
644+
}
645+
}
588646
b.Write(replace.ReplaceWith.Bytes())
589647
replaced = true
590648
}

0 commit comments

Comments
 (0)