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.

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

Poor error checking of socks proxy server address. #2743

Closed
tl121 opened this issue Nov 25, 2023 · 1 comment · Fixed by #2744
Closed

Poor error checking of socks proxy server address. #2743

tl121 opened this issue Nov 25, 2023 · 1 comment · Fixed by #2744

Comments

@tl121
Copy link

tl121 commented Nov 25, 2023

If the user configures EC network access via a Socks 5 server using a numeric ip address that begins with a leading blank prior to the correct server ip address,,
e.g " 192.168.8.229" instead of "192.168.8.229",
the user will not get any kind of error message, but no proxy connection will be made and the network status will remain disconnected, (red) if there is no server reachable except through a proxy.

This bug is reproducible. Subsequent deletion of the leading blank character and closing the pop up window will allow the proxy to work. Subsequent editing and adding the leading blank and closing the window will cause the proxy to fail, ...

This is a minor user experience problem. This exists in EC 4.3.1, and was also observed in Electrum 4.3.3.

EchterAgo added a commit to EchterAgo/Electron-Cash that referenced this issue Nov 26, 2023
This adds a new `HostValidator` class to validate hostnames and IP
addresses entered into a `QLineEdit`. We use this class to validate the
server and proxy host.

This also adds the existing `PortValidator` class to the server and
proxy port widgets.

fixes Electron-Cash#2743
@EchterAgo
Copy link

See #2744

EchterAgo added a commit to EchterAgo/Electron-Cash that referenced this issue Nov 26, 2023
This adds a new `HostValidator` class to validate hostnames and IP
addresses entered into a `QLineEdit`. We use this class to validate the
server and proxy host.

This also adds the existing `PortValidator` class to the server and
proxy port widgets.

fixes Electron-Cash#2743
EchterAgo added a commit to EchterAgo/Electron-Cash that referenced this issue Nov 26, 2023
This adds a new `HostValidator` class to validate hostnames and IP
addresses entered into a `QLineEdit`. We use this class to validate the
server and proxy host.

This also adds the existing `PortValidator` class to the server and
proxy port widgets.

fixes Electron-Cash#2743
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this issue Jan 18, 2024
Summary:
This adds a new `HostValidator` class to validate hostnames and IP
addresses entered into a `QLineEdit`. We use this class to validate the
server and proxy host.

This also adds the existing `PortValidator` class to the server and
proxy port widgets.

fixes [[Electron-Cash/Electron-Cash#2743 | electroncash#2743]]

This is a backport of [[Electron-Cash/Electron-Cash#2744 | electroncash#2744]]

Test Plan: Run the GUI, test the server host, server port, proxy host and proxy port fields. Invalid hostnames or ports should cause the widget to have a red border.

Reviewers: #bitcoin_abc, bytesofman

Reviewed By: #bitcoin_abc, bytesofman

Subscribers: bytesofman

Differential Revision: https://reviews.bitcoinabc.org/D15195
Fabcien pushed a commit to Bitcoin-ABC/ElectrumABC that referenced this issue Jan 19, 2024
Summary:
This adds a new `HostValidator` class to validate hostnames and IP
addresses entered into a `QLineEdit`. We use this class to validate the
server and proxy host.

This also adds the existing `PortValidator` class to the server and
proxy port widgets.

fixes [[Electron-Cash#2743 | electroncash#2743]]

This is a backport of [[Electron-Cash#2744 | electroncash#2744]]

Test Plan: Run the GUI, test the server host, server port, proxy host and proxy port fields. Invalid hostnames or ports should cause the widget to have a red border.

Reviewers: #bitcoin_abc, bytesofman

Reviewed By: #bitcoin_abc, bytesofman

Subscribers: bytesofman

Differential Revision: https://reviews.bitcoinabc.org/D15195
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants