added placeholder params support for Dao.FindRecordsByFilter and Dao.…

…FindFirstRecordByFilter

CHANGELOG.md

@@ -1,7 +1,7 @@
 ## v0.18.0 - WIP
 
 - Added new `SmtpConfig.LocalName` option to specify a custom domain name (or IP address) for the initial EHLO/HELO exchange ([#3097](https://github.com/pocketbase/pocketbase/discussions/3097)).
-  _This is usually required for verification purposes only by some SMTP providers, such as Gmail SMTP-relay._
+  _This is usually required for verification purposes only by some SMTP providers, such as on-premise [Gmail SMTP-relay](https://support.google.com/a/answer/2956491)._
 
 - Added cron expression macros ([#3132](https://github.com/pocketbase/pocketbase/issues/3132)):
   ```
@@ -14,9 +14,30 @@
   "@hourly":   "0 * * * *"
   ```
 
-- Added JSVM `$mails.*` binds for sending.
+- To minimize the footguns with `Dao.FindFirstRecordByFilter()` and `Dao.FindRecordsByFilter()`, the functions now supports an optional placeholder params argument that is safe to be populated with untrusted user input.
+  The placeholders are in the same format as when binding regular SQL parameters.
+  ```go
+  // unsanitized and untrusted filter variables
+  status := "..."
+  author := "..."
+
+  app.Dao().FindFirstRecordByFilter("articles", "status={:status} && author={:author}", dbx.Params{
+    "status": status,
+    "author": author,
+  })
+
+  app.Dao().FindRecordsByFilter("articles", "status={:status} && author={:author}", "-created", 10, 0, dbx.Params{
+    "status": status,
+    "author": author,
+  })
+  ```
+
+- ⚠️ Added offset argument `Dao.FindRecordsByFilter(collection, filter, sort, limit, offset, [params...])`.
+  _If you don't need an offset, you can set it to `0`._
+
+- Added JSVM `$mails.*` binds for the corresponding Go [mails package](https://pkg.go.dev/github.com/pocketbase/pocketbase/mails) functions.
 
-- Fill the `LastVerificationSentAt` and `LastResetSentAt` fields only after a successfull email send.
+- Fill the `LastVerificationSentAt` and `LastResetSentAt` fields only after a successfull email send ([#3121](https://github.com/pocketbase/pocketbase/issues/3121)).
 
 - Reflected the latest JS SDK changes in the Admin UI.
 
@@ -749,7 +770,7 @@
 
   _Note2: The old index (`"field.0":null`) and filename (`"field.filename.png":null`) based suffixed syntax for deleting files is still supported._
 
-- ! Added support for multi-match/match-all request data and collection multi-valued fields (`select`, `relation`) conditions.
+- ⚠️ Added support for multi-match/match-all request data and collection multi-valued fields (`select`, `relation`) conditions.
   If you want a "at least one of" type of condition, you can prefix the operator with `?`.
   ```js
   // for each someRelA.someRelB record require the "status" field to be "active"
@@ -829,7 +850,7 @@
 
 ## v0.10.3
 
-- ! Renamed the metadata key `original_filename` to `original-filename` due to an S3 file upload error caused by the underscore character ([#1343](https://github.com/pocketbase/pocketbase/pull/1343); thanks @yuxiang-gao).
+- ⚠️ Renamed the metadata key `original_filename` to `original-filename` due to an S3 file upload error caused by the underscore character ([#1343](https://github.com/pocketbase/pocketbase/pull/1343); thanks @yuxiang-gao).
 
 - Fixed request verification docs api url ([#1332](https://github.com/pocketbase/pocketbase/pull/1332); thanks @JoyMajumdar2001)
 
@@ -865,7 +886,7 @@
 
 - Refactored the `core.app.Bootstrap()` to be called before starting the cobra commands ([#1267](https://github.com/pocketbase/pocketbase/discussions/1267)).
 
-- ! Changed `pocketbase.NewWithConfig(config Config)` to `pocketbase.NewWithConfig(config *Config)` and added 4 new config settings:
+- ⚠️ Changed `pocketbase.NewWithConfig(config Config)` to `pocketbase.NewWithConfig(config *Config)` and added 4 new config settings:
   ```go
   DataMaxOpenConns int // default to core.DefaultDataMaxOpenConns
   DataMaxIdleConns int // default to core.DefaultDataMaxIdleConns
@@ -875,9 +896,9 @@
 
 - Added new helper method `core.App.IsBootstrapped()` to check the current app bootstrap state.
 
-- ! Changed `core.NewBaseApp(dir, encryptionEnv, isDebug)` to `NewBaseApp(config *BaseAppConfig)`.
+- ⚠️ Changed `core.NewBaseApp(dir, encryptionEnv, isDebug)` to `NewBaseApp(config *BaseAppConfig)`.
 
-- ! Removed `rest.UploadedFile` struct (see below `filesystem.File`).
+- ⚠️ Removed `rest.UploadedFile` struct (see below `filesystem.File`).
 
 - Added generic file resource struct that allows loading and uploading file content from
   different sources (at the moment multipart/form-data requests and from the local filesystem).

daos/record.go

@@ -252,23 +252,31 @@ func (dao *Dao) FindFirstRecordByData(
 // FindRecordsByFilter returns limit number of records matching the
 // provided string filter.
 //
+// NB! Use the last "params" argument to bind untrusted user variables!
+//
 // The sort argument is optional and can be empty string OR the same format
 // used in the web APIs, eg. "-created,title".
 //
 // If the limit argument is <= 0, no limit is applied to the query and
 // all matching records are returned.
 //
-// NB! Don't put untrusted user input in the filter string as it
-// practically would allow the users to inject their own custom filter.
-//
 // Example:
 //
-//	dao.FindRecordsByFilter("posts", "title ~ 'lorem ipsum' && visible = true", "-created", 10)
+//	dao.FindRecordsByFilter(
+//		"posts",
+//		"title ~ {:title} && visible = {:visible}",
+//		"-created",
+//		10,
+//		0,
+//		dbx.Params{"title": "lorem ipsum", "visible": true}
+//	)
 func (dao *Dao) FindRecordsByFilter(
 	collectionNameOrId string,
 	filter string,
 	sort string,
 	limit int,
+	offset int,
+	params ...dbx.Params,
 ) ([]*models.Record, error) {
 	collection, err := dao.FindCollectionByNameOrId(collectionNameOrId)
 	if err != nil {
@@ -286,7 +294,7 @@ func (dao *Dao) FindRecordsByFilter(
 		true,       // allow searching hidden/protected fields like "email"
 	)
 
-	expr, err := search.FilterData(filter).BuildExpr(resolver)
+	expr, err := search.FilterData(filter).BuildExpr(resolver, params...)
 	if err != nil || expr == nil {
 		return nil, errors.New("invalid or empty filter expression")
 	}
@@ -307,6 +315,10 @@ func (dao *Dao) FindRecordsByFilter(
 	resolver.UpdateQuery(q) // attaches any adhoc joins and aliases
 	// ---
 
+	if offset > 0 {
+		q.Offset(int64(offset))
+	}
+
 	if limit > 0 {
 		q.Limit(int64(limit))
 	}
@@ -322,14 +334,17 @@ func (dao *Dao) FindRecordsByFilter(
 
 // FindFirstRecordByFilter returns the first available record matching the provided filter.
 //
-// NB! Don't put untrusted user input in the filter string as it
-// practically would allow the users to inject their own custom filter.
+// NB! Use the last params argument to bind untrusted user variables!
 //
 // Example:
 //
-//	dao.FindFirstRecordByFilter("posts", "slug='test'")
-func (dao *Dao) FindFirstRecordByFilter(collectionNameOrId string, filter string) (*models.Record, error) {
-	result, err := dao.FindRecordsByFilter(collectionNameOrId, filter, "", 1)
+//	dao.FindFirstRecordByFilter("posts", "slug={:slug} && status='public'", dbx.Params{"slug": "test"})
+func (dao *Dao) FindFirstRecordByFilter(
+	collectionNameOrId string,
+	filter string,
+	params ...dbx.Params,
+) (*models.Record, error) {
+	result, err := dao.FindRecordsByFilter(collectionNameOrId, filter, "", 1, 0, params...)
 	if err != nil {
 		return nil, err
 	}

daos/record_test.go

@@ -436,6 +436,8 @@ func TestFindRecordsByFilter(t *testing.T) {
 		filter             string
 		sort               string
 		limit              int
+		offset             int
+		params             []dbx.Params
 		expectError        bool
 		expectRecordIds    []string
 	}{
@@ -445,6 +447,8 @@ func TestFindRecordsByFilter(t *testing.T) {
 			"id != ''",
 			"",
 			0,
+			0,
+			nil,
 			true,
 			nil,
 		},
@@ -454,6 +458,8 @@ func TestFindRecordsByFilter(t *testing.T) {
 			"",
 			"",
 			0,
+			0,
+			nil,
 			true,
 			nil,
 		},
@@ -463,6 +469,8 @@ func TestFindRecordsByFilter(t *testing.T) {
 			"someMissingField > 1",
 			"",
 			0,
+			0,
+			nil,
 			true,
 			nil,
 		},
@@ -472,6 +480,8 @@ func TestFindRecordsByFilter(t *testing.T) {
 			"id != ''",
 			"",
 			0,
+			0,
+			nil,
 			false,
 			[]string{
 				"llvuca81nly1qls",
@@ -485,54 +495,73 @@ func TestFindRecordsByFilter(t *testing.T) {
 			"id != '' && active=true",
 			"-created,title",
 			-1, // should behave the same as 0
+			0,
+			nil,
 			false,
 			[]string{
 				"0yxhwia2amd8gec",
 				"achvryl401bhse3",
 			},
 		},
 		{
-			"with limit",
+			"with limit and offset",
 			"demo2",
 			"id != ''",
 			"title",
 			2,
+			1,
+			nil,
 			false,
 			[]string{
-				"llvuca81nly1qls",
 				"achvryl401bhse3",
+				"0yxhwia2amd8gec",
+			},
+		},
+		{
+			"with placeholder params",
+			"demo2",
+			"active = {:active}",
+			"",
+			10,
+			0,
+			[]dbx.Params{{"active": false}},
+			false,
+			[]string{
+				"llvuca81nly1qls",
 			},
 		},
 	}
 
 	for _, s := range scenarios {
-		records, err := app.Dao().FindRecordsByFilter(
-			s.collectionIdOrName,
-			s.filter,
-			s.sort,
-			s.limit,
-		)
-
-		hasErr := err != nil
-		if hasErr != s.expectError {
-			t.Errorf("[%s] Expected hasErr to be %v, got %v (%v)", s.name, s.expectError, hasErr, err)
-			continue
-		}
+		t.Run(s.name, func(t *testing.T) {
+			records, err := app.Dao().FindRecordsByFilter(
+				s.collectionIdOrName,
+				s.filter,
+				s.sort,
+				s.limit,
+				s.offset,
+				s.params...,
+			)
+
+			hasErr := err != nil
+			if hasErr != s.expectError {
+				t.Fatalf("[%s] Expected hasErr to be %v, got %v (%v)", s.name, s.expectError, hasErr, err)
+			}
 
-		if hasErr {
-			continue
-		}
+			if hasErr {
+				return
+			}
 
-		if len(records) != len(s.expectRecordIds) {
-			t.Errorf("[%s] Expected %d records, got %d", s.name, len(s.expectRecordIds), len(records))
-			continue
-		}
+			if len(records) != len(s.expectRecordIds) {
+				t.Fatalf("[%s] Expected %d records, got %d", s.name, len(s.expectRecordIds), len(records))
+			}
 
-		for i, id := range s.expectRecordIds {
-			if id != records[i].Id {
-				t.Errorf("[%s] Expected record with id %q, got %q at index %d", s.name, id, records[i].Id, i)
+			for i, id := range s.expectRecordIds {
+				if id != records[i].Id {
+					t.Fatalf("[%s] Expected record with id %q, got %q at index %d", s.name, id, records[i].Id, i)
+				}
 			}
-		}
+		})
 	}
 }
 
@@ -544,48 +573,62 @@ func TestFindFirstRecordByFilter(t *testing.T) {
 		name               string
 		collectionIdOrName string
 		filter             string
+		params             []dbx.Params
 		expectError        bool
 		expectRecordId     string
 	}{
 		{
 			"missing collection",
 			"missing",
 			"id != ''",
+			nil,
 			true,
 			"",
 		},
 		{
 			"missing filter",
 			"demo2",
 			"",
+			nil,
 			true,
 			"",
 		},
 		{
 			"invalid filter",
 			"demo2",
 			"someMissingField > 1",
+			nil,
 			true,
 			"",
 		},
 		{
 			"valid filter but no matches",
 			"demo2",
 			"id = 'test'",
+			nil,
 			true,
 			"",
 		},
 		{
 			"valid filter and multiple matches",
 			"demo2",
 			"id != ''",
+			nil,
+			false,
+			"llvuca81nly1qls",
+		},
+		{
+			"with placeholder params",
+			"demo2",
+			"active = {:active}",
+			[]dbx.Params{{"active": false}},
 			false,
 			"llvuca81nly1qls",
 		},
 	}
 
 	for _, s := range scenarios {
-		record, err := app.Dao().FindFirstRecordByFilter(s.collectionIdOrName, s.filter)
+		record, err := app.Dao().FindFirstRecordByFilter(s.collectionIdOrName, s.filter, s.params...)
 
 		hasErr := err != nil
 		if hasErr != s.expectError {