Fix target search behavior with - and + symbols (#2067)

This PR makes the target search more user-friendly by stripping symbols that
have a special interpretation in MySQL FTS.

Closes #2017

server/datastore/mysql/fulltext.go

@@ -0,0 +1,30 @@
+package mysql
+
+import (
+	"regexp"
+	"strings"
+)
+
+var mysqlFTSSymbolRegexp = regexp.MustCompile("[-+]+")
+
+func queryMinLength(query string) bool {
+	return countLongestTerm(query) >= 3
+}
+
+func countLongestTerm(query string) int {
+	max := 0
+	for _, q := range strings.Split(query, " ") {
+		if len(q) > max {
+			max = len(q)
+		}
+	}
+	return max
+}
+
+// transformQuery replaces occurrences of characters that are treated specially
+// by the MySQL FTS engine to try to make the search more user-friendly
+func transformQuery(query string) string {
+	return strings.TrimSpace(
+		mysqlFTSSymbolRegexp.ReplaceAllLiteralString(query, " "),
+	) + "*"
+}

server/datastore/mysql/fulltext_test.go

@@ -0,0 +1,44 @@
+package mysql
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestTransformQuery(t *testing.T) {
+	testCases := []struct {
+		in  string
+		out string
+	}{
+		{"foobar", "foobar*"},
+		{"tim tom", "tim tom*"},
+		{"f%5", "f%5*"},
+		{"f-o-o-b-a-r", "f o o b a r*"},
+		{"f-o+o-b--+a-r+", "f o o b a r*"},
+	}
+
+	for _, tt := range testCases {
+		t.Run("", func(t *testing.T) {
+			assert.Equal(t, tt.out, transformQuery(tt.in))
+		})
+	}
+}
+
+func TestQueryMinLength(t *testing.T) {
+	testCases := []struct {
+		in  string
+		out bool
+	}{
+		{"a b c d", false},
+		{"foobar", true},
+		{"a foo fim b", true},
+		{"a fo fi b*", false},
+	}
+
+	for _, tt := range testCases {
+		t.Run("", func(t *testing.T) {
+			assert.Equal(t, tt.out, queryMinLength(tt.in))
+		})
+	}
+}

server/datastore/mysql/hosts.go

@@ -540,11 +540,7 @@ func (d *Datastore) MarkHostSeen(host *kolide.Host, t time.Time) error {
 }
 
 func (d *Datastore) searchHostsWithOmits(query string, omit ...uint) ([]*kolide.Host, error) {
-	hostnameQuery := query
-	if len(hostnameQuery) > 0 {
-		hostnameQuery += "*"
-	}
-
+	hostnameQuery := transformQuery(query)
 	ipQuery := `"` + query + `"`
 
 	sqlStatement :=
@@ -630,16 +626,14 @@ func (d *Datastore) searchHostsDefault(omit ...uint) ([]*kolide.Host, error) {
 // SearchHosts find hosts by query containing an IP address or a host name. Optionally
 // pass a list of IDs to omit from the search
 func (d *Datastore) SearchHosts(query string, omit ...uint) ([]*kolide.Host, error) {
-	if query == "" {
+	hostnameQuery := transformQuery(query)
+	if !queryMinLength(hostnameQuery) {
 		return d.searchHostsDefault(omit...)
 	}
 	if len(omit) > 0 {
 		return d.searchHostsWithOmits(query, omit...)
 	}
 
-	hostnameQuery := query
-	hostnameQuery += "*"
-
 	// Needs quotes to avoid each . marking a word boundary
 	ipQuery := `"` + query + `"`
 

server/datastore/mysql/labels.go

@@ -331,9 +331,8 @@ func (d *Datastore) ListUniqueHostsInLabels(labels []uint) ([]kolide.Host, error
 }
 
 func (d *Datastore) searchLabelsWithOmits(query string, omit ...uint) ([]kolide.Label, error) {
-	if len(query) > 0 {
-		query += "*"
-	}
+	transformedQuery := transformQuery(query)
+
 	sqlStatement := `
 		SELECT *
 		FROM labels
@@ -346,7 +345,7 @@ func (d *Datastore) searchLabelsWithOmits(query string, omit ...uint) ([]kolide.
 		LIMIT 10
 	`
 
-	sql, args, err := sqlx.In(sqlStatement, query, omit)
+	sql, args, err := sqlx.In(sqlStatement, transformedQuery, omit)
 	if err != nil {
 		return nil, errors.Wrap(err, "building query for labels with omits")
 	}
@@ -442,15 +441,14 @@ func (d *Datastore) searchLabelsDefault(omit ...uint) ([]kolide.Label, error) {
 
 // SearchLabels performs wildcard searches on kolide.Label name
 func (d *Datastore) SearchLabels(query string, omit ...uint) ([]kolide.Label, error) {
-	if query == "" {
+	transformedQuery := transformQuery(query)
+	if !queryMinLength(transformedQuery) {
 		return d.searchLabelsDefault(omit...)
 	}
 	if len(omit) > 0 {
 		return d.searchLabelsWithOmits(query, omit...)
 	}
 
-	query += "*"
-
 	// Ordering first by label_type ensures that built-in labels come
 	// first. We will probably need to make a custom ordering function here
 	// if additional label types are added. Ordering next by ID ensures
@@ -466,7 +464,7 @@ func (d *Datastore) SearchLabels(query string, omit ...uint) ([]kolide.Label, er
 		LIMIT 10
 	`
 	matches := []kolide.Label{}
-	err := d.db.Select(&matches, sqlStatement, query)
+	err := d.db.Select(&matches, sqlStatement, transformedQuery)
 	if err != nil {
 		return nil, errors.Wrap(err, "selecting labels for search")
 	}