Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coercing float to BigDecimal does not work #2032

Closed
tlconnor opened this issue Mar 31, 2020 · 2 comments
Closed

Coercing float to BigDecimal does not work #2032

tlconnor opened this issue Mar 31, 2020 · 2 comments
Labels

Comments

@tlconnor
Copy link
Contributor

Given param definitions:

params do
  requires :amount, type: BigDecimal
end

and input JSON { "amount": "1.23" }, the endpoint will see params

{ amount: 0.123e1 } # 0.123e1 is a BigDecimal

If a Float is instead passed { "amount": 1.23 } the endpoint will then see params

{ amount: 1.23 } # 1.23 is a Float when it should be a BigDecimal

There is currently a test which covers this case, however it is testing the wrong thing. If I change the test to output the .class of the resulting value, you can see that it is a Float instead of a BigDecimal:

diff --git a/spec/grape/validations/validators/coerce_spec.rb b/spec/grape/validations/validators/coerce_spec.rb
index e77d839..4f5ed00 100644
--- a/spec/grape/validations/validators/coerce_spec.rb
+++ b/spec/grape/validations/validators/coerce_spec.rb
@@ -162,12 +162,12 @@ describe Grape::Validations::CoerceValidator do
             requires :bigdecimal, type: BigDecimal
           end
           subject.post '/bigdecimal' do
-            params[:bigdecimal]
+            params[:bigdecimal].class
           end
 
           post '/bigdecimal', { bigdecimal: 45.1 }.to_json, headers
           expect(last_response.status).to eq(201)
-          expect(last_response.body).to eq('45.1')
+          expect(last_response.body).to eq('BigDecimal')
         end

results in

Failures:

  1) Grape::Validations::CoerceValidator coerce coerces json BigDecimal
     Failure/Error: expect(last_response.body).to eq('BigDecimal')
     
       expected: "BigDecimal"
            got: "Float"
     
       (compared using ==)
     # ./spec/grape/validations/validators/coerce_spec.rb:170:in `block (5 levels) in <top (required)>'

The root cause of the problem can be found here https://github.com/ruby-grape/grape/blob/master/lib/grape/validations/validators/coerce.rb#L50 and was introduced in 9b352a88#diff-4b67e961836edb86baea810dc3156e24R48.

The problem is that the code uses == to determine if a param value has changed after coersion, however a Float and BigDecimal can == each other:

> 1.23 == BigDecimal('1.23')
=> true

The Float never gets replaced with the BigDecimal. A previous issue covers this same problem, but the fix did not work properly.
#1969

@dblock
Copy link
Member

dblock commented Mar 31, 2020

Looks like a bug to me, fix welcome, /cc: @dnesteryuk.

@dnesteryuk
Copy link
Member

ok, it was fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants