Skip to content

RPCConsole: Throw when overflowing size_t type for array indices#446

Merged
maflcko merged 1 commit into
bitcoin-core:masterfrom
maflcko:2110-qtRpcCons
Oct 6, 2021
Merged

RPCConsole: Throw when overflowing size_t type for array indices#446
maflcko merged 1 commit into
bitcoin-core:masterfrom
maflcko:2110-qtRpcCons

Conversation

@maflcko
Copy link
Copy Markdown
Contributor

@maflcko maflcko commented Oct 5, 2021

To test:

-> getblock(getbestblockhash(), 1)[tx][22222222222222222222222222222]

Before:

<- 868693731dea69a197c13c2cfaa41c9f78fcdeb8ab8e9f8cdf2c9025147ee7d1 (hash of the coinbase tx)

After:

<- Error: Invalid result query

@jarolrod jarolrod added the Bug Something isn't working label Oct 5, 2021
@jarolrod
Copy link
Copy Markdown
Contributor

jarolrod commented Oct 5, 2021

concept ack

Copy link
Copy Markdown
Contributor

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK faa5e17

tested and code review ack. This is a nice catch!

on master:

<-  getblock(getbestblockhash(), 1)[tx][22222222222222222222222222222]
->  e3038545d5b68dc61a282f15eb8e3ffbfb50c3f9955380a1fde3c1c858414d38

on PR branch:

<-  getblock(getbestblockhash(), 1)[tx][22222222222222222222222222222]
->  Error: Invalid result query

Copy link
Copy Markdown
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK faa5e17
Tested on Ubuntu 20.04

This PR allows displaying runtime error for inputs greater than the limit of size_t type.

This is done by using the ToIntegral<size_t> function, which will check the size of the input argument. Earlier, the logic was to check if each character of the input argument is a digit or not without considering the overall length of the input argument.

This is undoubtedly an improvement over the current Master, and I agree with this PR. I am adding the screenshot of input and outputs that I used to test this PR on the GUI console. The test was done on the Signet network.

Screenshots:

Master (outputs null, without errors) PR (output error)
Screenshot from 2021-10-06 19-51-35 Screenshot from 2021-10-06 19-45-25

@maflcko maflcko merged commit 7962c97 into bitcoin-core:master Oct 6, 2021
@maflcko maflcko deleted the 2110-qtRpcCons branch October 6, 2021 17:49
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 6, 2021
… type for array indices

faa5e17 RPCConsole: Throw when overflowing size_t type for array indices (MarcoFalke)

Pull request description:

  To test:

  -> `getblock(getbestblockhash(), 1)[tx][22222222222222222222222222222]`

  Before:

  <- `868693731dea69a197c13c2cfaa41c9f78fcdeb8ab8e9f8cdf2c9025147ee7d1` (hash of the coinbase tx)

  After:

  <- `Error: Invalid result query`

ACKs for top commit:
  jarolrod:
    ACK faa5e17
  shaavan:
    ACK faa5e17

Tree-SHA512: ddff39aae1c15db45928b110a9f1c42eadc5404cdfa89d67ccffc4c6af24091967d43c068ce9e0c1b863cfc4eb5b4f12373a73756a9474f8294e8a44aabc28d8
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Oct 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants