From 91ac4286c65ba7ef33dd657599177c82b10307a6 Mon Sep 17 00:00:00 2001 From: Sang Date: Sat, 5 Sep 2026 05:03:22 +0700 Subject: [PATCH] Reject numbers the type cannot faithfully hold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Coercion promises that "a value the type cannot faithfully represent is a violation, not a guess". Four ways it wasn't: :float "1e400" -> Infinity (overflow, accepted) :float "1e-400" -> 0.0 (whole value lost, accepted) :float Float::NAN -> NaN (passed through) :decimal "NaN" -> NaN (a LITERAL STRING from a client) The last is the one that matters most: BigDecimal("NaN") and BigDecimal("Infinity") succeed where Float() raises, so a client could send the string "NaN" for a price field and have it stored. :float rejected exactly those strings while :decimal accepted them — the two types disagreeing is what marks this as accidental rather than designed. Non-finite results are now invalid_type for both types. A genuine zero is unaffected however it is spelled. Underflow is only visible against the source text, since the result is an ordinary 0.0, so a zero result is rejected only when the string named a nonzero SIGNIFICAND — the exponent's digits say nothing about the value. My first attempt scanned the whole string and rejected "0e10", which the spec caught; the check now splits on e/E first. :decimal keeps accepting the large exponents BigDecimal genuinely represents ("1e400" -> 0.1e401), since it has no exponent limit and so cannot overflow. Only NaN and Infinity are non-finite there. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 2 ++ README.md | 2 ++ lib/permittable.rb | 37 +++++++++++++++++++++++-- spec/permittable_spec.rb | 60 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b7a9b4..88f3f2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ - **The error-response schema had drifted from what the server renders.** `ERROR_SCHEMA` is the one hand-written part of the export, and it had fallen behind twice over: a violation on a field with `message:` (or with app I18n copy) carries a third key the schema didn't mention, so clients generating types from it dropped the human-readable copy; and the `code` enumeration never gained `depth`, which a `:json` field's `max_depth:` bound emits. `message` is now documented as an optional property — `required` stays `param` + `code`, since a violation without one keeps the bare shape — and `depth` is listed. A new spec renders a real violation and holds the schema to the envelope, so this half of the "docs cannot drift" claim is now guarded like the request-body half. - **Two controllers on one path and verb silently overwrote each other.** A document cannot carry two operations in one slot, and the second claim replaced the first with no indication anything had been lost. The loser now lands in `x-permittable-controllers`, which is where the exporter already puts an operation it cannot place. - **Every `:datetime` cast raised `NameError` in a host that had not loaded ActiveSupport's time extensions.** A standalone `Permittable::Contract` — validating a webhook payload or a job argument — got `uninitialized constant ActiveSupport::TimeWithZone` instead of a validated param, because activesupport does not load that class by default and the gem never asked for it. A Rails app gets it via `active_support/time` at boot, which is why the spec suite (`require "active_record"`) masked it, the same shape as the 0.5.1 nested-hash bug. Fixed by requiring `active_support/core_ext/time/calculations`, which loads `TimeWithZone` **and** the `Time` extensions it needs: the class alone is not self-sufficient, and converting a real zoned time calls `Time#sec_fraction`, so requiring only `time_with_zone` would have traded `NameError` for `NoMethodError` on activesupport 8.1. The bare-subprocess spec now casts every scalar type, including a real `TimeWithZone`, in a process with no Rails. +- **`:float` and `:decimal` accepted numbers the type cannot faithfully hold, including ones a client controls.** `Float("1e400")` is `Infinity` and `Float("1e-400")` is `0.0` — the first overflows, the second loses the entire value — and both were accepted silently, leaving a value no numeric column can store. `Float::INFINITY` and `Float::NAN` objects passed straight through for both types. Worst of the set: **`BigDecimal("NaN")` and `BigDecimal("Infinity")` succeed where `Float()` raises**, so a client could send the literal string `"NaN"` for a `:decimal` price and have it stored — and `:float` rejected exactly those strings, so the two types disagreed, which is what marks the behaviour as accidental rather than designed. Non-finite results are now `invalid_type` for both types. + A genuine zero is unaffected however it is spelled — `"0"`, `"0.0"`, `"0.0000"` and `"0e10"` all still cast to `0.0`. Underflow is only visible against the source text (the result is an ordinary `0.0`), so a zero result is rejected only when the string named a nonzero **significand**; the exponent's digits say nothing about the value, which is why `"0e10"` is fine. `:decimal` keeps accepting the large exponents `BigDecimal` genuinely represents (`"1e400"` → `0.1e401`), since it has no exponent limit to overflow. ### Changed - **A `Time` passed to a `:datetime` field is no longer converted in place.** Normalising to UTC went through `value.to_time.utc`; `Time#to_time` returns `self` and `Time#utc` converts its **receiver**, so validating a request quietly rewrote the caller's own object — after `call!(at: t)`, `t` had become UTC. The cast now returns a new instance and leaves the argument alone. An `ActiveSupport::TimeWithZone` is likewise no longer handed back by way of the UTC instance it caches internally. diff --git a/README.md b/README.md index f15b130..9c86c63 100644 --- a/README.md +++ b/README.md @@ -329,6 +329,8 @@ Coercion is **deliberately strict**, and deliberately *not* `ActiveModel::Type`. **Dates are parsed, never guessed.** `Date.parse` fills in what a string omits *from today* — `"09/2026"` becomes the 1st, `"5th"` becomes this month of this year — so the same request would mean different things on different days. A `:date` or `:datetime` string must therefore name all three of year, month and day; which **format** it names them in is `Date.parse`'s business, so every complete format it understands still works. A `:datetime` may omit the *time* part, which reads as midnight UTC. +**Numbers must be finite.** `Float("1e400")` is `Infinity` and `Float("1e-400")` is `0.0` — neither represents what was sent, and neither is a value a numeric column can store, so both are `invalid_type`. A genuine zero is unaffected however it is spelled (`"0"`, `"0.0"`, `"0e10"`). `:decimal` has no exponent limit, so `"1e400"` is fine there — but `BigDecimal("NaN")` and `BigDecimal("Infinity")` *succeed* where `Float()` raises, so those literal strings are rejected explicitly. + Two more behaviours worth committing to memory: - **Type confusion is a violation, not a 500.** A request of `?age[]=1` against a scalar `:integer` field yields `invalid_type`. Arrays, hashes, and nested `ActionController::Parameters` can never satisfy a scalar type, so the classic "`NoMethodError` on `[]`" crash is impossible. diff --git a/lib/permittable.rb b/lib/permittable.rb index ac21409..f0bc714 100644 --- a/lib/permittable.rb +++ b/lib/permittable.rb @@ -469,23 +469,54 @@ def cast_integer(value) def cast_float(value) case value - when Numeric then [:ok, value.to_f] - when String then [:ok, Float(value)] + when Numeric then finite_float(value.to_f) + when String then finite_float(Float(value), source: value) else [:error, "invalid_type"] end rescue ArgumentError [:error, "invalid_type"] end + # A Float that is not finite does not represent what was sent. "1e400" + # overflows to Infinity and "1e-400" underflows to zero — both silently, + # and both leaving a value no column can faithfully store. + # + # Underflow is only visible against the source text, since the result is + # an ordinary 0.0: a zero result is rejected when the string it came from + # named a nonzero SIGNIFICAND. Only the significand, because "0e10" is a + # genuine zero whose exponent digits say nothing about the value — as are + # "0", "0.0" and "0.0000". + def finite_float(result, source: nil) + return [:error, "invalid_type"] unless result.finite? + return [:error, "invalid_type"] if result.zero? && nonzero_significand?(source) + + [:ok, result] + end + + def nonzero_significand?(source) + return false unless source + + source.split(/[eE]/, 2).first.match?(/[1-9]/) + end + def cast_decimal(value) case value - when Numeric, String then [:ok, BigDecimal(value.to_s)] + when Numeric, String then finite_decimal(BigDecimal(value.to_s)) else [:error, "invalid_type"] end rescue ArgumentError [:error, "invalid_type"] end + # BigDecimal has no exponent limit, so a :decimal cannot overflow — but + # BigDecimal("NaN") and BigDecimal("Infinity") SUCCEED where Float() + # raises, so a client could send the literal string "NaN" for a price and + # have it stored. Nothing else in the gem disagreed with itself this + # loudly: :float rejected those strings and :decimal did not. + def finite_decimal(result) + result.finite? ? [:ok, result] : [:error, "invalid_type"] + end + def cast_boolean(value) return [:ok, true] if TRUE_VALUES.include?(value) return [:ok, false] if FALSE_VALUES.include?(value) diff --git a/spec/permittable_spec.rb b/spec/permittable_spec.rb index 7387c23..72eb95b 100644 --- a/spec/permittable_spec.rb +++ b/spec/permittable_spec.rb @@ -413,6 +413,66 @@ def violations_for(params, action: "create", &declaration) end end + describe "numbers the type cannot faithfully hold" do + let(:decl) do + proc do + permit_params(:create) do + optional :f, :float + optional :d, :decimal + end + end + end + + def rejected(key, value, &decl) + violations_for({ key => value }, &decl).details + end + + it "rejects a :float that overflowed to Infinity" do + ["1e400", "-1e400", "1#{"0" * 400}"].each do |value| + expect(rejected(:f, value, &decl)).to eq([{ param: "f", code: "invalid_type" }]), "for #{value[0, 12]}" + end + end + + it "rejects a :float that underflowed to zero, losing the whole value" do + ["1e-400", "-1e-400", "0.1e-400"].each do |value| + expect(rejected(:f, value, &decl)).to eq([{ param: "f", code: "invalid_type" }]), "for #{value}" + end + end + + it "still accepts a genuine zero, however it is spelled" do + ["0", "0.0", "-0.0", "0e10", "0.0000"].each do |value| + expect(permit({ f: value }, &decl)[:f]).to eq(0.0), "for #{value}" + end + end + + it "rejects non-finite Float objects for both numeric types" do + [Float::INFINITY, -Float::INFINITY, Float::NAN].each do |value| + expect(rejected(:f, value, &decl)).to eq([{ param: "f", code: "invalid_type" }]), "for :float #{value}" + expect(rejected(:d, value, &decl)).to eq([{ param: "d", code: "invalid_type" }]), "for :decimal #{value}" + end + end + + it "rejects the literal strings a client could send for a :decimal" do + # BigDecimal("NaN") succeeds where Float("NaN") raises, so :decimal + # accepted these while :float did not. + %w[NaN Infinity -Infinity].each do |value| + expect(rejected(:d, value, &decl)).to eq([{ param: "d", code: "invalid_type" }]), "for #{value}" + end + end + + it "keeps accepting the large exponents BigDecimal genuinely represents" do + expect(permit({ d: "1e400" }, &decl)[:d]).to eq(BigDecimal("1e400")) + expect(permit({ d: "0.0000000000000000001" }, &decl)[:d]).to eq(BigDecimal("1e-19")) + end + + it "leaves ordinary numbers alone" do + result = permit({ f: "1.5", d: "2.50" }, &decl) + expect(result[:f]).to eq(1.5) + expect(result[:d]).to eq(BigDecimal("2.5")) + expect(permit({ f: 3, d: 4 }, &decl).to_h).to eq("f" => 3.0, "d" => BigDecimal("4")) + end + end + describe "validations" do it "checks in: as Range (cover) and as Array (inclusion)" do decl = proc { permit_params(:create) { required :age, :integer, in: 18..120 } }