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.

Sign up for GitHub

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

Jump to bottom

cli: Reject arguments to -getinfo #11710

Merged
merged 1 commit into from Nov 18, 2017
Merged

cli: Reject arguments to -getinfo #11710

merged 1 commit into from Nov 18, 2017

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Nov 17, 2017 •
edited

Currently it's possible to accidentally type e.g.

bitcoin-cli -getinfo getbalance

and get an answer which can be confusing; the trailing arguments are just ignored.

To avoid this, throw an error if the user provides arguments to
-getinfo.

@fanquake
Copy link
Member

Travis failure:

bitcoin_cli.py failed, Duration: 1 s
stdout:
2017-11-17 13:47:24.232000 TestFramework (INFO): Initializing test directory /tmp/bitcoin_test_runner_20171117_134443/bitcoin_cli_303
2017-11-17 13:47:24.492000 TestFramework (INFO): Compare responses from gewalletinfo RPC and `bitcoin-cli getwalletinfo`
2017-11-17 13:47:24.503000 TestFramework (INFO): Compare responses from getblockchaininfo RPC and `bitcoin-cli getblockchaininfo`
2017-11-17 13:47:24.509000 TestFramework (INFO): Test -stdinrpcpass option
2017-11-17 13:47:24.768000 TestFramework (INFO): Test -stdin and -stdinrpcpass
2017-11-17 13:47:25.030000 TestFramework (INFO): Compare responses from `bitcoin-cli -getinfo` and the RPCs data is retrieved from.
2017-11-17 13:47:25.041000 TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 120, in main
    self.run_test()
  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-i686-pc-linux-gnu/test/functional/bitcoin_cli.py", line 39, in run_test
    cli_get_info = self.nodes[0].cli('-getinfo').help()
  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_node.py", line 211, in dispatcher
    return self.send_cli(command, *args, **kwargs)
  File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_node.py", line 229, in send_cli
    raise subprocess.CalledProcessError(returncode, self.binary, output=cli_stderr)
subprocess.CalledProcessError: Command '/home/travis/build/bitcoin/bitcoin/build/bitcoin-i686-pc-linux-gnu/src/bitcoin-cli' returned non-zero exit status 1
2017-11-17 13:47:25.042000 TestFramework (INFO): Stopping nodes

@laanwj
Copy link
Member Author

laanwj commented Nov 17, 2017

Strange! Seems a legit error. Will look at it.

Ah, yes, the test seems to test something that should be failing:

    cli_get_info = self.nodes[0].cli('-getinfo').help()

Will execute

    bitcoin-cli -getinfo help

Which makes no sense :) Will update the test.

@laanwj
cli: Reject arguments to -getinfo
dcfef27 
Currently it's possible to accidentally type e.g.

    bitcoin-cli -getinfo getbalance

and get an answer which can be confusing; the trialing arguments are
just ignored.

To avoid this, throw an error if the user provides arguments to
`-getinfo`.
@laanwj
Copy link
Member Author

laanwj commented Nov 17, 2017

Updated the test, now it tests both cases correctly, travis issue should be solved.

promag
promag reviewed Nov 17, 2017
View reviewed changes
Copy link
Member

@promag promag left a comment

Choose a reason for hiding this comment

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

utACK dcfef27.

test/functional/bitcoin_cli.py
@@ -35,8 +35,11 @@ def run_test(self):
assert_equal(["foo", "bar"], self.nodes[0].cli('-rpcuser=%s' % user, '-stdin', '-stdinrpcpass', input=password + "\nfoo\nbar").echo())
assert_raises_process_error(1, "incorrect rpcuser or rpcpassword", self.nodes[0].cli('-rpcuser=%s' % user, '-stdin', '-stdinrpcpass', input="foo").echo)

self.log.info("Make sure that -getinfo with arguments fails")
assert_raises_process_error(1, "-getinfo takes no arguments", self.nodes[0].cli('-getinfo').help)
Copy link
Member

Choose a reason for hiding this comment

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

👍

@maflcko
Copy link
Member

maflcko commented Nov 17, 2017

Tested ACK dcfef27

@jonasschnelli
Copy link
Contributor

utACK dcfef27

@meshcollider
Copy link
Contributor

utACK dcfef27

@laanwj laanwj merged commit dcfef27 into bitcoin:master Nov 18, 2017
laanwj added a commit that referenced this pull request Nov 18, 2017
@laanwj
Merge #11710: cli: Reject arguments to -getinfo
9388639 
dcfef27 cli: Reject arguments to -getinfo (Wladimir J. van der Laan)

Pull request description:

  Currently it's possible to accidentally type e.g.

      bitcoin-cli -getinfo getbalance

  and get an answer which can be confusing; the trailing arguments are just ignored.

  To avoid this, throw an error if the user provides arguments to
  `-getinfo`.

Tree-SHA512: 3603e8fa852b884d1dd3b7462db40b092fe8b3390fd4384b4ee330315d797aff711e9f62990012fd4b5a55c8678734ba8497a5488a09ee6b65cf8a99017d6eb4
markblundeberg pushed a commit to markblundeberg/bitcoin-abc that referenced this pull request Jun 7, 2019
@laanwj
Merge #11710: cli: Reject arguments to -getinfo
9dae9ed 
Summary:
dcfef27 cli: Reject arguments to -getinfo (Wladimir J. van der Laan)

