ExportXMLWordPrintableJSON

    • Type: Bug
    • Resolution: Unresolved
    • Priority: Minor - P4
    • None
    • Affects Version/s: None
    • Component/s: None
    • None
    • None
    • Ruby Drivers
    • None
    • None
    • None
    • None
    • None
    • None

      Summary

      For Integer and Float fields, the query path and the write path parse the same string into different numbers. Nothing raises, so queries, updates and deletes silently target a value the application never persisted.

      Example on master (Mongoid 9.1.0):

      Coupon.where(code: "010").selector =>

      {"code" => 8}

      Coupon.new(code: "010").code => 10

      Reproduced

      Verified end-to-end against a local replica set on master (8c976ba1c):

      • where(code: "010") on a collection holding code=10 and code=8 returned the code=8 document.
      • Coupon.where(code: "010").delete_all removed the code=8 document and left code=10 intact.
      • find_or_create_by(code: "010") created a new duplicate on every call: three calls against a collection already holding code=10 left four documents with code=10.
      • find_or_create_by(code: "010") returned an unrelated pre-existing code=8 document when one was present.
      • No exception is raised in any of these cases.

      Root cause

      lib/mongoid/criteria/queryable/extensions/numeric.rb:55 calls Kernel#Integer(str) with no base, so Ruby radix rules apply: "010" parses as octal 8, "0x1f" as 31, "0b11" as 3. The Float(object) fallback on line 57 additionally accepts hex-float literals. evolve (numeric.rb:72-78) feeds every criteria value through numeric, and its rescue only covers failed conversions, never wrong successful ones.

      The write path disagrees: Integer.mongoize (lib/mongoid/extensions/integer.rb:51) uses decimal to_i, guarded by String#numeric? (lib/mongoid/extensions/string.rb:84), which is Float-based and so accepts "0x1f" while to_i returns 0.

      find_or_create_by (lib/mongoid/criteria/modifiable.rb:199) exercises both parses inside Mongoid itself, which is why the uniqueness invariant breaks with correct application code.

      Affected scope

      Integer and Float fields only.

      Divergent inputs are strings matching \A[+-]?0[0-7]

      {2,}

      \z (561 of the 1103 leading-zero forms enumerated over 000 to 0999), plus 0x, 0b and 0o prefixes and hex floats.

      Not affected, contrary to what might be assumed:

      • BigDecimal fields. where(d: "010") evolves to Decimal128('10'), which is correct, and "0x1f" raises BSON::Error::InvalidDecimal128String rather than diverging silently.
      • "1_000" gives 1000 on both paths.
      • "08" and "09" give 8 and 9 on both paths, because Integer() raises on the invalid octal digit and the Float fallback normalises back to the decimal reading.
      • validates_uniqueness_of is not affected. It validates the mongoized value and correctly flags the duplicate.

      Regression origin and affected releases

      This is a regression introduced by the ReDoS remediation in MONGOID-5743 (b083e2c31, 2024-07-01), backported as MONGOID-5793 and MONGOID-5794. The regex it replaced routed all-digit strings to to_i, so "010" evolved to decimal 10.

      First affected releases: 9.0.1, 8.1.6, 8.0.9. All maintained lines are affected through 9.1.0, 9.0.11, 8.1.12 and 8.0.12.

      The 0x, 0b and hex-float divergence predates that change and comes from the Float fallback.

      Suggested direction

      The two paths need to agree, and decimal is the reading users expect and the one Mongoid shipped before the regression. Making evolve parse plain digit strings in base 10 addresses the leading-zero case.

      The prefixed forms are a second, older question: String#numeric? accepting "0x1f" while to_i returns 0 means the write path stores 0 for input the guard called numeric. Worth deciding alongside, rather than fixing only the query side.

      No existing spec asserts radix behaviour. numeric_spec.rb and integer_spec.rb pass unchanged (38 examples), so the documented contract does not block a fix.

            Assignee:
            Unassigned
            Reporter:
            Dmitry Rybakov
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: