fix(db): handle usage of special characters in searches (#721)

* handle full text search for failing cases * added test * test getbookmarkcount too * replaceall, fix getbookmarkcount
go-shiori · Sep 11, 2023 · ef1d18d · ef1d18d
1 parent f4817cb
commit ef1d18d
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 36 deletions.
diff --git a/internal/database/database_test.go b/internal/database/database_test.go
@@ -14,18 +14,19 @@ type testDatabaseFactory func(ctx context.Context) (DB, error)
 func testDatabase(t *testing.T, dbFactory testDatabaseFactory) {
 	tests := map[string]databaseTestCase{
 		// Bookmarks
-		"testBookmarkAutoIncrement":       testBookmarkAutoIncrement,
-		"testCreateBookmark":              testCreateBookmark,
-		"testCreateBookmarkWithContent":   testCreateBookmarkWithContent,
-		"testCreateBookmarkTwice":         testCreateBookmarkTwice,
-		"testCreateBookmarkWithTag":       testCreateBookmarkWithTag,
-		"testCreateTwoDifferentBookmarks": testCreateTwoDifferentBookmarks,
-		"testUpdateBookmark":              testUpdateBookmark,
-		"testUpdateBookmarkWithContent":   testUpdateBookmarkWithContent,
-		"testGetBookmark":                 testGetBookmark,
-		"testGetBookmarkNotExistant":      testGetBookmarkNotExistant,
-		"testGetBookmarks":                testGetBookmarks,
-		"testGetBookmarksCount":           testGetBookmarksCount,
+		"testBookmarkAutoIncrement":         testBookmarkAutoIncrement,
+		"testCreateBookmark":                testCreateBookmark,
+		"testCreateBookmarkWithContent":     testCreateBookmarkWithContent,
+		"testCreateBookmarkTwice":           testCreateBookmarkTwice,
+		"testCreateBookmarkWithTag":         testCreateBookmarkWithTag,
+		"testCreateTwoDifferentBookmarks":   testCreateTwoDifferentBookmarks,
+		"testUpdateBookmark":                testUpdateBookmark,
+		"testUpdateBookmarkWithContent":     testUpdateBookmarkWithContent,
+		"testGetBookmark":                   testGetBookmark,
+		"testGetBookmarkNotExistant":        testGetBookmarkNotExistant,
+		"testGetBookmarks":                  testGetBookmarks,
+		"testGetBookmarksWithSQLCharacters": testGetBookmarksWithSQLCharacters,
+		"testGetBookmarksCount":             testGetBookmarksCount,
 		// Tags
 		"testCreateTag":  testCreateTag,
 		"testCreateTags": testCreateTags,
@@ -261,6 +262,36 @@ func testGetBookmarks(t *testing.T, db DB) {
 	assert.Equal(t, savedBookmark.ID, results[0].ID, "bookmark should be the one saved")
 }
 
+func testGetBookmarksWithSQLCharacters(t *testing.T, db DB) {
+	ctx := context.TODO()
+
+	// _ := 0
+	book := model.Bookmark{
+		URL:   "https://github.com/go-shiori/shiori",
+		Title: "shiori",
+	}
+	_, err := db.SaveBookmarks(ctx, true, book)
+	assert.NoError(t, err, "Save bookmarks must not fail")
+
+	characters := []string{";", "%", "_", "\\", "\"", ":"}
+
+	for _, char := range characters {
+		t.Run("GetBookmarks/"+char, func(t *testing.T) {
+			_, err := db.GetBookmarks(ctx, GetBookmarksOptions{
+				Keyword: char,
+			})
+			assert.NoError(t, err, "Get bookmarks should not fail")
+		})
+
+		t.Run("GetBookmarksCount/"+char, func(t *testing.T) {
+			_, err := db.GetBookmarksCount(ctx, GetBookmarksOptions{
+				Keyword: char,
+			})
+			assert.NoError(t, err, "Get bookmarks count should not fail")
+		})
+	}
+}
+
 func testGetBookmarksCount(t *testing.T, db DB) {
 	ctx := context.TODO()
 

diff --git a/internal/database/pg.go b/internal/database/pg.go
@@ -237,13 +237,12 @@ func (db *PGDatabase) GetBookmarks(ctx context.Context, opts GetBookmarksOptions
 	// Add where clause for search keyword
 	if opts.Keyword != "" {
 		query += ` AND (
-			url LIKE :lkw OR
-			title LIKE :kw OR
-			excerpt LIKE :kw OR
-			content LIKE :kw
+			url LIKE '%' || :kw || '%' OR
+			title LIKE '%' || :kw || '%' OR
+			excerpt LIKE '%' || :kw || '%' OR
+			content LIKE '%' || :kw || '%'
 		)`
 
-		arg["lkw"] = "%" + opts.Keyword + "%"
 		arg["kw"] = opts.Keyword
 	}
 
@@ -372,10 +371,10 @@ func (db *PGDatabase) GetBookmarksCount(ctx context.Context, opts GetBookmarksOp
 	// Add where clause for search keyword
 	if opts.Keyword != "" {
 		query += ` AND (
-			url LIKE :lurl OR
-			title LIKE :kw OR
-			excerpt LIKE :kw OR
-			content LIKE :kw
+			url LIKE '%' || :kw || '%' OR
+			title LIKE '%' || :kw || '%' OR
+			excerpt LIKE '%' || :kw || '%' OR
+			content LIKE '%' || :kw || '%'
 		)`
 
 		arg["lurl"] = "%" + opts.Keyword + "%"

diff --git a/internal/database/sqlite.go b/internal/database/sqlite.go
@@ -270,19 +270,22 @@ func (db *SQLiteDatabase) GetBookmarks(ctx context.Context, opts GetBookmarksOpt
 
 	// Add where clause for search keyword
 	if opts.Keyword != "" {
-		query += ` AND (b.url LIKE ? OR b.excerpt LIKE ? OR b.id IN (
+		query += ` AND (b.url LIKE '%' || ? || '%' OR b.excerpt LIKE '%' || ? || '%' OR b.id IN (
 			SELECT docid id
 			FROM bookmark_content
 			WHERE title MATCH ? OR content MATCH ?))`
 
-		args = append(args,
-			"%"+opts.Keyword+"%",
-			"%"+opts.Keyword+"%")
-
-		// Replace dash with spaces since FTS5 uses `-name` as column identifier
-		opts.Keyword = strings.Replace(opts.Keyword, "-", " ", -1)
 		args = append(args, opts.Keyword, opts.Keyword)
 
+		// Replace dash with spaces since FTS5 uses `-name` as column identifier and double quote
+		// since FTS5 uses double quote as string identifier
+		// Reference: https://sqlite.org/fts5.html#fts5_strings
+		ftsKeyword := strings.ReplaceAll(opts.Keyword, "-", " ")
+
+		// Properly set double quotes for string literals in sqlite's fts
+		ftsKeyword = strings.ReplaceAll(ftsKeyword, "\"", "\"\"")
+
+		args = append(args, "\""+ftsKeyword+"\"", "\""+ftsKeyword+"\"")
 	}
 
 	// Add where clause for tags.
@@ -461,19 +464,22 @@ func (db *SQLiteDatabase) GetBookmarksCount(ctx context.Context, opts GetBookmar
 
 	// Add where clause for search keyword
 	if opts.Keyword != "" {
-		query += ` AND (b.url LIKE ? OR b.excerpt LIKE ? OR b.id IN (
+		query += ` AND (b.url LIKE '%' || ? || '%' OR b.excerpt LIKE '%' || ? || '%' OR b.id IN (
 			SELECT docid id
 			FROM bookmark_content
 			WHERE title MATCH ? OR content MATCH ?))`
 
-		args = append(args,
-			"%"+opts.Keyword+"%",
-			"%"+opts.Keyword+"%",
-		)
-
-		// Replace dash with spaces since FTS5 uses `-name` as column identifier
-		opts.Keyword = strings.Replace(opts.Keyword, "-", " ", -1)
 		args = append(args, opts.Keyword, opts.Keyword)
+
+		// Replace dash with spaces since FTS5 uses `-name` as column identifier and double quote
+		// since FTS5 uses double quote as string identifier
+		// Reference: https://sqlite.org/fts5.html#fts5_strings
+		ftsKeyword := strings.ReplaceAll(opts.Keyword, "-", " ")
+
+		// Properly set double quotes for string literals in sqlite's fts
+		ftsKeyword = strings.ReplaceAll(ftsKeyword, "\"", "\"\"")
+
+		args = append(args, "\""+ftsKeyword+"\"", "\""+ftsKeyword+"\"")
 	}
 
 	// Add where clause for tags.