diff --git a/internal/adapters/db/like.go b/internal/adapters/db/like.go new file mode 100644 index 0000000..66e737f --- /dev/null +++ b/internal/adapters/db/like.go @@ -0,0 +1,14 @@ +package db + +import "strings" + +// likeWildcards escapes the three characters LIKE treats as special. The backslash goes first, or escaping the other two would escape the escapes. +var likeWildcards = strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`) + +// containsPattern builds the argument for a `col ILIKE ? ESCAPE '\'` containment match. +// +// % and _ are wildcards to LIKE and ordinary characters to whoever typed them into a search box: "100%" has to find "100%" rather than everything that +// starts with 100, and "user_id" must not also match "userXid". +func containsPattern(s string) string { + return "%" + likeWildcards.Replace(s) + "%" +} diff --git a/internal/adapters/db/like_test.go b/internal/adapters/db/like_test.go new file mode 100644 index 0000000..ac5cd71 --- /dev/null +++ b/internal/adapters/db/like_test.go @@ -0,0 +1,29 @@ +package db + +import "testing" + +// The search box is the only place a user types these characters, and every one +// of them used to reach LIKE as a wildcard. +func TestContainsPatternEscapesWildcards(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"plain text is only wrapped", "timeout", `%timeout%`}, + {"percent is literal", "100%", `%100\%%`}, + {"underscore is literal", "user_id", `%user\_id%`}, + {"backslash is literal", `C:\tmp`, `%C:\\tmp%`}, + // The backslash has to be escaped before the others + {"backslash before a percent", `50\%`, `%50\\\%%`}, + {"empty stays a match-all", "", `%%`}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := containsPattern(c.in); got != c.want { + t.Errorf("containsPattern(%q) = %q, want %q", c.in, got, c.want) + } + }) + } +} diff --git a/internal/adapters/db/log_repo.go b/internal/adapters/db/log_repo.go index 0db6d77..6d8d0f6 100644 --- a/internal/adapters/db/log_repo.go +++ b/internal/adapters/db/log_repo.go @@ -107,13 +107,14 @@ func sessionScopeSubquery(db *gorm.DB, projectID uuid.UUID, s domain.SessionScop q = q.Where("sdk_version = ?", s.SDKVersion) } if s.Device != "" { - q = q.Where("device_model ILIKE ?", "%"+s.Device+"%") + q = q.Where(`device_model ILIKE ? ESCAPE '\'`, containsPattern(s.Device)) } if s.OS != "" { // Matches either half of what a person reads on screen: `os:Android` // and `os:14` both find "Android 14", because the row shows them // together and nobody remembers which column they came from. - q = q.Where("(os_name ILIKE ? OR os_version ILIKE ?)", "%"+s.OS+"%", "%"+s.OS+"%") + q = q.Where(`(os_name ILIKE ? ESCAPE '\' OR os_version ILIKE ? ESCAPE '\')`, + containsPattern(s.OS), containsPattern(s.OS)) } return q } @@ -147,7 +148,8 @@ func (r *LogRepo) List(ctx context.Context, opts domain.LogListOpts) (*domain.Lo query = query.Where("fingerprint = ?", *f.Fingerprint) } if f.TextQuery != "" { - query = query.Where("message LIKE ?", "%"+f.TextQuery+"%") + // nobody recalls a message in the case it was logged in. + query = query.Where(`message ILIKE ? ESCAPE '\'`, containsPattern(f.TextQuery)) } if scope := sessionScopeSubquery(db.WithContext(ctx), opts.ProjectID, f.Session); scope != nil { query = query.Where("session_id IN (?)", scope) @@ -620,7 +622,7 @@ func (r *LogRepo) ListNetworkCalls(ctx context.Context, projectID uuid.UUID, f d } if f.Path != "" { // ILIKE, because nobody types a path with the case they logged it in. - query = query.Where("url ILIKE ?", "%"+f.Path+"%") + query = query.Where(`url ILIKE ? ESCAPE '\'`, containsPattern(f.Path)) } if f.Method != "" { query = query.Where("UPPER(method) = ?", strings.ToUpper(f.Method)) diff --git a/internal/adapters/db/log_repo_integration_test.go b/internal/adapters/db/log_repo_integration_test.go index b7540c5..1d9f26e 100644 --- a/internal/adapters/db/log_repo_integration_test.go +++ b/internal/adapters/db/log_repo_integration_test.go @@ -846,3 +846,40 @@ func TestNetworkCallsIgnoreNonNetworkAndOtherProjects(t *testing.T) { t.Errorf("want only this project's one call, got %+v", calls) } } + +// Two bugs on the free-text filter, both of which quietly returned the wrong +// rows rather than failing: +// +// - it was LIKE, not ILIKE, so searching "error" never found "Error"; every message that began a sentence was invisible to the obvious search for it. +// - the pattern was "%"+text+"%" with nothing escaped, so "100%" matched anything containing "100", and "user_id" matched "userXid" too. +func TestTextSearchIsCaseInsensitiveAndTakesWildcardsLiterally(t *testing.T) { + db := testDB(t) + repo := NewLogRepo(db) + ctx := context.Background() + projectID := seedProject(t, db) + + logs := []domain.Log{ + taggedLog(projectID, "Error: upload failed", "error", nil), + taggedLog(projectID, "battery at 100% before sync", "info", nil), + taggedLog(projectID, "battery at 1004 mAh", "info", nil), + taggedLog(projectID, "missing user_id on request", "info", nil), + taggedLog(projectID, "missing userXid on request", "info", nil), + } + if _, err := repo.CreateBatch(ctx, logs); err != nil { + t.Fatalf("seed: %v", err) + } + + find := func(query string) []string { + return listWith(t, repo, projectID, domain.SearchFilter{TextQuery: query}) + } + + if got := find("error"); len(got) != 1 || got[0] != "Error: upload failed" { + t.Errorf(`searching "error" should find the capitalised message, got %v`, got) + } + if got := find("100%"); len(got) != 1 || got[0] != "battery at 100% before sync" { + t.Errorf(`"%%" should be a literal, not a wildcard reaching "1004", got %v`, got) + } + if got := find("user_id"); len(got) != 1 || got[0] != "missing user_id on request" { + t.Errorf(`"_" should be a literal, not a wildcard reaching "userXid", got %v`, got) + } +} diff --git a/internal/adapters/db/project_repo.go b/internal/adapters/db/project_repo.go index 8d53558..e67e198 100644 --- a/internal/adapters/db/project_repo.go +++ b/internal/adapters/db/project_repo.go @@ -264,7 +264,7 @@ func (r *ProjectRepo) List(ctx context.Context, opts domain.ProjectListOpts) (*d query := db.WithContext(ctx).Model(&ProjectModel{}) if opts.Search != "" { - query = query.Where("projects.name ILIKE ?", "%"+opts.Search+"%") + query = query.Where(`projects.name ILIKE ? ESCAPE '\'`, containsPattern(opts.Search)) } // One join carries both jobs: an inner join filters to the user's diff --git a/internal/domain/search.go b/internal/domain/search.go index c6e1a92..9d8bd56 100644 --- a/internal/domain/search.go +++ b/internal/domain/search.go @@ -22,7 +22,7 @@ type SearchFilter struct { // Tags, so `tag:checkout -tag:heartbeat` is a sensible pair rather than a // contradiction. ExcludeTags []string - TextQuery string // LIKE match on message field + TextQuery string // case-insensitive substring match on message; % and _ are literal IsNetwork *bool // filter network calls only SessionID *uuid.UUID RequestID *uuid.UUID