Merge pull request #202 from newrelic/daffi/issue-201

fix: Base/Config/Validate panic

go.mod

@@ -13,7 +13,6 @@ require (
 	github.com/shirou/w32 v0.0.0-20160930032740-bb4de0191aa4
 	github.com/stretchr/testify v1.8.4
 	golang.org/x/exp v0.0.0-20230905200255-921286631fa9
-	golang.org/x/net v0.15.0
 	golang.org/x/sys v0.12.0
 	gopkg.in/cheggaaa/pb.v1 v1.0.28
 	gopkg.in/yaml.v3 v3.0.1
@@ -38,6 +37,7 @@ require (
 	github.com/tklauser/go-sysconf v0.3.12 // indirect
 	github.com/tklauser/numcpus v0.6.1 // indirect
 	github.com/yusufpapurcu/wmi v1.2.3 // indirect
+	golang.org/x/net v0.15.0 // indirect
 	golang.org/x/text v0.13.0 // indirect
 	golang.org/x/tools v0.13.0 // indirect
 )

suites/suiteDefinitions.go

@@ -22,7 +22,7 @@ var suiteDefinitions = []Suite{
 	{
 		Identifier:  "infra:debug",
 		DisplayName: "Infrastructure Agent (Debug)",
-		Description: "Infrastructure Agent installation with 3 minutes of debug log collection",
+		Description: "Infrastructure Agent installation with 5 minutes of debug log collection",
 		Tasks: []string{
 			"Base/*",
 			"Infra/*",

tasks/base/config/fixtures/Should_return_matching_array_elements.golden

@@ -1,18 +1,18 @@
 /allow_all_headers: true
 /app_name/0: My Application
 /attributes: {
-/attributes.exclude/0: 'request.headers.cookie'
-/attributes.exclude/1: 'request.headers.authorization'
-/attributes.exclude/2: 'request.headers.proxyAuthorization'
-/attributes.exclude/3: 'request.headers.setCookie*'
-/attributes.exclude/4: 'request.headers.x*'
-/attributes.exclude/5: 'response.headers.cookie'
-/attributes.exclude/6: 'response.headers.authorization'
-/attributes.exclude/7: 'response.headers.proxyAuthorization'
-/attributes.exclude/8: 'response.headers.setCookie*'
-/attributes.exclude/9: 'response.headers.x*'
-/attributes.include/0: 'request.include.something'
-/attributes.include/1: 'response.include.something*'
+/attributes.exclude/0: request.headers.cookie
+/attributes.exclude/1: request.headers.authorization
+/attributes.exclude/2: request.headers.proxyAuthorization
+/attributes.exclude/3: request.headers.setCookie*
+/attributes.exclude/4: request.headers.x*
+/attributes.exclude/5: response.headers.cookie
+/attributes.exclude/6: response.headers.authorization
+/attributes.exclude/7: response.headers.proxyAuthorization
+/attributes.exclude/8: response.headers.setCookie*
+/attributes.exclude/9: response.headers.x*
+/attributes.include/0: request.include.something
+/attributes.include/1: response.include.something*
 /license_key: license-key-val-node
 /logging: {
 /logging.level: trace

tasks/base/config/fixtures/Should_return_matching_es6_config.golden

@@ -1,4 +1,3 @@
-/.env({ separator: '__' })
 /agent_enabled: configuration.get('monitoring').newrelic && configuration.get('monitoring').newrelic.enabled
 /app_name/0: ${configuration.get('service').name}: ${configuration.get('service').environment}
 /license_key: configuration.get('monitoring').newrelic.license

tasks/base/config/fixtures/Should_return_matching_typescript_config.golden

@@ -1,4 +1,3 @@
-/Object.defineProperty(exports, "__esModule", { value: true });
 /agent_enabled: ServiceConfiguration_1.getConfigStore().monitoring.newrelic && ServiceConfiguration_1.getConfigStore().monitoring.newrelic.enabled
 /app_name/0: ${ServiceConfiguration_1.getConfigStore().service.name}: ${ServiceConfiguration_1.getConfigStore().service.environment}
 /license_key: ServiceConfiguration_1.getConfigStore().monitoring.newrelic.license

tasks/base/config/integrationTests.yml

@@ -65,7 +65,7 @@
     docker_cmd: ./nrdiag && cat nrdiag-output.json
  -  test_name: ProxyDetectNode
     dockerfile_lines:
-     - COPY tasks/fixtures/integration/proxyDetect/newrelicnode /app/newrelic.js
+     - COPY tasks/fixtures/integration/proxyDetect/newrelicnode.js /app/newrelic.js
     log_entry_expected:
      - Success.*Base/Config/ProxyDetect
      - http://user:pass@10.0.0.1:8000

tasks/base/config/validate.go

@@ -283,59 +283,98 @@ func ParseJSONarray(reader io.Reader) (result []tasks.ValidateBlob, err error) {
 	return validateBlobs, nil
 }
 
-func parseJs(reader io.Reader) (result tasks.ValidateBlob, err error) {
-	tempMap := make(map[string]interface{})
-	//So this function basically reads the newrelic.js and re-constructs the exported json config from the `exports.config=` beyond
-
-	var jsonString []string
-	var isComment bool
-
-	scanner := bufio.NewScanner(reader)
-	scanner.Split(bufio.ScanLines)
-	startComment, _ := regexp.Compile("^[/][*].*$")         // looks for /* at the beginning of lines in the config file to remove lines with comments
-	endComment, _ := regexp.Compile("^.*[*][/]$")           // looks for */ at the end of lines to assist in removal of comments
-	oneLineComment, _ := regexp.Compile("^[/][*].*[*][/]$") // looks for single line comments /* like this */
-	slashComment, _ := regexp.Compile("^[/][/].*$")         // this looks for comments // like this
-
-	for scanner.Scan() {
-		scannerText := strings.TrimSpace(scanner.Text()) // trim leading and trailing whitespace
-		if oneLineComment.MatchString(scannerText) || slashComment.MatchString(scannerText) {
-			// check for one liners first since startComment and endComment both match one liners as well and will mess up the logic below
-			continue
-		}
-		if startComment.MatchString(scannerText) {
-			isComment = true // starts a comment block this and the following lines are ignored until we see */
-			continue
+// formatJs - Outputs the newrelic.js file into a more parsable format with comments and other non-essential items removed
+func formatJs(jsString string) ([]string, error) {
+	// remove comments that start with //
+	// have to be careful not to remove proxy configs
+	slashCommentRe, err := regexp.Compile(`(?m)(^.*)([/][/].*)$`)
+	if err != nil {
+		return nil, err
+	}
+	quoteRe, err := regexp.Compile("['`\"]")
+	if err != nil {
+		return nil, err
+	}
+	removeSlashComments := slashCommentRe.ReplaceAllFunc([]byte(jsString), func(b []byte) []byte {
+		s := string(b)
+		checkForComment := strings.Split(s, "//")
+		// nothing on the left of the //, whole line is a comment, just remove it
+		if checkForComment[0] == "" {
+			return nil
 		}
-		if endComment.MatchString(scannerText) {
-			isComment = false // ends the comment block
-			continue
+		// check to see if the // is within quotes like 'http://...'
+		quoteCount := len(quoteRe.FindAllStringIndex(checkForComment[0], -1))
+		if quoteCount == 0 || quoteCount%2 == 0 {
+			// not in quotes
+			return []byte(checkForComment[0])
 		}
+		// keep the //, it was in quotes
+		return b
+	})
+	// remove \n and \r
+	removeLineBreaks := strings.ReplaceAll(string(removeSlashComments), "\n", "")
+	removeCarriageReturn := strings.ReplaceAll(removeLineBreaks, "\r", "")
 
-		if !isComment {
-			if strings.Contains(scannerText, "use strict") {
-				continue
-			}
+	// remove everything before exports.config =
+	exportObj := strings.Split(removeCarriageReturn, "exports.config =")[1]
 
-			if strings.Contains(scannerText, "=") {
-				t := strings.Split(scannerText, "=")
-				jsonString = append(jsonString, strings.TrimSpace(t[1]))
-				continue
+	// remove /* this type of comment */
+	commentRe, err := regexp.Compile("[/][*].*?[*][/]")
+	if err != nil {
+		return nil, err
+	}
+	removeComments := commentRe.ReplaceAllString(exportObj, "")
 
-			}
+	// find start curlies ({) and add a linebreak after it
+	startCurliesRe, err := regexp.Compile("(^{|[^$]{)")
+	if err != nil {
+		return nil, err
+	}
+	fixStartCurlies := startCurliesRe.ReplaceAllString(removeComments, "{\n")
 
-			jsonString = append(jsonString, scannerText)
+	// fix formatting for arrays, make them multi-line
+	fixStartArrays := strings.ReplaceAll(fixStartCurlies, "[", "[\n")
+	fixEndArrays := strings.ReplaceAll(fixStartArrays, "]", "\n]")
 
+	// add a linebreak after commas
+	fixCommas := strings.ReplaceAll(fixEndArrays, ",", ",\n")
+
+	// create the array and trim whitespace
+	var jsonString []string
+	for _, s := range strings.Split(fixCommas, "\n") {
+		// fix end curlies - only add a line break if { isn't also on the line
+		if strings.Contains(s, "}") && !strings.Contains(s, "{") {
+			fixEndCurlies := strings.ReplaceAll(s, "}", "\n}")
+			splitAgain := strings.Split(fixEndCurlies, "\n")
+			for _, ss := range splitAgain {
+				jsonString = append(jsonString, strings.TrimSpace(ss))
+			}
+			continue
 		}
+		jsonString = append(jsonString, strings.TrimSpace(s))
 	}
+	return jsonString, nil
+}
+
+func parseJs(rawFile io.Reader) (result tasks.ValidateBlob, err error) {
+	jsBytes, err := io.ReadAll(rawFile)
+	if err != nil {
+		return
+	}
+	jsonString, err := formatJs(string(jsBytes))
+	if err != nil {
+		return
+	}
+	log.Debug("Formatted js: ", strings.Join(jsonString, "\n"))
+	tempMap := make(map[string]interface{})
 
 	location := ""
 	arrayLocation := ""
 	var buildarray []string
 
 loop:
 	for lineNum, keyValue := range jsonString {
-		log.Debug("keyvalue", keyValue)
+		log.Debugf("keyValue: `%s`", keyValue)
 		switch keyValue {
 		case "{": //do nothing here
 		case "}", "},":
@@ -357,14 +396,15 @@ loop:
 
 		default:
 			if arrayLocation != "" {
-				log.Debug("creating multi-line array", strings.Replace(keyValue, ",", "", 1))
-				buildarray = append(buildarray, strings.Replace(keyValue, ",", "", 1))
+				log.Debug("creating multi-line array", sanitizeValue(keyValue))
+				buildarray = append(buildarray, sanitizeValue(keyValue))
 				continue // we are in an array, go to next line
 			}
 
 			keyMap := strings.SplitN(keyValue, ":", 2)
-			if len(keyMap) != 2 {
-				log.Debug("Couldn't parse line", lineNum)
+
+			if len(keyMap) != 2 || keyMap[1] == "" {
+				log.Debugf("Couldn't parse line number %d, line text: %v\n", lineNum, keyMap)
 				break
 			}
 
@@ -391,10 +431,8 @@ loop:
 					finalSlice = append(finalSlice, sanitizeValue(value))
 				}
 				tempMap[location+keyMap[0]] = finalSlice
-
 			} else {
 				tempMap[location+keyMap[0]] = sanitizeValue(keyMap[1])
-
 			}
 
 			if strings.TrimSpace(keyMap[1]) == "{" {
@@ -404,7 +442,7 @@ loop:
 
 		}
 	}
-	//log.Dump(tempMap)
+
 	return convertToValidateBlob(tempMap), nil
 }
 
@@ -497,8 +535,6 @@ func iterateMap(parent string, input interface{}) []tasks.ValidateBlob {
 		b.Key = key
 		b.Path = parent
 
-		//log.Debug("value type is ", reflect.TypeOf(value))
-
 		//Add Children if value is a map or slice
 		switch castValue := value.(type) {
 		case map[interface{}]interface{}, map[string]interface{}, []interface{}, []string:

tasks/base/config/validate_test.go

@@ -3,7 +3,6 @@ package config
 import (
 	"flag"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"strings"
 	"testing"
@@ -384,7 +383,7 @@ var _ = Describe("Base/Config/Validate", func() {
 			})
 		})
 		Context("When parsing js config with arrays", func() {
-			file, _ := os.Open("../../fixtures/node/newrelicWithArrays.js")
+			file, _ := os.Open("./fixtures/validate_testdata_js_with_array.js")
 			defer file.Close()
 			result, _ := parseJs(file)
 			result.Sort()
@@ -674,7 +673,7 @@ var _ = Describe("Base/Config/Validate", func() {
 })
 
 func readFile(file string) string {
-	content, err := ioutil.ReadFile(file)
+	content, err := os.ReadFile(file)
 	if err != nil {
 		log.Info("error reading file", err)
 	}

tasks/fixtures/integration/proxyDetect/newrelicnode → tasks/fixtures/integration/proxyDetect/newrelicnode.js

@@ -6,15 +6,16 @@
  * See lib/config.defaults.js in the agent distribution for a more complete
  * description of configuration variables and their potential values.
  */
-exports.config = { 
+// slash comment
+exports.config = { // slash comment
   /**
    * Array of application names.
    */
   app_name: ['My Node App'],
   /**
    * Your New Relic license key.
    */
-  license_key: 'license-key-val-node',
+  license_key: 'license-key-val-node',//comment
   proxy: 'http://user:pass@10.0.0.1:8000',
   logging: {
     /**