qa: quote path in authproxy for external multiwallets #13823

MarcoFalke commented at 7:23 PM on July 31, 2018: member

When using external multiwallets they are specified by their full path which might contain non-ascii characters (e.g. umlauts or emojis).

Fix this by url-quoting the path.

MarcoFalke added the label Tests on Jul 31, 2018

fanquake commented at 9:22 AM on August 2, 2018: member

ACK fae6ad9

Tested by adjusting the wallet_multiwallet.py test to create a wallet with the name ₿. See master output below. Should we add a similar testcase to the list of wallet_names?

master (aa30e4b):

test/functional/test_runner.py wallet_multiwallet.py
Temporary test directory at /var/folders/tg/m2x9fgs930d8xrf6n6gldrtc0000gn/T/bitcoin_test_runner_20180802_171457
............................................................................................................................................
wallet_multiwallet.py failed, Duration: 70 s

stdout:
2018-08-02T09:14:57.920000Z TestFramework (INFO): Initializing test directory /var/folders/tg/m2x9fgs930d8xrf6n6gldrtc0000gn/T/bitcoin_test_runner_20180802_171457/wallet_multiwallet_0
2018-08-02T09:15:03.198000Z TestFramework (INFO): Do not allow -zapwallettxes with multiwallet
2018-08-02T09:15:03.978000Z TestFramework (INFO): Do not allow -salvagewallet with multiwallet
2018-08-02T09:15:04.500000Z TestFramework (INFO): Do not allow -upgradewallet with multiwallet
2018-08-02T09:15:08.082000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
  File "/Users/michael/GitHub/bitcoin/test/functional/test_framework/test_framework.py", line 160, in main
    self.run_test()
  File "/Users/michael/github/bitcoin/test/functional/wallet_multiwallet.py", line 135, in run_test
    wallets[0].generate(1)
  File "/Users/michael/GitHub/bitcoin/test/functional/test_framework/coverage.py", line 47, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/Users/michael/GitHub/bitcoin/test/functional/test_framework/authproxy.py", line 137, in __call__
    response = self._request('POST', self.__url.path, postdata.encode('utf-8'))
  File "/Users/michael/GitHub/bitcoin/test/functional/test_framework/authproxy.py", line 107, in _request
    self.__conn.request(method, path, postdata, headers)
  File "/usr/local/Cellar/python/3.6.5_1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/http/client.py", line 1239, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/local/Cellar/python/3.6.5_1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/http/client.py", line 1250, in _send_request
    self.putrequest(method, url, **skips)
  File "/usr/local/Cellar/python/3.6.5_1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/http/client.py", line 1117, in putrequest
    self._output(request.encode('ascii'))
UnicodeEncodeError: 'ascii' codec can't encode character '\u20bf' in position 13: ordinal not in range(128)
2018-08-02T09:15:08.136000Z TestFramework (INFO): Stopping nodes
2018-08-02T09:15:08.136000Z TestFramework.node0 (ERROR): Unable to stop node.

qa: Quote wallet name for rpc path fa67505e1e

in test/functional/test_framework/authproxy.py:105 in fae6ad9c73 outdated

 101 | @@ -102,6 +102,7 @@ def _request(self, method, path, postdata):
 102 |                     'User-Agent': USER_AGENT,
 103 |                     'Authorization': self.__auth_header,
 104 |                     'Content-type': 'application/json'}
 105 | +        path = urllib.parse.quote(path)

laanwj commented at 11:12 AM on August 2, 2018:

I'm not sure this is the correct place to do URL-quoting (in general, you never want to url-quote a path, just path components). Wouldn't the best place to this be where the path is generated?

laanwj commented at 11:16 AM on August 2, 2018:

so this would be get_wallet_rpc(self, wallet_name)

MarcoFalke commented at 12:25 PM on August 2, 2018:

Indeed, we woulndn't want to double-quote in case the path is already quoted.

MarcoFalke force-pushed on Aug 2, 2018

MarcoFalke commented at 12:27 PM on August 2, 2018: member

Quoted the wallet name as per @laanwj.

Can be tested with ./test/functional/wallet_multiwallet.py --tmpdir=temp👛001

