Skip to content

A Url validation rule, and per-call-site messages - #176

Merged
anilcancakir merged 2 commits into
masterfrom
feat/url-rule-and-custom-messages
Sep 20, 2026
Merged

anilcancakir merged 2 commits into
masterfrom
feat/url-rule-and-custom-messages

Conversation

@anilcancakir

Copy link
Copy Markdown
Member

Two Laravel parity gaps, both found by trying to move a consumer's credential form onto the rules and discovering the move would be a downgrade.

Url

Laravel has url; this package did not, so every consumer validating a typed-in endpoint wrote the same startsWith('http://') pair by hand and each drew its own conclusion about a scheme it had not thought of.

'website': [Required(), Url()],                    // http or https
'webhook': [Required(), Url(schemes: ['https'])],  // https only

It checks a scheme from its allowlist and a non-empty host, and nothing else. It reaches no network and resolves no host, because a rule answering a form field synchronously cannot know any of that, and a rule that pretended to would be wrong in the direction that blocks a valid address.

The allowlist is the security-shaped half. Uri.parse accepts javascript:alert(1) and file:///etc/passwd without complaint; both have a scheme and both parse cleanly. Nothing else keeps them out of a field whose value becomes a request or a link.

Whitespace is rejected before parsing, because Uri does not treat it as an error. Measured: Uri.tryParse('http://exa mple.com') succeeds and percent-encodes the space into the host as exa%20mple.com, an address no DNS lookup can resolve. Laravel rejects it too, via FILTER_VALIDATE_URL.

messages

A rule's message came from its own key and nothing else, so a screen wanting Şifre gerekli. rather than the catalogue's generic :attribute alanı zorunludur. had to abandon the rules and hand-roll a closure, which is the thing the rules exist to prevent. This is Laravel's third Validator::make argument.

FormValidator.rules(
  [Required(), Url()],
  field: 'address',
  messages: {
    'required': 'provider.error.address_required',
    'url': 'provider.error.address_scheme',
  },
)

The value is a key, not a finished sentence. An override taking a sentence would make every consumer using it monolingual, which is the opposite of what the rules are for. Rule parameters still reach it, so :attribute and :schemes work in an override, and a key with no sentence renders as itself, which is trans's own contract.

The one decision worth reviewing

The map is keyed by a new Rule.name, derived from the rule's message key (validation.required gives required) rather than from runtimeType.

That is load-bearing. runtimeType.toString() is not a dependable identifier in a release build, so a messages map keyed on it would match in development and silently stop matching in production. Two first-party sources:

  • dart2js minifies class names and carries a branch for reporting it: if (JS_GET_FLAG('MINIFIED')) return 'minified:$rawClassName'; (dart-sdk/lib/_internal/js_runtime/lib/js_helper.dart:107).
  • Flutter's own framework declines to use it outside asserts. objectRuntimeType (foundation/object.dart) returns runtimeType.toString() only when asserts are enabled and a caller-supplied constant otherwise, because "calling toString on a runtime type is a non-trivial operation".

A message key is a literal in the source and survives both.

Also

Rule gains a const constructor and the four stateless rules (Required, Email, Accepted, Url) declare one. Additive: a rule with its own non-const constructor is unaffected. A stateless rule is written inline in a widget's build, where a const instance is one allocation that never happens again.

Gates

  • dart analyze — no issues
  • dart format . — no diff
  • flutter test — 1580 green, 18 of them new
  • Three mutations run: removing the whitespace guard, the empty-host guard, and the messages lookup each turn their own tests red
  • Post-change sync: CHANGELOG.md, doc/digging-deeper/validation.md (two new sections plus TOC and anchors), skills/magic-framework/references/forms-validation.md, SKILL.md version bumped

Two Laravel parity gaps, both found while trying to move a consumer's
credential form onto the rules and discovering the move would be a
downgrade.

Url: Laravel has it and this package did not, so every consumer validating a
typed-in endpoint wrote the same startsWith('http://') pair by hand and each
drew its own conclusion about a scheme it had not thought of. It checks a
scheme from its allowlist and a non-empty host, and nothing else: it reaches
no network and resolves no host, because a rule answering a form field
synchronously cannot know any of that.

The allowlist is the security-shaped half. Uri.parse accepts
javascript:alert(1) and file:///etc/passwd without complaint; both have a
scheme and both parse cleanly, and nothing else keeps them out of a field
whose value becomes a request or a link.

Whitespace is rejected before parsing, because Uri does not treat it as an
error: Uri.tryParse('http://exa mple.com') succeeds and percent-encodes the
space into the host as exa%20mple.com, which no DNS lookup can resolve.
Measured, and Laravel rejects it too via FILTER_VALIDATE_URL.

messages: a rule's message came from its own key and nothing else, so a
screen wanting 'Şifre gerekli.' rather than the catalogue's generic
':attribute alanı zorunludur.' had to abandon the rules and hand-roll a
closure, which is the thing the rules exist to prevent.

