From 4f28e5555c543a2d754f78ccb609705fb14f49e7 Mon Sep 17 00:00:00 2001
From: Markus Zolliker <markus.zolliker@psi.ch>
Date: Mon, 7 Aug 2023 13:18:28 +0200
Subject: [PATCH] improve error message on client when host/port is bad

for host name without port, None was used for the port
leading to a confusing error message

- do not call parse_host_port with None as defaultport argument
- improve error message when connection fails

+ fix an error in last line of parse_ipv6_host_and_port
+ fix some issues breaking PEP 8 rules

Change-Id: I437360be96449c164f0080e3c60f1685825d4780
Reviewed-on: https://forge.frm2.tum.de/review/c/secop/frappy/+/31911
Tested-by: Jenkins Automated Tests <pedersen+jenkins@frm2.tum.de>
Reviewed-by: Enrico Faulhaber <enrico.faulhaber@frm2.tum.de>
Reviewed-by: Markus Zolliker <markus.zolliker@psi.ch>
---
 frappy/lib/__init__.py | 24 +++++++++++++++---------
 frappy/lib/asynconn.py |  6 +++---
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/frappy/lib/__init__.py b/frappy/lib/__init__.py
index 6e639a9..8b67cc2 100644
--- a/frappy/lib/__init__.py
+++ b/frappy/lib/__init__.py
@@ -1,4 +1,3 @@
-#  -*- coding: utf-8 -*-
 # *****************************************************************************
 #
 # This program is free software; you can redistribute it and/or modify it under
@@ -17,6 +16,8 @@
 #
 # Module authors:
 #   Enrico Faulhaber <enrico.faulhaber@frm2.tum.de>
+#   Markus Zolliker <markus.zolliker@psi.ch>
+#   Alexander Zaft <a.zaft@fz-juelich.de>
 #
 # *****************************************************************************
 """Define helpers"""
@@ -32,6 +33,9 @@ from configparser import ConfigParser
 from os import environ, path
 
 
+SECoP_DEFAULT_PORT = 10767
+
+
 class GeneralConfig:
     """generalConfig holds server configuration items
 
@@ -297,6 +301,7 @@ def formatException(cut=0, exc_info=None, verbose=False):
 
 HOSTNAMEPART = re.compile(r'^(?!-)[a-z0-9-]{1,63}(?<!-)$', re.IGNORECASE)
 
+
 def validate_hostname(host):
     """checks if the rules for valid hostnames are adhered to"""
     if len(host) > 255:
@@ -325,7 +330,7 @@ def validate_ipv6(addr):
     return True
 
 
-def parse_ipv6_host_and_port(addr, defaultport=10767):
+def parse_ipv6_host_and_port(addr, defaultport=SECoP_DEFAULT_PORT):
     """ Parses IPv6 addresses with optional port. See parse_host_port for valid formats"""
     if ']' in addr:
         host, port = addr.rsplit(':', 1)
@@ -333,9 +338,10 @@ def parse_ipv6_host_and_port(addr, defaultport=10767):
     if '.' in addr:
         host, port = addr.rsplit('.', 1)
         return host, int(port)
-    return (host, defaultport)
+    return addr, defaultport
 
-def parse_host_port(host, defaultport=10767):
+
+def parse_host_port(host, defaultport=SECoP_DEFAULT_PORT):
     """Parses hostnames and IP (4/6) addressses.
 
     The accepted formats are:
@@ -346,16 +352,16 @@ def parse_host_port(host, defaultport=10767):
     - IPv6 addresses in the forms '[IPv6]:port' or 'IPv6.port'
     """
     colons = host.count(':')
-    if colons == 0: # hostname/ipv4 wihtout port
+    if colons == 0:  # hostname/ipv4 without port
         port = defaultport
-    elif colons == 1: # hostname or ipv4 with port
+    elif colons == 1:  # hostname or ipv4 with port
         host, port = host.split(':')
         port = int(port)
-    else: # ipv6
+    else:  # ipv6
         host, port = parse_ipv6_host_and_port(host, defaultport)
     if (validate_ipv4(host) or validate_hostname(host) or validate_ipv6(host)) \
-        and 0 < port < 65536:
-        return (host, port)
+            and 0 < port < 65536:
+        return host, port
     raise ValueError(f'invalid host {host!r} or port {port}')
 
 
diff --git a/frappy/lib/asynconn.py b/frappy/lib/asynconn.py
index 143d58a..7b4b1db 100644
--- a/frappy/lib/asynconn.py
+++ b/frappy/lib/asynconn.py
@@ -35,7 +35,7 @@ import time
 import re
 
 from frappy.errors import CommunicationFailedError, ConfigError
-from frappy.lib import closeSocket, parse_host_port
+from frappy.lib import closeSocket, parse_host_port, SECoP_DEFAULT_PORT
 
 try:
     from serial import Serial
@@ -176,11 +176,11 @@ class AsynTcp(AsynConn):
             uri = uri[6:]
         try:
 
-            host, port = parse_host_port(uri, self.default_settings.get('port'))
+            host, port = parse_host_port(uri, self.default_settings.get('port', SECoP_DEFAULT_PORT))
             self.connection = socket.create_connection((host, port), timeout=self.timeout)
         except (ConnectionRefusedError, socket.gaierror, socket.timeout) as e:
             # indicate that retrying might make sense
-            raise CommunicationFailedError(str(e)) from None
+            raise CommunicationFailedError(f'can not connect to {host}:{port}, {e}') from None
 
     def shutdown(self):
         if self.connection: