quotasets.Get().Extract() fails with JSON parser error on large values

I am trying to use quotasets.Get().Extract() on a tenant that has 9,216,000 MB of memory quota. It fails with this error:

json: cannot unmarshal number 9.216e+06 into Go value of type int

Taking a look at what Get() returns before the Extract():

{quotaResult:{Result:{Body:map[quota_set:map[security_group_rules:20 security_groups:10 key_pairs:100 instances:1
0 ram:9.216e+06 fixed_ips:-1 metadata_items:128 server_groups:10 injected_file_content_bytes:10240 cores:20 id:083cd4d7080f44988ff7d6
423e958d5e injected_files:5 injected_file_path_bytes:255 server_group_members:10 floating_ips:10]] Header:map[] Err:}}}

Digging a little further, the value from Get() seems fine:

    result := res.Result
    body := result.Body.(map[string]interface{})
    qs :=  body["quota_set"].(map[string]interface{})
    log.Printf("ram intitialy %f [%+v]\n", qs["ram"].(float64), qs["ram"].(float64))

ram intitialy 9216000.000000 [9.216e+06]

And the error comes out of gophercloud/results.go ExtractInto(), where it does the Marshal of res.Body followed by the attempted Unmarshal into a *QuotaSet. I can't figure out a work around for this; is it a fundamental bug in the json package?