The value is a KEY rather than a sentence: an override taking a sentence
would make every consumer using it monolingual. Rule parameters still reach
it, so :attribute and :schemes work in an override.

Keyed by a new Rule.name derived from the message key rather than from
runtimeType, and that choice is load-bearing: runtimeType.toString() is
minified in a Flutter web release build, so a messages map keyed on it would
match in development and silently stop matching in production.

Rule gains a const constructor and the four stateless rules declare one.
Additive; a rule with its own non-const constructor is unaffected.

Three mutations run: removing the whitespace guard, the empty-host guard, and
the messages lookup each turn their own tests red.
I asserted that runtimeType.toString() is minified in a Flutter web release
build without a source, in four places. Two first-party ones say something
stronger and narrower:

dart2js minifies class names and carries a branch for reporting them,
if (JS_GET_FLAG('MINIFIED')) return 'minified:$rawClassName'
(dart-sdk/lib/_internal/js_runtime/lib/js_helper.dart:107).

And Flutter's framework declines to use it at all outside asserts:
objectRuntimeType (foundation/object.dart) returns runtimeType.toString()
only when asserts are enabled and a caller-supplied constant otherwise,
because 'calling toString on a runtime type is a non-trivial operation'.

So the honest claim is that it is not a dependable identifier in a release
build, which is broader than web and better evidenced than what I wrote.
@codecov

codecov Bot commented Sep 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@kodizm

kodizm Bot commented Sep 20, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

The Url rule is sound and well covered; the messages map's key derivation silently misses Min and Max, and the override is not reachable from the two entry points consumers actually use.

Major

lib/src/validation/contracts/rule.dart:75name takes the segment after the last dot of message(), but Min and Max return 'validation.min.$_type' (lib/src/validation/rules/min.dart:58, max.dart:58). So Min(8).name is 'string', 'numeric' or 'list' — never 'min'. A caller writing the documented messages: {'min': 'x.y'} gets a silent no-op, and the key that would match changes with the value's runtime type, because _type is only assigned inside passes() and rule.name is read after it (form_validator.dart:130). The existing test/validation/type_message_test.dart confirms the keys are validation.min.string / .numeric / .list. Either Min/Max should override name, or name should take the segment after validation.. (correctness)

Minor

lib/src/ui/magic_view.dart:205MagicView.rules() is the wrapper the docs recommend inside MagicForm, and it forwards field, extraData and _controller but not messages. A consumer on that path cannot reach the new override at all without dropping to FormValidator.rules and losing the automatic controller injection.

lib/src/validation/validator.dart:64Validator.make takes no messages, so the parity claimed in CHANGELOG.md and doc/digging-deeper/validation.md ("Laravel's third Validator::make argument") holds only for the Flutter form path. FormRequest / validate() callers still get the catalogue message. Worth saying so in the doc rather than implying full parity.

assets/stubs/install/lang_en.stub:11 — the shipped validation block gains no url entry, so a freshly scaffolded app renders the raw key validation.url on the first failure. The stub already carries required, email, min, max, confirmed, so this is the one new rule breaking that set. A line like "url": "The :attribute must start with :schemes." keeps it whole.

test/validation/url_rule_test.dart:64 — the comment says "Uri.parse throws rather than returning a bad Uri" for [external link removed] mple.com, which contradicts the rule's own docstring and is measurably false: Uri.tryParse('[external link removed] mple.com') returns scheme=http host=exa%20mple.com. Only '::::' returns null. The test asserts the right thing; the comment explains it wrongly. (maintainability)

Tests

url_rule_test.dart covers the accept set, the reject set, scheme narrowing/widening, case-insensitivity and the params map; form_validator_messages_test.dart covers override, fall-through, per-rule keying and a stale key. Nothing covers Rule.name for a multi-segment message key, which is where the defect above lives, nor MagicView.rules.

Checks I ran

  • flutter analyze --no-fatal-infosNo issues found! (ran in 27.9s), exit 0
  • flutter test test/validationAll tests passed! (137), exit 0 (full suite not run)
  • dart format --set-exit-if-changed .354 files (0 changed), exit 0
  • dart probe of Uri.tryParse on the rule's edge inputs, for the finding above

CHANGELOG/doc/skill patches read; no other file in the pull request was left unreviewed.

@anilcancakir
anilcancakir merged commit bdd66cb into master Sep 20, 2026
6 checks passed
anilcancakir added a commit that referenced this pull request Sep 20, 2026
…atter (#177)

The stamp comment said Skill v0.1.31 while the frontmatter said 0.1.34. The
review that spotted it called the drift pre-existing; it is not. At the 0.0.14
release both read 0.1.31, and the three bumps that opened the gap are mine:
#174, #176 and #175 each raised the frontmatter and left the comment alone.

The stamp is what a reader checks to see whether the skill was verified
against the current API surface, so one that lags by three revisions says the
opposite of what it is for.
@anilcancakir anilcancakir mentioned this pull request Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant