steemit / steem

The blockchain for Smart Media Tokens (SMTs) and decentralized applications.
https://steem.com
Other
1.95k stars 793 forks source link

JSONRPC ids handled incorrectly in steemd and appbase #1945

Open john-g-g opened 6 years ago

john-g-g commented 6 years ago

steemd

When the value of a JSONRPC request id exceeds the max value of an int64 but is less than the max value of a uint64, the JSONRPC response id value is incorrectly cast as an int64, probably here: https://github.com/steemit/fc/blob/master/src/rpc/json_connection.cpp#L150

id < max int64 : id is correct value but incorrect type, should be int not string

$ http https://steemd.steemit.com id:=9223372036854775807 jsonrpc='2.0' method='get_dynamic_global_properties'
HTTP/1.1 200 OK
Access-Control-Allow-Headers: DNT,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range
Access-Control-Allow-Methods: GET, POST, OPTIONS
Access-Control-Allow-Origin: *
Connection: keep-alive
Content-Length: 1074
Date: Fri, 22 Dec 2017 21:39:20 GMT
Server: nginx
Strict-Transport-Security: max-age=31557600; includeSubDomains; preload

{"id":"9223372036854775807","result":{"id":0,"head_block_number":18319693,"head_block_id":"0117894df4d2a3a5fe0f9772741d2c6d5ef91d74","time":"2017-12-22T21:39:18","current_witness":"smooth.witness","total_pow":514415,"num_pow_witnesses":172,"virtual_supply":"262943737.090 STEEM","current_supply":"261607573.201 STEEM","confidential_supply":"0.000 STEEM","current_sbd_supply":"4139435.730 SBD","confidential_sbd_supply":"0.000 SBD","total_vesting_fund_steem":"196441492.955 STEEM","total_vesting_shares":"402725396294.551504 VESTS","total_reward_fund_steem":"0.000 STEEM","total_reward_shares2":"0","pending_rewarded_vesting_shares":"389831091.707760 VESTS","pending_rewarded_vesting_steem":"189278.995 STEEM","sbd_interest_rate":0,"sbd_print_rate":10000,"maximum_block_size":65536,"current_aslot":18381186,"recent_slots_filled":"340282366920938463463374607431768211455","participation_count":128,"last_irreversible_block_num":18319675,"vote_power_reserve_rate":10,"current_reserve_ratio":200000000,"average_block_size":11173,"max_virtual_bandwidth":"264241152000000000000"}}

max int64 < id < max uint64: incorrect id value, correct id type

$ http https://steemd.steemit.com id:=9223372036854775808 jsonrpc='2.0' method='get
_dynamic_global_properties'
HTTP/1.1 200 OK
Access-Control-Allow-Headers: DNT,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range
Access-Control-Allow-Methods: GET, POST, OPTIONS
Access-Control-Allow-Origin: *
Connection: keep-alive
Content-Length: 1063
Date: Fri, 22 Dec 2017 21:39:33 GMT
Server: nginx
Strict-Transport-Security: max-age=31557600; includeSubDomains; preload

{"id":-9223372036854775808,"result":{"id":0,"head_block_number":18319697,"head_block_id":"0117895191b07e67cdfba7b3058303b654bfa8b5","time":"2017-12-22T21:39:30","current_witness":"anyx","total_pow":514415,"num_pow_witnesses":172,"virtual_supply":"262943745.993 STEEM","current_supply":"261607581.532 STEEM","confidential_supply":"0.000 STEEM","current_sbd_supply":"4139437.503 SBD","confidential_sbd_supply":"0.000 SBD","total_vesting_fund_steem":"196441499.399 STEEM","total_vesting_shares":"402725406725.529365 VESTS","total_reward_fund_steem":"0.000 STEEM","total_reward_shares2":"0","pending_rewarded_vesting_shares":"389824102.862052 VESTS","pending_rewarded_vesting_steem":"189275.586 STEEM","sbd_interest_rate":0,"sbd_print_rate":10000,"maximum_block_size":65536,"current_aslot":18381190,"recent_slots_filled":"340282366920938463463374607431768211455","participation_count":128,"last_irreversible_block_num":18319679,"vote_power_reserve_rate":10,"current_reserve_ratio":200000000,"average_block_size":11111,"max_virtual_bandwidth":"264241152000000000000"}}

max int64 < id < max uint64: incorrect value, correct id type

$ http https://steemd.steemit.com id:=18446744073709551615 jsonrpc='2.0' method='get_dynamic_global_properties'
HTTP/1.1 200 OK
Access-Control-Allow-Headers: DNT,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range
Access-Control-Allow-Methods: GET, POST, OPTIONS
Access-Control-Allow-Origin: *
Connection: keep-alive
Content-Length: 1047
Date: Fri, 22 Dec 2017 21:32:01 GMT
Server: nginx
Strict-Transport-Security: max-age=31557600; includeSubDomains; preload

{"id":-1,"result":{"id":0,"head_block_number":18319547,"head_block_id":"011788bb6199a78d275f1c0eb03ac46169b86f36","time":"2017-12-22T21:32:00","current_witness":"clayop","total_pow":514415,"num_pow_witnesses":172,"virtual_supply":"262943406.785 STEEM","current_supply":"261607326.702 STEEM","confidential_supply":"0.000 STEEM","current_sbd_supply":"4139176.098 SBD","confidential_sbd_supply":"0.000 SBD","total_vesting_fund_steem":"196441949.885 STEEM","total_vesting_shares":"402726434524.822409 VESTS","total_reward_fund_steem":"0.000 STEEM","total_reward_shares2":"0","pending_rewarded_vesting_shares":"389826150.832497 VESTS","pending_rewarded_vesting_steem":"189276.584 STEEM","sbd_interest_rate":0,"sbd_print_rate":10000,"maximum_block_size":65536,"current_aslot":18381040,"recent_slots_filled":"340282366920936045611735378173418799103","participation_count":127,"last_irreversible_block_num":18319529,"vote_power_reserve_rate":10,"current_reserve_ratio":200000000,"average_block_size":11899,"max_virtual_bandwidth":"264241152000000000000"}}

id > max uint64: no id value

$ http https://steemd.steemit.com id:=18446744073709551616 jsonrpc='2.0' method='get_dynamic_global_properties'
HTTP/1.1 200 OK
Access-Control-Allow-Headers: DNT,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range
Access-Control-Allow-Methods: GET, POST, OPTIONS
Access-Control-Allow-Origin: *
Connection: keep-alive
Content-Length: 389
Date: Fri, 22 Dec 2017 21:32:15 GMT
Server: nginx
Strict-Transport-Security: max-age=31557600; includeSubDomains; preload

4 parse_error_exception: Parse Error
Couldn't parse uint64_t
    {}
    th_a  string.cpp:113 to_uint64

    {"i":"18446744073709551616"}
    th_a  string.cpp:116 to_uint64
Error parsing object
    {}
    th_a  json.cpp:218 objectFromStream

    {"str":"{\"id\": 18446744073709551616, \"jsonrpc\": \"2.0\", \"method\": \"get_dynamic_global_properties\"}"}
    th_a  json.cpp:478 from_string

appbase

id < max uint64: id is correct value but incorrect type, should be int not string

$ http https://steemd.steemitdev.com id:=9223372036854775808 jsonrpc='2.0' method='database_api.get_dynamic_global_properties'
HTTP/1.1 200 OK
Access-Control-Allow-Headers: DNT,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range
Access-Control-Allow-Methods: GET, POST, OPTIONS
Access-Control-Allow-Origin: *
Connection: keep-alive
Content-Length: 979
Date: Fri, 22 Dec 2017 21:43:53 GMT
Server: nginx
Strict-Transport-Security: max-age=31557600; includeSubDomains; preload

{"jsonrpc":"2.0","result":{"id":0,"head_block_number":18319784,"head_block_id":"011789a8b90e04b0cf98795ea6155d4bde868d7d","time":"2017-12-22T21:43:51","current_witness":"themarkymark","total_pow":514415,"num_pow_witnesses":172,"virtual_supply":"262943943.454 STEEM","current_supply":"261607753.227 STEEM","confidential_supply":"0.000 STEEM","current_sbd_supply":"4139517.325 SBD","confidential_sbd_supply":"0.000 SBD","total_vesting_fund_steem":"196441647.128 STEEM","total_vesting_shares":"402725649116.151865 VESTS","total_reward_fund_steem":"0.000 STEEM","total_reward_shares2":"0","pending_rewarded_vesting_shares":"389743661.924439 VESTS","pending_rewarded_vesting_steem":"189236.349 STEEM","sbd_interest_rate":0,"sbd_print_rate":10000,"maximum_block_size":65536,"current_aslot":18381277,"recent_slots_filled":"340282366920938463463374607431768211455","participation_count":128,"last_irreversible_block_num":18319764,"vote_power_reserve_rate":10},"id":"9223372036854775808"}
jnordberg commented 6 years ago

FYI if you try to send anything larger than 9007199254740991 to any of our node.js services they will throw an error

mvandeberg commented 6 years ago

The ID field in FC's jsonrpc parser is known to be buggy. (#216)

That was one of the big motivations for replacing it with the jsonrpc plugin in appbase. I will not be fixing any problems with master and only appbase.

A bit more descriptive bug report for appbase is the ID field returns the incorrect type for IDs greater than 2^32 - 1.

I did some debugging and the type parsing in the jsonrpc plugin is working correctly for 2^32. It correctly identifies and stores that value as a uint64. However, it serializes as a string.

When I did an idump of 2^32 it logged as an int. Which leads me to believe there is something in the json parser that is causing the value to be written as a string.