MarcoFalke added this to the milestone 0.17.0 on Aug 2, 2018

fanquake commented at 12:59 PM on August 2, 2018: member

Re-tested fa67505 using ./test/functional/wallet_multiwallet.py --tmpdir=temp👛001 from above.

master 990e182:

./test/functional/wallet_multiwallet.py --tmpdir=temp👛001
2018-08-02T12:55:54.136000Z TestFramework (INFO): Initializing test directory /Users/michael/GitHub/bitcoin/temp👛001
2018-08-02T12:55:59.574000Z TestFramework (INFO): Do not allow -zapwallettxes with multiwallet
2018-08-02T12:56:00.355000Z TestFramework (INFO): Do not allow -salvagewallet with multiwallet
2018-08-02T12:56:00.877000Z TestFramework (INFO): Do not allow -upgradewallet with multiwallet
2018-08-02T12:56:04.538000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
  File "/Users/michael/GitHub/bitcoin/test/functional/test_framework/test_framework.py", line 161, in main
    self.run_test()
  File "./test/functional/wallet_multiwallet.py", line 137, in run_test
    info = wallet.getwalletinfo()
  File "/Users/michael/GitHub/bitcoin/test/functional/test_framework/coverage.py", line 47, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/Users/michael/GitHub/bitcoin/test/functional/test_framework/authproxy.py", line 136, in __call__
    response = self._request('POST', self.__url.path, postdata.encode('utf-8'))
  File "/Users/michael/GitHub/bitcoin/test/functional/test_framework/authproxy.py", line 106, in _request
    self.__conn.request(method, path, postdata, headers)
  File "/usr/local/Cellar/python/3.6.5_1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/http/client.py", line 1239, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/local/Cellar/python/3.6.5_1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/http/client.py", line 1250, in _send_request
    self.putrequest(method, url, **skips)
  File "/usr/local/Cellar/python/3.6.5_1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/http/client.py", line 1117, in putrequest
    self._output(request.encode('ascii'))
UnicodeEncodeError: 'ascii' codec can't encode character '\U0001f45b' in position 47: ordinal not in range(128)
2018-08-02T12:56:04.592000Z TestFramework (INFO): Stopping nodes
2018-08-02T12:56:04.593000Z TestFramework.node0 (ERROR): Unable to stop node.

with fa67505:

2018-08-02T12:59:21.998000Z TestFramework (INFO): Cleaning up /Users/michael/GitHub/bitcoin/temp👛001 on exit
2018-08-02T12:59:21.998000Z TestFramework (INFO): Tests successful

Sjors commented at 1:28 PM on August 2, 2018: member

tACK fa67505 on macOS 10.13.6

laanwj commented at 1:59 PM on August 2, 2018: member

utACK fa67505e1ea007bdc081bc7425fb83d5455d8308

laanwj merged this on Aug 2, 2018

laanwj closed this on Aug 2, 2018

laanwj referenced this in commit 489b51b08e on Aug 2, 2018

MarcoFalke deleted the branch on Aug 2, 2018

laanwj referenced this in commit 238432302a on Aug 6, 2018

PastaPastaPasta referenced this in commit 4aa5b84b0f on Dec 16, 2020

PastaPastaPasta referenced this in commit 8f747d236a on Dec 18, 2020

PastaPastaPasta referenced this in commit e6089da994 on Dec 18, 2020

UdjinM6 referenced this in commit 04cf057672 on Jun 29, 2021

UdjinM6 referenced this in commit 56df77cd9f on Jun 29, 2021

UdjinM6 referenced this in commit 1f8e0730f8 on Jun 29, 2021

UdjinM6 referenced this in commit 273a594c1f on Jun 29, 2021

UdjinM6 referenced this in commit 6a3d8ae15f on Jun 30, 2021

UdjinM6 referenced this in commit 17d89ffadf on Jul 1, 2021

UdjinM6 referenced this in commit 99bd57d7df on Jul 2, 2021

UdjinM6 referenced this in commit 37edce56fb on Jul 2, 2021

MarcoFalke locked this on Sep 8, 2021