This seems to be en issue with encoding/json. If the json decoder encounters a number in scientific notation (e.g. 9.216e+06 ) it assumes the number to be of type float64. The Ram field in the QuotaSet-Struct is of type int. So the decoder doesn't try to convert the float to an int and throws an error instead. (Here is an example: https://play.golang.org/p/gG4neHJmMN )
However, this does not seem to be a bug in encoding/json. It is the expected behavior ( golang/go#5562 )

Do you know if the OpenStack API returns the number in scientific notation, or does gophercloud somehow accidentally do the conversion?

I'm not sure what the OpenStack API returns, but with my debugging (as per my previous comment) I can see that the struct that ExtractInto() (and subsequently json.Marshal) operates on contains a float64 (after conversion from interface{}).

Changing the definition of type QuotaSet Ram field from int to float64 does fix the problem, but would no doubt break lots of existing code.

Knowing exactly how the response from OpenStack looks would probably help to find a better fix for this. Can you just throw a Print() into gophercloud/provider_client.go Request() to get the raw response Body?

I think I found the root of the problem.
gophercloud/provider_client.go Request() decodes the response body into an interface{}. Because of the lack of type information in an interface{} is decides to use float64 as type for Ram.
In ExtractInto() we Marshall the interface{}. Encoding/Json will encode large floats using scientific notation. Everything written in scientific notation is expected to be a float during decoding. So when we try to unmarshall this float into an int, we get an error.

I did:

diff --git a/provider_client.go b/provider_client.go
index f886823..757c114 100644
--- a/provider_client.go
+++ b/provider_client.go
@@ -7,6 +7,7 @@ import (
        "io/ioutil"
        "net/http"
        "strings"
+       "log"
 )

 // DefaultUserAgent is the default User-Agent string set in the request header.
@@ -280,8 +281,11 @@ func (client *ProviderClient) Request(method, url string, options *RequestOpts)

        // Parse the response body as JSON, if requested to do so.
        if options.JSONResponse != nil {
+               log.Printf("raw body: %+v\n", resp.Body)
                defer resp.Body.Close()
-               if err := json.NewDecoder(resp.Body).Decode(options.JSONResponse); err != nil {
+               dec := json.NewDecoder(resp.Body)
+               //dec.UseNumber()
+               if err := dec.Decode(options.JSONResponse); err != nil {
                        return nil, err
                }
        }

raw body: &{body:0xc4201a9840 mu:{state:0 sema:0} closed:false rerr: fn:0x6dc6b0 earlyCloseFn:0x6dc640}

Sorry, not sure how to do a better job of printing that out. But I can say that if I uncomment the dec.UseNumber() that I added there, then that also fixes the problem (leaving the field in the struct defined as an int). Not sure what impact that has on all other things that flow through Receive().

@jrperritt

This sounds like a good solution to me. UseNumber() forces the decoder to use json.Number as type instead of float64. When marshalling a json.Number the scientific notation will not be used (in contrast to when marshalling a float64) (example: https://play.golang.org/p/A1tpO2P8lk ). Without the scientific notation we can unmarshall to an int without any problems.

I do not think this fix would cause any problems, but this is what we have tests for.
But we should probably first ask a core developer, like @jrperritt , what he thinks about this solution.

I can confirm that the API is not returning a scientific notation. This issue is very similar to #195. Perhaps I was incorrect with the proposed solution of using int64 in the results, though?

This indeed looks like the same bug. The error message is exactly the same. and the number that triggered the bug in #195 (1.994752e+06) is not big enough to overflow an int(32), so its not an overflow bug (so int64 would not help)

Whats really nice about using UseNumber() in Request() to fix this, is that it would probably automatically fix this bug for every resource (not only quotas and flavors).

We've been handling number formats on an individual basis, but UseNumber seems like it could be a better alternative.

@dbaumgarten

@dbaumgarten I totally didn't have my glasses on and 1994752 looked longer than what it really is. :)

I've closed #195 in favor of this issue since there's a larger discussion taking place here.

Also, the good news is that this can easily be reproduced with a simple unit test. For example, modify openstack/compute/v2/flavors/testing/requests_test.go and replace 512 with 1994752 then run go test -v.

@jtopjian

@jtopjian My first thought when reading the issue was also "This must be an int overflow!" ;)

Modifying the (and maybe some other) unit test to use a large enough int is a good idea. That way we can hopefully find(or prevent) this kind of bugs easier in the future.

Augmenting gophercloud.ProviderClient.Request with UseNumber sounds reasonable to me if someone wants to send a PR and appropriate unit tests.

This may seem like a little silly, but I can't figure out how to properly run gophercloud's complete test suite, nor how I could test this. If someone could offer some brief pointers to get me started, I can do the PR. (I've been using this change in production since December with no problems, at least the way I'm using things.)

@sb10

@sb10 If you want to run the entire suite:

$ cd ~/go/src/github.com/gophercloud/gophercloud
$ go get -v -tags 'fixtures acceptance' ./...
$ ./script/unittest

Fix gophercloud#209

@sb10

@sb10 Are you still interested in raising this as a PR?

@fatmcgav

@fatmcgav Yes, I just haven't had time to work up the tests. If someone else thinks they can jump in quickly, please go ahead. Otherwise, I'll report back here when I actually begin work on the tests.

Fix gophercloud#209

Resolve gophercloud#209 - Always be able to handle large integers in …

Add 'vendor_options' to networking_router_v2 resource. (gophercloud#209)

Navigation Menu

Repository navigation

quotasets.Get().Extract() fails with JSON parser error on large values #209

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

quotasets.Get().Extract() fails with JSON parser error on large values #209

Description

Activity

sb10 commented on Dec 1, 2016

dbaumgarten commented on Dec 1, 2016

sb10 commented on Dec 1, 2016

sb10 commented on Dec 1, 2016

dbaumgarten commented on Dec 1, 2016

dbaumgarten commented on Dec 1, 2016

sb10 commented on Dec 1, 2016

dbaumgarten commented on Dec 1, 2016

jtopjian commented on Dec 1, 2016

dbaumgarten commented on Dec 1, 2016

jrperritt commented on Dec 1, 2016

jtopjian commented on Dec 2, 2016

dbaumgarten commented on Dec 2, 2016

jrperritt commented on May 8, 2017

sb10 commented on May 8, 2017

jtopjian commented on May 8, 2017

fatmcgav commented on Jun 13, 2017

sb10 commented on Jun 14, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions