From c269886389a95b11dc16e9221c8f2e4ca170a19f Mon Sep 17 00:00:00 2001 From: Zachary Wasserman Date: Wed, 3 Jul 2019 10:47:43 -0700 Subject: [PATCH] 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 | 30 +++++++++++++++++ server/datastore/mysql/fulltext_test.go | 44 +++++++++++++++++++++++++ server/datastore/mysql/hosts.go | 12 ++----- server/datastore/mysql/labels.go | 14 ++++---- 4 files changed, 83 insertions(+), 17 deletions(-) create mode 100644 server/datastore/mysql/fulltext.go create mode 100644 server/datastore/mysql/fulltext_test.go diff --git a/server/datastore/mysql/fulltext.go b/server/datastore/mysql/fulltext.go new file mode 100644 index 0000000000..a490941660 --- /dev/null +++ b/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, " "), + ) + "*" +} diff --git a/server/datastore/mysql/fulltext_test.go b/server/datastore/mysql/fulltext_test.go new file mode 100644 index 0000000000..c9b9aed048 --- /dev/null +++ b/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)) + }) + } +} diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index 770004ddf1..45c4f47b5c 100644 --- a/server/datastore/mysql/hosts.go +++ b/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 + `"` diff --git a/server/datastore/mysql/labels.go b/server/datastore/mysql/labels.go index 21ef9db6e9..3f2d3a5096 100644 --- a/server/datastore/mysql/labels.go +++ b/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") }