golem15/summercms
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(lagoon): check min and max as length on every non-numeric field (quick-261104-fx1)

Browse Source 
- min and max compare the characters of a string or a number's decimal
  form and the element count of a slice or map on any field without
  numeric or integer (Laravel's getSize), with the min.array and
  max.array messages for lists
- blank values, booleans, dates and byte slices are not measured; a
  malformed bound is an error on every non-numeric field
- reverse the 261104-f4x pin, prove it through the cabana settings save
  sequence, and update the README, docs and the storePassword comment
This commit is contained in:
Jakub Zych
2026-11-04 11:42:15 +01:00
parent 0785336a50
commit 335f8e608e
6 changed files with 177 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
modules/cabana/backend_users_admin.go
View File

@@ -868,9 +868,10 @@ const adminPasswordMinChars = 4
// storePassword hashes a typed password onto user and reports whether one
// was typed. A password under adminPasswordMinChars characters or over
// bcrypt's 72-byte limit is a 422 on the field. The length is checked here
// because the form's min:4 rule is a string rule, which the model validator
// does not apply.
// bcrypt's 72-byte limit is a 422 on the field. The form's min:4 rule
// already refuses a shorter password in lagoon.Validate before this hook
// runs; the hook repeats the check so it never hashes one, and adds bcrypt's
// 72-byte limit.
func (c usersAdmin) storePassword(ctx context.Context, user *BackendUser) (bool, error) {
plain := submittedPassword(ctx)
if plain == "" {

17
modules/cabana/settings_string_url_test.go
View File

@@ -14,25 +14,28 @@ import (
// stringURLSettings is a settings model whose rules use the Laravel string
// and url tokens the way plugin settings models do (title, feed URL,
// language code).
// language code); one rule (code) has no string token, so its max is a
// length all the same.
type stringURLSettings struct {
ID uint `gorm:"column:id;primaryKey"`
Title *string `gorm:"column:title"`
Website *string `gorm:"column:website"`
Language string `gorm:"column:language"`
Code *string `gorm:"column:code"`
}
func (stringURLSettings) TableName() string { return "cabana_string_url_settings" }
func (stringURLSettings) Fillable() []string { return []string{"title", "website", "language"} }
func (stringURLSettings) Fillable() []string { return []string{"title", "website", "language", "code"} }
func (stringURLSettings) Rules() map[string]string {
return map[string]string{
"title": "nullable|string|max:255",
"website": "nullable|url|max:500",
"language": "nullable|string|max:10",
"code": "nullable|max:5",
}
}
const stringURLSettingsFields = "fields:\n title:\n type: text\n website:\n type: text\n language:\n type: text\n"
const stringURLSettingsFields = "fields:\n title:\n type: text\n website:\n type: text\n language:\n type: text\n code:\n type: text\n"
func compileStringURLSetting(t *testing.T) *CompiledSetting {
t.Helper()
@@ -47,8 +50,9 @@ func compileStringURLSetting(t *testing.T) *CompiledSetting {
// TestSettingsStringURLRules runs the validation sequence of
// SettingsService.Put without a database: the string and url rules are
// accepted, valid, empty and null values pass, and an invalid URL is a
// message on the field.
// accepted, valid, empty and null values pass, and an invalid URL or an
// over-long value (with or without the string token) is a message on the
// field.
func TestSettingsStringURLRules(t *testing.T) {
setting := compileStringURLSetting(t)
cases := []struct {
@@ -61,6 +65,9 @@ func TestSettingsStringURLRules(t *testing.T) {
{"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."}}},
{"code at max", map[string]any{"code": "abcde"}, nil},
{"empty code", map[string]any{"code": ""}, nil},
{"code too long", map[string]any{"code": "abcdef"}, map[string][]string{"code": {"The code may not be greater than 5 characters."}}},
}
for _, tc := range cases {
model := &stringURLSettings{}

2
modules/lagoon/README.md
View File

@@ -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`. 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`.
- 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 field without `integer` or `numeric`, `min` and `max` compare the length in characters of a string or of a number's decimal form (the `lagoon::validation` messages `max.string`, `min.string`) and the element count of a slice or map (`max.array`, `min.array`); an empty or blank value, a boolean and a date are not measured, 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`).

102
modules/lagoon/validate.go
View File

@@ -119,10 +119,11 @@ 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()
// On a field that is not numeric or integer, min and max are a length
// (Laravel's getSize): characters of a string or number, items of a list.
// A malformed bound fails loudly whatever the value, so a typo in Rules()
// cannot disable the limit unnoticed.
lengthRange := (hasString || hasURL) && !hasNumeric && !hasInteger
lengthRange := !hasNumeric && !hasInteger
minLen, maxLen := -1, -1
if lengthRange {
var err error
@@ -148,16 +149,10 @@ func validateField(ctx context.Context, tx *gorm.DB, model any, field, rule stri
if maxArg != "" {
rangeMax = maxArg
}
} else {
if betweenMin != "" {
tags = append(tags, "min="+betweenMin, "max="+betweenMax)
}
if minArg != "" {
tags = append(tags, "min="+minArg)
}
if maxArg != "" {
tags = append(tags, "max="+maxArg)
}
} else if betweenMin != "" {
// The tags keep the between length check below running; the tags
// loop itself skips min and max.
tags = append(tags, "min="+betweenMin, "max="+betweenMax)
}
if numericRange && (rangeMin != "" || rangeMax != "") {
s, ok := numericString(val)
@@ -176,31 +171,30 @@ func validateField(ctx context.Context, tx *gorm.DB, model any, field, rule stri
if required && isEmptyValue(val) {
return []string{validateMessage(ctx, tr, "required", field, nil)}, nil
}
if hasString || hasURL {
// Like Laravel, string and url skip a blank value; required has
// already handled emptiness.
s, isStr := ruleString(val)
if !isEmptyValue(val) && (!isStr || strings.TrimSpace(s) != "") {
if hasString && !isStr {
return []string{validateMessage(ctx, tr, "string", field, nil)}, nil
}
var msgs []string
if hasURL && (!isStr || !urlPattern.MatchString(s)) {
msgs = append(msgs, validateMessage(ctx, tr, "url", field, nil))
}
if lengthRange && isStr {
n := utf8.RuneCountInString(s)
// Like Laravel, string, url, min and max skip a blank value; required
// has already handled emptiness.
s, isStr := ruleString(val)
if !isEmptyValue(val) && (!isStr || strings.TrimSpace(s) != "") {
if hasString && !isStr {
return []string{validateMessage(ctx, tr, "string", field, nil)}, nil
}
var msgs []string
if hasURL && (!isStr || !urlPattern.MatchString(s)) {
msgs = append(msgs, validateMessage(ctx, tr, "url", field, nil))
}
if lengthRange && (minLen >= 0 || maxLen >= 0) {
if n, kind, ok := ruleLength(val); ok {
params := map[string]string{"min": minArg, "max": maxArg}
if minLen >= 0 && n < minLen {
msgs = append(msgs, validateMessage(ctx, tr, "min.string", field, params))
msgs = append(msgs, validateMessage(ctx, tr, "min."+kind, field, params))
}
if maxLen >= 0 && n > maxLen {
msgs = append(msgs, validateMessage(ctx, tr, "max.string", field, params))
msgs = append(msgs, validateMessage(ctx, tr, "max."+kind, field, params))
}
}
if len(msgs) > 0 {
return msgs, nil
}
}
if len(msgs) > 0 {
return msgs, nil
}
}
usedBetween := betweenMin != "" && betweenMax != ""
@@ -408,6 +402,44 @@ func ruleString(val any) (string, bool) {
return rv.String(), true
}
// ruleLength is Laravel's getSize without a numeric rule: the characters
// (runes) of a string, a named string type or a number's decimal form, with
// kind "string", and the element count of a slice or map, with kind "array".
// Pointers and interfaces are dereferenced (a nil one is not measured).
// Booleans, structs (time.Time, Date, TimeOfDay), fixed-size arrays and byte
// slices are not measured: ok is false.
func ruleLength(val any) (n int, kind string, ok bool) {
if val == nil {
return 0, "", false
}
rv := reflect.ValueOf(val)
for rv.Kind() == reflect.Ptr || rv.Kind() == reflect.Interface {
if rv.IsNil() {
return 0, "", false
}
rv = rv.Elem()
}
switch rv.Kind() {
case reflect.String:
return utf8.RuneCountInString(rv.String()), "string", true
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
return utf8.RuneCountInString(strconv.FormatInt(rv.Int(), 10)), "string", true
case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
return utf8.RuneCountInString(strconv.FormatUint(rv.Uint(), 10)), "string", true
case reflect.Float32, reflect.Float64:
return utf8.RuneCountInString(strconv.FormatFloat(rv.Float(), 'f', -1, 64)), "string", true
case reflect.Slice:
if rv.Type().Elem().Kind() == reflect.Uint8 {
return 0, "", false
}
return rv.Len(), "array", true
case reflect.Map:
return rv.Len(), "array", true
default:
return 0, "", false
}
}
func numericString(val any) (string, bool) {
if val == nil {
return "", false
@@ -509,7 +541,7 @@ func validateMessage(ctx context.Context, tr *phrasebook.Translator, rule, field
params["attribute"] = laravelAttribute(field)
key := "lagoon::validate." + rule
switch rule {
case "between.numeric", "min.string", "max.string":
case "between.numeric", "min.string", "max.string", "min.array", "max.array":
key = "lagoon::validation." + rule
}
if tr != nil {
@@ -556,6 +588,10 @@ func validateMessage(ctx context.Context, tr *phrasebook.Translator, rule, field
return "The " + attr + " must be at least " + params["min"] + " characters."
case "max.string":
return "The " + attr + " may not be greater than " + params["max"] + " characters."
case "min.array":
return "The " + attr + " must have at least " + params["min"] + " items."
case "max.array":
return "The " + attr + " may not have more than " + params["max"] + " items."
default:
return "The " + attr + " is invalid."
}

97
modules/lagoon/validate_test.go
View File

@@ -415,9 +415,9 @@ 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.
// TestValidateStringLength: min and max compare the length in characters
// (runes, Laravel's mb_strlen) with the string size messages on every field
// that is not numeric or integer, with or without string or url.
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"
@@ -435,10 +435,8 @@ func TestValidateStringLength(t *testing.T) {
{"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},
{"max without string", "max:5", "0123456789", []string{"The title may not be greater than 5 characters."}},
{"nullable max without string", "nullable|max:5", "0123456789", []string{"The title may not be greater than 5 characters."}},
{"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},
})
@@ -458,3 +456,88 @@ func TestValidateStringLength(t *testing.T) {
t.Fatalf("malformed min: err = %v", err)
}
}
// TestValidateLengthWithoutStringRule: on a field without numeric or integer,
// min and max are Laravel's getSize: the characters of a string or of a
// number's decimal form and the element count of a slice or map. Blank
// values, booleans, dates and byte slices are not measured, and a bound that
// is not a non-negative whole number is an error whatever the value.
func TestValidateLengthWithoutStringRule(t *testing.T) {
empty, long := "", "abcdef"
tooLong := []string{"The title may not be greater than 5 characters."}
runRuleCases(t, "title", []ruleCase{
{"255 polish letters", "required|max:255", strings.Repeat("ż", 255), nil},
{"256 polish letters", "required|max:255", strings.Repeat("ż", 256), []string{"The title may not be greater than 255 characters."}},
{"nil", "nullable|max:5", nil, nil},
{"empty", "nullable|max:5", "", nil},
{"blank", "nullable|max:5", " ", nil},
{"nil pointer", "nullable|max:5", (*string)(nil), nil},
{"pointer to empty", "nullable|max:5", &empty, nil},
{"at max", "nullable|max:5", "abcde", nil},
{"over max", "nullable|max:5", "abcdef", tooLong},
{"pointer over max", "nullable|max:5", &long, tooLong},
{"below min", "min:4", "abc", []string{"The title must be at least 4 characters."}},
{"min empty", "min:4", "", nil},
{"min nil", "min:4", nil, nil},
{"min blank", "min:4", " ", nil},
{"required min empty", "required|min:4", "", []string{"The title field is required."}},
{"email under max", "required|email|max:255", "a@example.com", nil},
{"int digits over max", "max:4", 12345, []string{"The title may not be greater than 4 characters."}},
{"int digits at max", "max:4", 2024, nil},
{"float digits at max", "max:4", float64(2024), nil},
{"float form over max", "max:2", float64(1.5), []string{"The title may not be greater than 2 characters."}},
{"bool not measured", "max:0", true, nil},
{"time not measured", "max:1", time.Date(2026, 11, 4, 12, 0, 0, 0, time.UTC), nil},
{"date not measured", "max:1", NewDate(2026, 11, 4), nil},
})
runRuleCases(t, "tags", []ruleCase{
{"slice over max", "max:2", []string{"a", "b", "c"}, []string{"The tags may not have more than 2 items."}},
{"slice at max", "max:2", []any{"a", "b"}, nil},
{"slice below min", "min:2", []string{"a"}, []string{"The tags must have at least 2 items."}},
{"empty slice skips", "min:2", []string{}, nil},
{"map over max", "max:1", map[string]any{"a": 1, "b": 2}, []string{"The tags may not have more than 1 items."}},
{"bytes not measured", "max:3", []byte("abcdef"), nil},
})
runRuleCases(t, "views", []ruleCase{
{"integer max", "integer|max:10", 11, []string{"The views may not be greater than 10."}},
{"numeric range", "numeric|min:0|max:10", "5", nil},
})
malformed := []struct {
rule string
value any
token string
}{
{"max:abc", "x", "max:abc"},
{"max:abc", nil, "max:abc"},
{"nullable|min:-1", "abc", "min:-1"},
}
for _, tc := range malformed {
_, err := Validate(t.Context(), nil, nil, map[string]string{"title": tc.rule}, map[string]any{"title": tc.value}, nil)
if err == nil || !strings.Contains(err.Error(), tc.token) {
t.Fatalf("%q on %#v: err = %v", tc.rule, tc.value, err)
}
}
}
// TestValidateLengthWithoutStringRuleTranslated: the character and item
// messages are the lagoon::validation lines of the request locale.
func TestValidateLengthWithoutStringRuleTranslated(t *testing.T) {
tr := requestTranslator(t)
cases := []struct {
field, rule string
value any
want string
}{
{"title", "max:5", "abcdef", "title nie może być dłuższy niż 5 znaków."},
{"tags", "max:2", []string{"a", "b", "c"}, "tags nie może mieć więcej niż 2 elementó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)
if err != nil {
t.Fatal(err)
}
if got := errs[tc.field]; len(got) != 1 || got[0] != tc.want {
t.Fatalf("%s %q = %q, want %q", tc.field, tc.rule, got, tc.want)
}
}
}