Make array length: a bound, not just a report

[VSN2015]

The most consequential of the bug fixes so far. Found by asking "does the length: bound actually stop anything?"

The bug

length: recorded its violation and then cast, checked and reported on every element anyway. So an oversized array cost far more to reject than a valid one costs to accept.

200,000 elements against array :tags, of: :string, length: 0..10:

	Before	After
Violations returned	200,001	1
Error response body	~9.5 MB	~40 bytes
Contract CPU	~9.2 s	~57 ms

That is a request the contract refused on its very first check, failing against the exact bound a developer declares to prevent this.

The 422 body was itself the amplification: one small request in, a multi-megabyte response out, with every violation individually constructed and message-resolved along the way.

The fix

An array outside its length: bound is rejected whatever its contents, so examining those contents can only add cost and noise. It now returns immediately:

if field[:length] && !Coercion.length_ok?(field[:length], value.length)
  violations << permittable_violation(field, path, "length")
  return nil
end

On the measurement above, the remaining wall time is the HashWithIndifferentAccess conversion of the payload itself (901 ms of the 958 ms) — that happens before any field is examined, and a real Rails request has already paid it during parameter parsing. The contract's own work went from ~8.3 s to ~57 ms.

One consequence worth knowing

validate: and transform: are no longer handed an array the contract has already rejected. That's the same rule transform: already followed for element violations, now applied to the count too — the existing comment says it best: "a partially-nil one would hand user code garbage it never agreed to see."

What doesn't change

• Arrays within their bounds still report every element violation, with indices.
• Arrays with no length: declared behave exactly as before.
• Valid arrays are untouched.

The :json field's length:/max_depth: already worked this way (returning on the first failed bound), so this brings arrays in line with it.

Also

The README now says plainly that length: is a bound and advises declaring it on every array — it's the only thing standing between a client and however much work your contract is willing to do.

Verification

• 205 examples, 0 failures (6 new, written before the fix): scalar arrays, arrays of nested hashes where every element would otherwise violate loudly, validate:/transform: not being called, in-bounds arrays unchanged, no-length: arrays unchanged, valid arrays still valid
• RuboCop clean

[VSN2015]

Code review — #24 Array length: as a bound

Verdict: mergeable with fixes. The code change is correct, minimal, and verified end to end. What needs fixing is around it: the README's new "declare length: on every array" advice is contradicted by the README's own example directly beneath it and by what the generator drafts, and the PR body cites a :json/max_depth precedent that does not exist in this repo.

Verified

• bundle exec rspec at decaa0c: 205 examples, 0 failures. rubocop: no offenses.
• Before/after measured on base 228d14d vs head:
  • array :tags, of: :string, length: 0..10, 200,000 hash elements, monitor mode: base 200,001 violations in 8.5 s; head 1 violation in 0.95 s (all of it HWIA conversion of the payload).
  • array :items, length: 0..10 do ... end, 50,000 {} elements: base 100,001 violations, 4.58 MB body, 4.5 s; head 1 violation, 35 B body, 173 ms.
  • Real ActionController::Parameters (actionpack 8.1.3.1), 200k strings: head 1 violation, 422, 164 ms. to_unsafe_h + HWIA conversion happen in permittable_plain_params (lib/permittable.rb:892-896) before any field is examined; the length check is O(1) on the already-converted array.
• Bound shapes all correct: 2.., ..2, exact 3, 0, 1..5 with [], exclusive 0...3.
• Nested arrays and arrays inside array-of-hash elements short-circuit correctly at depth.
• The new nil return is safe: result[key] = nil at :940 is never surfaced because enforce raises (:798), monitor returns source (:812-821), Contract#call returns params: nil, and finalize is gated on violations.empty? (:791).

Strengths

• lib/permittable.rb:947-957: the guard sits in permittable_check_array, the single path every array takes (sole caller :940). Nested fields, array-of-hash elements and standalone Permittable::Contract all inherit it.
• The comment at :948-953 explains why (a bound, not a report), not what.
• spec/permittable_spec.rb:547-597: six specs cover both element kinds, assert validate:/transform: are not invoked, and pin the three unchanged cases. Real objects through FakeController, exact details equality.
• CHANGELOG.md:6 is candid that payload conversion still dominates wall time. My measurements agree.

Important

1. README.md:174 vs README.md:177-181. The paragraph ends "Declare length: on every array you accept"; the very next code block declares array :line_items, required: true do with no length:. Add a bound to the example.
2. lib/permittable/generator.rb:192 still drafts unbounded arrays with no hint. It emits array :#{name}, of: :string # TODO: confirm the element type (pinned by spec/generator_spec.rb:204, shown at README.md:473). The generator is the adoption on-ramp, and its own rule is "everything it cannot know stays visible". Extend the TODO to # TODO: confirm the element type and declare length: and update the spec regex and README:473.

Minor

• PR body: "The :json field's length:/max_depth: already worked this way". There is no :json type on master (SCALAR_TYPES, lib/permittable.rb:136) and max_depth appears nowhere. Drop the sentence (or it belongs in Add the :json field type for free-form hashes #15's description).
• CHANGELOG.md:6 "200,001 violations" holds only when the 200k elements are also non-strings; with valid strings base returned 1 violation while still spending ~1.1 s casting. Say "200,000 non-string elements" or use the array-of-hashes case.
• Say explicitly that there is no default cap: an array without length: is unbounded exactly as before. A reader skimming the CHANGELOG could assume the fix covers arrays it does not touch. An app-wide Permittable.default_array_length would be a reasonable follow-up.
• lib/permittable.rb:955: the single violation carries no count or bound, so the client cannot tell too-few from too-many without the JSON Schema. Pre-existing shape; follow-up.
• lib/permittable.rb:550-560: an array default: is not checked against length:. array :t, of: :string, length: 0..1, default: %w[a b] loads and hands the action an out-of-bounds default, on base and head. README.md:194 says defaults are validated against the field's contract at class load. One-line fix in validate_array_authored_value!.
• No re-runnable benchmark for the headline numbers; benchmark/overhead.rb measures a valid payload only. A short benchmark/oversized_array.rb would let the claim be re-checked.
• README.md:174 "a few milliseconds instead of seconds": the whole call is 150 to 950 ms on my machine because payload conversion dominates. "Milliseconds of contract work instead of seconds" is accurate.

[VSN2015]

Review of PR #24: High-impact performance and DoS hardening fix. Short-circuiting on array length violations prevents runaway CPU and memory usage from oversized payloads.

[VSN2015]

Returning early with a single length violation when !Coercion.length_ok? prevents allocating thousands of unnecessary element violation objects for a payload that is already rejected.

[VSN2015]

Asserting that validate: and transform: callbacks do not execute on rejected oversized arrays ensures consistent lifecycle behavior with invalid payloads.