Skip to content

Conversation

ryanofsky
Copy link
Contributor

Prevent -noproxy and -proxy=0 settings from interacting with -listen, -upnp, and -natpmp settings.

These settings started being handled inconsistently in the AppInitMain and InitParameterInteraction functions starting in commit baf0507 from #6272:

bitcoin/src/init.cpp

Lines 990 to 991 in baf0507

std::string proxyArg = GetArg("-proxy", "");
if (proxyArg != "" && proxyArg != "0") {

if (mapArgs.count("-proxy")) {

This commit changes both functions to handle proxy arguments the same way so
there are not side effects from specifying a proxy=0 setting.

This change was originally part of #24830 but really is independent and makes more sense as a separate PR

…her settings

Prevent -noproxy and -proxy=0 settings from interacting with -listen, -upnp,
and -natpmp settings.

These settings started being handled inconsistently in the `AppInitMain` and
`InitParameterInteraction` functions starting in commit
baf0507 from bitcoin#6272:

https://github.com/bitcoin/bitcoin/blob/baf05075fae2cc2625a2a74b35cc66902f3cbfa3/src/init.cpp#L990-L991
https://github.com/bitcoin/bitcoin/blob/baf05075fae2cc2625a2a74b35cc66902f3cbfa3/src/init.cpp#L687

This commit changes both functions to handle proxy arguments the same way so
there are not side effects from specifying a proxy=0 setting.
Comment on lines +663 to +664
std::string proxy_arg = args.GetArg("-proxy", "");
if (proxy_arg != "" && proxy_arg != "0") {
Copy link
Contributor Author

@ryanofsky ryanofsky Apr 12, 2022

Choose a reason for hiding this comment

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

In commit "init: Prevent -noproxy and -proxy=0 settings from interacting with other settings" (3429d67)

These lines are copied from AppInitMain below

Comment on lines +663 to +664
std::string proxy_arg = args.GetArg("-proxy", "");
if (proxy_arg != "" && proxy_arg != "0") {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
std::string proxy_arg = args.GetArg("-proxy", "");
if (proxy_arg != "" && proxy_arg != "0") {
if (args.GetBoolArg("-proxy")) {

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind, this can do the wrong thing if a long decimal IP is specified.

Copy link
Member

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

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

utACK

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 3429d67, tested on Ubuntu 22.04.

@maflcko maflcko merged commit 2074d7d into bitcoin:master Apr 17, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 18, 2022
…ting with other settings

3429d67 init: Prevent -noproxy and -proxy=0 settings from interacting with other settings (Ryan Ofsky)

Pull request description:

  Prevent `-noproxy` and `-proxy=0` settings from interacting with `-listen`, `-upnp`, and `-natpmp` settings.

  These settings started being handled inconsistently in the `AppInitMain` and `InitParameterInteraction` functions starting in commit baf0507 from bitcoin#6272:

  https://github.com/bitcoin/bitcoin/blob/baf05075fae2cc2625a2a74b35cc66902f3cbfa3/src/init.cpp#L990-L991
  https://github.com/bitcoin/bitcoin/blob/baf05075fae2cc2625a2a74b35cc66902f3cbfa3/src/init.cpp#L687

  This commit changes both functions to handle proxy arguments the same way so
  there are not side effects from specifying a proxy=0 setting.

  This change was originally part of bitcoin#24830 but really is independent and makes more sense as a separate PR

ACKs for top commit:
  hebasto:
    ACK 3429d67, tested on Ubuntu 22.04.

Tree-SHA512: c4c6b4aeb3c07321700e974c16fd47a1bd3d469f273a6b308a69638db81c88c4e67208fddc96fcda9c8bd85f3ae22c98ca131c9622895edaa34eb65c194f35db
@bitcoin bitcoin locked and limited conversation to collaborators Apr 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants