feat(lagoon): check min and max as character length on string and url fields (quick-261104-f4x)

- on a string or url field that is not numeric or integer, min and max
  compare utf8.RuneCountInString (Laravel's mb_strlen) and answer with
  the lagoon::validation min.string / max.string messages
- a min or max argument that is not a whole number is the
  unrecognized-rule error, whatever the value
- rule strings without string or url keep their results (pinned by a
  test); between is unchanged

docs/database/casts-and-validation.md

@@ -108,7 +108,7 @@ fmt.Println(string(out))
 // {"title":["The title field is required."],"views":["The views may not be greater than 1000."]}
 ```
 
-A zero `time.Time`, `lagoon.Date` or `lagoon.TimeOfDay`, or a pointer to one, is empty for `required`, so a required date field with no value fails instead of saving year 1. The supported rules are `required`, `nullable`, `integer`, `numeric`, `between`, `min`, `max`, `in`, `unique`, `boolean`, `email`, `string`, `url`, `confirmed`, `different` and `mimes`. Any other rule is an error, so a rule that SummerCMS does not implement cannot be skipped by accident. `string` accepts a string or a pointer to one, and `url` a URL that matches Laravel's pattern (the same one request validation uses, so `javascript:` links and addresses without a scheme are refused). Both skip a missing, empty or blank value; use `required` to refuse those. On a field that is `integer` or `numeric`, `min`, `max` and `between` compare the number; on other fields they compare the length. A number below the lower bound gets the `min` message, one above the upper bound the `max` message, and a bound that came from `between` gets the numeric `between` message (`The views must be between 0 and 10.`).
+A zero `time.Time`, `lagoon.Date` or `lagoon.TimeOfDay`, or a pointer to one, is empty for `required`, so a required date field with no value fails instead of saving year 1. The supported rules are `required`, `nullable`, `integer`, `numeric`, `between`, `min`, `max`, `in`, `unique`, `boolean`, `email`, `string`, `url`, `confirmed`, `different` and `mimes`. Any other rule is an error, so a rule that SummerCMS does not implement cannot be skipped by accident. `string` accepts a string or a pointer to one, and `url` a URL that matches Laravel's pattern (the same one request validation uses, so `javascript:` links and addresses without a scheme are refused). Both skip a missing, empty or blank value; use `required` to refuse those. On an `integer` or `numeric` field `min`, `max` and `between` compare the number. On a `string` or `url` field `min` and `max` compare the length in characters (`The title may not be greater than 255 characters.`), and a bound that is not a whole number is an error. `between` compares the length on any other field. A number below the lower bound gets the `min` message, one above the upper bound the `max` message, and a bound that came from `between` gets the numeric `between` message (`The views must be between 0 and 10.`).
 
 `unique:<table>` runs a query to check that no other row of the table has the value in the field's column, so it needs a database handle; pass the transaction you are writing in. Soft-deleted rows do not count, and when the model you pass has an ID, its own row does not count either, so the same rules work for create and update. Pass a `phrasebook.Translator` as the last argument to get the messages in the request locale from the `lagoon::validate` catalog; with `nil` they are in English. See [Localization](../services/localization.md).
 

modules/cabana/settings_string_url_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"reflect"
+	"strings"
 	"testing"
 	"testing/fstest"
 
@@ -59,6 +60,7 @@ func TestSettingsStringURLRules(t *testing.T) {
 		{"empty website", map[string]any{"website": ""}, nil},
 		{"null website and title", map[string]any{"website": nil, "title": nil}, nil},
 		{"invalid website", map[string]any{"website": "not a url"}, map[string][]string{"website": {"The website format is invalid."}}},
+		{"title too long", map[string]any{"title": strings.Repeat("a", 256)}, map[string][]string{"title": {"The title may not be greater than 255 characters."}}},
 	}
 	for _, tc := range cases {
 		model := &stringURLSettings{}
@@ -117,4 +119,18 @@ func TestSettingsStringURLRulesPostgres(t *testing.T) {
 	if stored.Website == nil || *stored.Website != feedURL {
 		t.Fatalf("stored website = %v", stored.Website)
 	}
+	_, err = svc.Put(ctx, setting, map[string]any{"title": strings.Repeat("a", 256)})
+	if !errors.As(err, &invalid) {
+		t.Fatalf("long title error = %T %v", err, err)
+	}
+	if got, _ := invalid.Details["title"].([]string); !reflect.DeepEqual(got, []string{"The title may not be greater than 255 characters."}) {
+		t.Fatalf("details = %#v", invalid.Details)
+	}
+	stored = stringURLSettings{}
+	if err := db.First(&stored, 1).Error; err != nil {
+		t.Fatal(err)
+	}
+	if stored.Title == nil || *stored.Title != "Feed" {
+		t.Fatalf("stored title = %v", stored.Title)
+	}
 }

modules/lagoon/README.md

@@ -29,7 +29,7 @@ Use cases:
 - After-commit work: `lagoon.Transaction` runs a function in a transaction and then the callbacks registered with `lagoon.AfterCommit`, in order, only after the commit succeeds; a nested `lagoon.Transaction` is a savepoint whose callbacks are dropped with it when it fails. A nested `lagoon.Transaction` must be given the outer transaction's handle: given a root handle it returns an error without running its function, rather than open an independent transaction whose callbacks would wait on the outer one. A single-statement write for which GORM opens its own implicit transaction runs its callbacks from `lagoon:after_commit` once GORM commits, and never when the write fails. A GORM callback that runs after `lagoon:after_commit` on the same write registers its work too late: the work never runs, and `lagoon.AfterCommit` logs a warning. A callback registered inside a foreign plain GORM transaction is unsafe because Lagoon cannot observe its commit, so `lagoon.AfterCommit` warns and skips it. Outside a transaction, callbacks run immediately. The handle a supported callback receives always has an empty statement on the connection its work belongs to. A panicking callback is logged and never turns a committed write into an error.
 - Per-plugin migrations: `lagoon.Migrate` runs the framework's `system_files` set (`attach.Migrations`), backend admin identity set (`lagoon.BackendAdminMigrations`), `deferred_bindings` set (`lagoon.DeferredBindingMigrations`, under the `lagoon.DeferredHistoryID` history) and job-queue set (`lagoon.QueueMigrations`: River's schema pinned at `lagoon.RiverSchemaVersion`, then the `lagoon.JobsTable` record table, under the `lagoon.QueueHistoryID` history), then every `pact.HasMigrations` set in plugin activation order, each in its own `summer_migrations_<plugin_id>` history table (`lagoon.HistoryTableName`). `lagoon.RollbackLast` and `lagoon.Status` cover rollback and history.
 - Mass assignment: `lagoon.Fill` copies only allow-listed keys onto a model by GORM column name and silently drops the rest, logging each dropped key once outside production. A `json.Number` (from a decoder using `UseNumber`) fills integer, unsigned and float fields. A value that does not fit its column (a fraction, an exponent or an overflow for an integer field, or a value of the wrong type) is a `lagoon.FillTypeError` naming the key, so a caller can answer it as a validation failure on that field. `lagoon.HasFillable` and `lagoon.HasHidden` are the Go forms of `$fillable` and `$hidden`.
-- Validation: `lagoon.Validate` (where `required` fails on a zero date or time) accepts Laravel-style rule strings (`required`, `nullable`, `integer`, `numeric`, `between`, `min`, `max`, `in`, `unique`, `boolean`, `email`, `string`, `url`, `confirmed`, `different`, `mimes`) and returns a field-to-messages map, translated through phrasebook when a translator is given. Unknown rule tokens are an error. `string` requires a string value (a pointer to one included) and `url` a URL matching Laravel's pattern, the one `lagoon.ValidateRequest` uses; both skip an empty or blank value, which is left to `required`. A failed numeric range reports the bound that failed: the `min` message below the lower bound, the `max` message above the upper one, and the numeric `between` message when the bound came from `between`.
+- Validation: `lagoon.Validate` (where `required` fails on a zero date or time) accepts Laravel-style rule strings (`required`, `nullable`, `integer`, `numeric`, `between`, `min`, `max`, `in`, `unique`, `boolean`, `email`, `string`, `url`, `confirmed`, `different`, `mimes`) and returns a field-to-messages map, translated through phrasebook when a translator is given. Unknown rule tokens are an error. `string` requires a string value (a pointer to one included) and `url` a URL matching Laravel's pattern, the one `lagoon.ValidateRequest` uses; both skip an empty or blank value, which is left to `required`. On a `string` or `url` field `min` and `max` compare the length in characters with the `lagoon::validation` string size messages (`max.string`, `min.string`), and a bound that is not a whole number is an error. A failed numeric range reports the bound that failed: the `min` message below the lower bound, the `max` message above the upper one, and the numeric `between` message when the bound came from `between`.
 - Request validation: `lagoon.ValidateRequest` reproduces Laravel 9 request validation for ported API endpoints, so a 422 body matches the PHP one message for message. It takes the decoded input and an ordered `lagoon.RequestRule` table (attribute names may hold `*` wildcards, expanded against the input to `posts.0.title`), runs the rules of each attribute in order and stops an attribute after a failed implicit rule (`required`, `present`, `filled`, `accepted`) or, under `bail`, after any failure. A non-implicit rule is skipped for an absent attribute, a blank string, a null value under `nullable` and an absent key under `sometimes`. Supported rules: `required`, `present`, `filled`, `accepted`, `nullable`, `sometimes`, `bail`, `array`, `string`, `integer`, `numeric`, `boolean`, `email` (PHP `FILTER_VALIDATE_EMAIL`, WinterCMS's default), `url`, `date`, `after`, `after_or_equal`, `before`, `before_or_equal` (a date, a relative word such as `tomorrow`, or another field), `exists:table,column`, `regex`, `not_regex`, `in`, `not_in`, `file`, `image`, `mimes`, `min`, `max`, `size`, `between` and `prohibited` (fails for any value `required` would accept, `0` and `false` included; not implicit, so an absent key, `null`, a blank string and an empty array pass, and with no catalog line its message is the key `validation.prohibited`), plus closure rules built with `lagoon.CustomRule`. The size rules compare the number under `numeric` or `integer` (exactly, as decimals), the element count of an array, kilobytes of a `lagoon.UploadedFile`, and otherwise the length in characters, and pick the matching message. Messages come from the `lagoon::validation` catalog in the request locale; `lagoon.ErrorKeys` gives the attribute order of PHP's message bag.
 - Safe ordering: `lagoon.OrderBy` appends an ORDER BY only for an allow-listed column and an `asc` or `desc` direction, and `lagoon.Collate` adds a validated `COLLATE` clause for language-specific text order (for example the ICU collation `pl-x-icu`); lagoon puts no requirement on the database's default locale.
 - Pagination: `lagoon.Paginate` builds a `lagoon.Page` with `data` and `meta` (`current_page`, `last_page`, `per_page`, `total`).

modules/lagoon/validate.go

@@ -10,6 +10,7 @@ import (
 	"strconv"
 	"strings"
 	"time"
+	"unicode/utf8"
 
 	"git.golem15.com/golem15/summercms/modules/phrasebook"
 	"github.com/go-playground/validator/v10"
@@ -118,6 +119,20 @@ func validateField(ctx context.Context, tx *gorm.DB, model any, field, rule stri
 			return nil, fmt.Errorf("lagoon: unrecognized validation rule %q", tok)
 		}
 	}
+	// On a string or url field min and max are a length in characters. A
+	// malformed bound fails loudly whatever the value, so a typo in Rules()
+	// cannot disable the limit unnoticed.
+	lengthRange := (hasString || hasURL) && !hasNumeric && !hasInteger
+	minLen, maxLen := -1, -1
+	if lengthRange {
+		var err error
+		if minLen, err = lengthBound("min", minArg); err != nil {
+			return nil, err
+		}
+		if maxLen, err = lengthBound("max", maxArg); err != nil {
+			return nil, err
+		}
+	}
 	if nullable && isEmptyValue(val) && !required {
 		return nil, nil
 	}
@@ -173,6 +188,16 @@ func validateField(ctx context.Context, tx *gorm.DB, model any, field, rule stri
 			if hasURL && (!isStr || !urlPattern.MatchString(s)) {
 				msgs = append(msgs, validateMessage(ctx, tr, "url", field, nil))
 			}
+			if lengthRange && isStr {
+				n := utf8.RuneCountInString(s)
+				params := map[string]string{"min": minArg, "max": maxArg}
+				if minLen >= 0 && n < minLen {
+					msgs = append(msgs, validateMessage(ctx, tr, "min.string", field, params))
+				}
+				if maxLen >= 0 && n > maxLen {
+					msgs = append(msgs, validateMessage(ctx, tr, "max.string", field, params))
+				}
+			}
 			if len(msgs) > 0 {
 				return msgs, nil
 			}
@@ -350,6 +375,19 @@ func isIntegerValue(val any) bool {
 	}
 }
 
+// lengthBound parses a min or max argument used as a character length; an
+// absent one is -1 and a malformed one the unrecognized-rule error.
+func lengthBound(name, arg string) (int, error) {
+	if arg == "" {
+		return -1, nil
+	}
+	n, err := strconv.Atoi(strings.TrimSpace(arg))
+	if err != nil || n < 0 {
+		return -1, fmt.Errorf("lagoon: unrecognized validation rule %q", name+":"+arg)
+	}
+	return n, nil
+}
+
 // ruleString answers the string a string or url rule checks: it
 // dereferences pointers and interfaces (a nil one is no string) and accepts
 // any value whose kind is string, named string types included.
@@ -470,8 +508,9 @@ func validateMessage(ctx context.Context, tr *phrasebook.Translator, rule, field
 	}
 	params["attribute"] = laravelAttribute(field)
 	key := "lagoon::validate." + rule
-	if rule == "between.numeric" {
-		key = "lagoon::validation.between.numeric"
+	switch rule {
+	case "between.numeric", "min.string", "max.string":
+		key = "lagoon::validation." + rule
 	}
 	if tr != nil {
 		s := tr.Get(ctx, key, params)
@@ -513,6 +552,10 @@ func validateMessage(ctx context.Context, tr *phrasebook.Translator, rule, field
 		return "The " + attr + " must be a string."
 	case "url":
 		return "The " + attr + " format is invalid."
+	case "min.string":
+		return "The " + attr + " must be at least " + params["min"] + " characters."
+	case "max.string":
+		return "The " + attr + " may not be greater than " + params["max"] + " characters."
 	default:
 		return "The " + attr + " is invalid."
 	}

modules/lagoon/validate_test.go

@@ -401,6 +401,8 @@ func TestValidateStringURLTranslated(t *testing.T) {
 	}{
 		{"title", "nullable|string|max:255", 5, "title musi być ciągiem znaków."},
 		{"website", "nullable|url|max:500", "not a url", "Format website jest nieprawidłowy."},
+		{"title", "nullable|string|max:255", strings.Repeat("a", 256), "title nie może być dłuższy niż 255 znaków."},
+		{"title", "string|min:3", "ab", "title musi mieć przynajmniej 3 znaków."},
 	}
 	for _, tc := range cases {
 		errs, err := Validate(inLocale("pl"), nil, nil, map[string]string{tc.field: tc.rule}, map[string]any{tc.field: tc.value}, tr)
@@ -412,3 +414,47 @@ func TestValidateStringURLTranslated(t *testing.T) {
 		}
 	}
 }
+
+// TestValidateStringLength: on a string or url field min and max compare
+// the length in characters (runes, Laravel's mb_strlen) with the string
+// size messages; rule strings without string or url keep their old results.
+func TestValidateStringLength(t *testing.T) {
+	const longURL = "https://example.com/a-very-long-path/feed.png"
+	const longWords = "this is not a url and it is far too long"
+	if len(longWords) != 40 {
+		t.Fatalf("fixture length %d", len(longWords))
+	}
+	runRuleCases(t, "title", []ruleCase{
+		{"255 ascii", "nullable|string|max:255", strings.Repeat("a", 255), nil},
+		{"256 ascii", "nullable|string|max:255", strings.Repeat("a", 256), []string{"The title may not be greater than 255 characters."}},
+		{"255 polish letters", "nullable|string|max:255", strings.Repeat("ż", 255), nil},
+		{"256 polish letters", "nullable|string|max:255", strings.Repeat("ż", 256), []string{"The title may not be greater than 255 characters."}},
+		{"below min", "string|min:3", "ab", []string{"The title must be at least 3 characters."}},
+		{"at min", "string|min:3", "abc", nil},
+		{"min empty", "string|min:3", "", nil},
+		{"min nil", "string|min:3", nil, nil},
+		{"required min empty", "required|string|min:3", "", []string{"The title field is required."}},
+		{"non-string with max", "string|max:3", 12345, []string{"The title must be a string."}},
+		// Pinned: without string or url, min and max are not checked on a
+		// non-numeric field (unchanged behaviour, see the follow-up).
+		{"max without string", "max:5", "0123456789", nil},
+		{"nullable max without string", "nullable|max:5", "0123456789", nil},
+		{"string between keeps its check", "string|between:1,5", "0123456789", []string{"The title must be between 1 and 5 characters."}},
+		{"string between passes", "string|between:1,5", "abc", nil},
+	})
+	runRuleCases(t, "website", []ruleCase{
+		{"short url", "nullable|url|max:30", "https://example.com/feed.png", nil},
+		{"long url", "nullable|url|max:30", longURL, []string{"The website may not be greater than 30 characters."}},
+		{"long non-url", "nullable|url|max:30", longWords, []string{"The website format is invalid.", "The website may not be greater than 30 characters."}},
+	})
+	for _, v := range []any{"x", nil} {
+		_, err := Validate(t.Context(), nil, nil, map[string]string{"title": "string|max:abc"}, map[string]any{"title": v}, nil)
+		if err == nil || !strings.Contains(err.Error(), "max:abc") {
+			t.Fatalf("malformed max on %#v: err = %v", v, err)
+		}
+	}
+	_, err := Validate(t.Context(), nil, nil, map[string]string{"title": "nullable|string|min:x"}, map[string]any{"title": "abc"}, nil)
+	if err == nil || !strings.Contains(err.Error(), "min:x") {
+		t.Fatalf("malformed min: err = %v", err)
+	}
+}