Pull request description:

  Currently it's possible to accidentally type e.g.

      bitcoin-cli -getinfo getbalance

  and get an answer which can be confusing; the trailing arguments are just ignored.

  To avoid this, throw an error if the user provides arguments to
  `-getinfo`.

Tree-SHA512: 3603e8fa852b884d1dd3b7462db40b092fe8b3390fd4384b4ee330315d797aff711e9f62990012fd4b5a55c8678734ba8497a5488a09ee6b65cf8a99017d6eb4

Backport of Core PR11710
bitcoin/bitcoin#11710

Please note, this is `./bitcoin-cli -getinfo`, NOT the deprecated/removed RPC call.

Test Plan:
  make check
  test_runner.py

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc, markblundeberg

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, markblundeberg

Subscribers: markblundeberg

Differential Revision: https://reviews.bitcoinabc.org/D3229
jtoomim pushed a commit to jtoomim/bitcoin-abc that referenced this pull request Jun 29, 2019
@laanwj @jtoomim
Merge #11710: cli: Reject arguments to -getinfo
3e92f5e 
Summary:
dcfef27 cli: Reject arguments to -getinfo (Wladimir J. van der Laan)

Pull request description:

  Currently it's possible to accidentally type e.g.

      bitcoin-cli -getinfo getbalance

  and get an answer which can be confusing; the trailing arguments are just ignored.

  To avoid this, throw an error if the user provides arguments to
  `-getinfo`.

Tree-SHA512: 3603e8fa852b884d1dd3b7462db40b092fe8b3390fd4384b4ee330315d797aff711e9f62990012fd4b5a55c8678734ba8497a5488a09ee6b65cf8a99017d6eb4

Backport of Core PR11710
bitcoin/bitcoin#11710

Please note, this is `./bitcoin-cli -getinfo`, NOT the deprecated/removed RPC call.

Test Plan:
  make check
  test_runner.py

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc, markblundeberg

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, markblundeberg

Subscribers: markblundeberg

Differential Revision: https://reviews.bitcoinabc.org/D3229
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 13, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11710: cli: Reject arguments to -getinfo
5a43b0d 
dcfef27 cli: Reject arguments to -getinfo (Wladimir J. van der Laan)

Pull request description:

  Currently it's possible to accidentally type e.g.

      bitcoin-cli -getinfo getbalance

  and get an answer which can be confusing; the trailing arguments are just ignored.

  To avoid this, throw an error if the user provides arguments to
  `-getinfo`.

Tree-SHA512: 3603e8fa852b884d1dd3b7462db40b092fe8b3390fd4384b4ee330315d797aff711e9f62990012fd4b5a55c8678734ba8497a5488a09ee6b65cf8a99017d6eb4
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 27, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11710: cli: Reject arguments to -getinfo
09a7b62 
dcfef27 cli: Reject arguments to -getinfo (Wladimir J. van der Laan)

Pull request description:

  Currently it's possible to accidentally type e.g.

      bitcoin-cli -getinfo getbalance

  and get an answer which can be confusing; the trailing arguments are just ignored.

  To avoid this, throw an error if the user provides arguments to
  `-getinfo`.

Tree-SHA512: 3603e8fa852b884d1dd3b7462db40b092fe8b3390fd4384b4ee330315d797aff711e9f62990012fd4b5a55c8678734ba8497a5488a09ee6b65cf8a99017d6eb4
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 14, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11710: cli: Reject arguments to -getinfo
150c838 
dcfef27 cli: Reject arguments to -getinfo (Wladimir J. van der Laan)

Pull request description:

  Currently it's possible to accidentally type e.g.

      bitcoin-cli -getinfo getbalance

  and get an answer which can be confusing; the trailing arguments are just ignored.

  To avoid this, throw an error if the user provides arguments to
  `-getinfo`.

Tree-SHA512: 3603e8fa852b884d1dd3b7462db40b092fe8b3390fd4384b4ee330315d797aff711e9f62990012fd4b5a55c8678734ba8497a5488a09ee6b65cf8a99017d6eb4
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 14, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11710: cli: Reject arguments to -getinfo
0c97cd5 
dcfef27 cli: Reject arguments to -getinfo (Wladimir J. van der Laan)

Pull request description:

  Currently it's possible to accidentally type e.g.

      bitcoin-cli -getinfo getbalance

  and get an answer which can be confusing; the trailing arguments are just ignored.

  To avoid this, throw an error if the user provides arguments to
  `-getinfo`.

Tree-SHA512: 3603e8fa852b884d1dd3b7462db40b092fe8b3390fd4384b4ee330315d797aff711e9f62990012fd4b5a55c8678734ba8497a5488a09ee6b65cf8a99017d6eb4
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@promag promag promag left review comments

Assignees
No one assigned
Labels
RPC/REST/ZMQ
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@laanwj @fanquake @maflcko @jonasschnelli @meshcollider @promag