Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions internal/adapters/db/like.go
Original file line number Diff line number Diff line change
@@ -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) + "%"
}
29 changes: 29 additions & 0 deletions internal/adapters/db/like_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
10 changes: 6 additions & 4 deletions internal/adapters/db/log_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand Down
37 changes: 37 additions & 0 deletions internal/adapters/db/log_repo_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
2 changes: 1 addition & 1 deletion internal/adapters/db/project_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/domain/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading