Uniform behaviour of WiFiClientSecure and WiFiClient setTimeout() (#6562)

* Uniform timeout WiFiClient-WiFiClientSecure

* Added missing prototype

* Add socket check on setTimeout

Author: gonzabrusco

Diff for: libraries/WiFi/src/WiFiClient.cpp

@@ -175,11 +175,11 @@ class WiFiClientSocketHandle {
     }
 };
 
-WiFiClient::WiFiClient():_connected(false),next(NULL)
+WiFiClient::WiFiClient():_connected(false),_timeout(WIFI_CLIENT_DEF_CONN_TIMEOUT_MS),next(NULL)
 {
 }
 
-WiFiClient::WiFiClient(int fd):_connected(true),next(NULL)
+WiFiClient::WiFiClient(int fd):_connected(true),_timeout(WIFI_CLIENT_DEF_CONN_TIMEOUT_MS),next(NULL)
 {
     clientSocketHandle.reset(new WiFiClientSocketHandle(fd));
     _rxBuffer.reset(new WiFiClientRxBuffer(fd));
@@ -208,10 +208,11 @@ void WiFiClient::stop()
 
 int WiFiClient::connect(IPAddress ip, uint16_t port)
 {
-    return connect(ip,port,WIFI_CLIENT_DEF_CONN_TIMEOUT_MS);
-   }
-   int WiFiClient::connect(IPAddress ip, uint16_t port, int32_t timeout   )
+    return connect(ip,port,_timeout);
+}
+int WiFiClient::connect(IPAddress ip, uint16_t port, int32_t timeout)
 {
+    _timeout = timeout;
     int sockfd = socket(AF_INET, SOCK_STREAM, 0);
     if (sockfd < 0) {
         log_e("socket: %d", errno);
@@ -230,7 +231,7 @@ int WiFiClient::connect(IPAddress ip, uint16_t port)
     FD_ZERO(&fdset);
     FD_SET(sockfd, &fdset);
     tv.tv_sec = 0;
-    tv.tv_usec = timeout * 1000;
+    tv.tv_usec = _timeout * 1000;
 
 #ifdef ESP_IDF_VERSION_MAJOR
     int res = lwip_connect(sockfd, (struct sockaddr*)&serveraddr, sizeof(serveraddr));
@@ -243,13 +244,13 @@ int WiFiClient::connect(IPAddress ip, uint16_t port)
         return 0;
     }
 
-    res = select(sockfd + 1, nullptr, &fdset, nullptr, timeout<0 ? nullptr : &tv);
+    res = select(sockfd + 1, nullptr, &fdset, nullptr, _timeout<0 ? nullptr : &tv);
     if (res < 0) {
         log_e("select on fd %d, errno: %d, \"%s\"", sockfd, errno, strerror(errno));
         close(sockfd);
         return 0;
     } else if (res == 0) {
-        log_i("select returned due to timeout %d ms for fd %d", timeout, sockfd);
+        log_i("select returned due to timeout %d ms for fd %d", _timeout, sockfd);
         close(sockfd);
         return 0;
     } else {
@@ -270,6 +271,14 @@ int WiFiClient::connect(IPAddress ip, uint16_t port)
         }
     }
 
+#define ROE_WIFICLIENT(x,msg) { if (((x)<0)) { log_e("LWIP Socket config of " msg " failed."); return -1; }}
+    ROE_WIFICLIENT(lwip_setsockopt(sockfd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)),"SO_RCVTIMEO");
+    ROE_WIFICLIENT(lwip_setsockopt(sockfd, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv)),"SO_SNDTIMEO");
+
+    // These are also set in WiFiClientSecure, should be set here too?
+    //ROE_WIFICLIENT(lwip_setsockopt(sockfd, IPPROTO_TCP, TCP_NODELAY, &enable, sizeof(enable)),"TCP_NODELAY"); 
+    //ROE_WIFICLIENT (lwip_setsockopt(sockfd, SOL_SOCKET, SO_KEEPALIVE, &enable, sizeof(enable)),"SO_KEEPALIVE");
+
     fcntl( sockfd, F_SETFL, fcntl( sockfd, F_GETFL, 0 ) & (~O_NONBLOCK) );
     clientSocketHandle.reset(new WiFiClientSocketHandle(sockfd));
     _rxBuffer.reset(new WiFiClientRxBuffer(sockfd));
@@ -279,9 +288,10 @@ int WiFiClient::connect(IPAddress ip, uint16_t port)
 
 int WiFiClient::connect(const char *host, uint16_t port)
 {
-    return connect(host,port,WIFI_CLIENT_DEF_CONN_TIMEOUT_MS);
-   }
-   int WiFiClient::connect(const char *host, uint16_t port, int32_t timeout   )
+    return connect(host,port,_timeout);
+}
+
+int WiFiClient::connect(const char *host, uint16_t port, int32_t timeout)
 {
     IPAddress srv((uint32_t)0);
     if(!WiFiGenericClass::hostByName(host, srv)){
@@ -301,14 +311,20 @@ int WiFiClient::setSocketOption(int option, char* value, size_t len)
 
 int WiFiClient::setTimeout(uint32_t seconds)
 {
-    Client::setTimeout(seconds * 1000);
-    struct timeval tv;
-    tv.tv_sec = seconds;
-    tv.tv_usec = 0;
-    if(setSocketOption(SO_RCVTIMEO, (char *)&tv, sizeof(struct timeval)) < 0) {
-        return -1;
+    Client::setTimeout(seconds * 1000); // This should be here?
+    _timeout = seconds * 1000;
+    if(fd() >= 0) {
+        struct timeval tv;
+        tv.tv_sec = seconds;
+        tv.tv_usec = 0;
+        if(setSocketOption(SO_RCVTIMEO, (char *)&tv, sizeof(struct timeval)) < 0) {
+            return -1;
+        }
+        return setSocketOption(SO_SNDTIMEO, (char *)&tv, sizeof(struct timeval));
+    }
+    else {
+        return 0;
     }
-    return setSocketOption(SO_SNDTIMEO, (char *)&tv, sizeof(struct timeval));
 }
 
 int WiFiClient::setOption(int option, int *value)

Diff for: libraries/WiFi/src/WiFiClient.h

@@ -42,6 +42,7 @@ class WiFiClient : public ESPLwIPClient
     std::shared_ptr<WiFiClientSocketHandle> clientSocketHandle;
     std::shared_ptr<WiFiClientRxBuffer> _rxBuffer;
     bool _connected;
+    int _timeout;
 
 public:
     WiFiClient *next;

Diff for: libraries/WiFiClientSecure/src/WiFiClientSecure.cpp

@@ -32,6 +32,7 @@
 WiFiClientSecure::WiFiClientSecure()
 {
     _connected = false;
+    _timeout = 30000; // Same default as ssl_client
 
     sslclient = new sslclient_context;
     ssl_init(sslclient);
@@ -52,7 +53,7 @@ WiFiClientSecure::WiFiClientSecure()
 WiFiClientSecure::WiFiClientSecure(int sock)
 {
     _connected = false;
-    _timeout = 0;
+    _timeout = 30000; // Same default as ssl_client
 
     sslclient = new sslclient_context;
     ssl_init(sslclient);
@@ -128,9 +129,6 @@ int WiFiClientSecure::connect(IPAddress ip, uint16_t port, const char *CA_cert,
 
 int WiFiClientSecure::connect(const char *host, uint16_t port, const char *CA_cert, const char *cert, const char *private_key)
 {
-    if(_timeout > 0){
-        sslclient->handshake_timeout = _timeout;
-    }
     int ret = start_ssl_client(sslclient, host, port, _timeout, CA_cert, _use_ca_bundle, cert, private_key, NULL, NULL, _use_insecure, _alpn_protos);
     _lastError = ret;
     if (ret < 0) {
@@ -148,9 +146,6 @@ int WiFiClientSecure::connect(IPAddress ip, uint16_t port, const char *pskIdent,
 
 int WiFiClientSecure::connect(const char *host, uint16_t port, const char *pskIdent, const char *psKey) {
     log_v("start_ssl_client with PSK");
-    if(_timeout > 0){
-        sslclient->handshake_timeout = _timeout;
-    }
     int ret = start_ssl_client(sslclient, host, port, _timeout, NULL, false, NULL, NULL, pskIdent, psKey, _use_insecure, _alpn_protos);
     _lastError = ret;
     if (ret < 0) {
@@ -365,3 +360,27 @@ void WiFiClientSecure::setAlpnProtocols(const char **alpn_protos)
 {
     _alpn_protos = alpn_protos;
 }
+int WiFiClientSecure::setTimeout(uint32_t seconds)
+{
+    _timeout = seconds * 1000;
+    if (sslclient->socket >= 0) {
+        struct timeval tv;
+        tv.tv_sec = seconds;
+        tv.tv_usec = 0;
+        if(setSocketOption(SO_RCVTIMEO, (char *)&tv, sizeof(struct timeval)) < 0) {
+            return -1;
+        }
+        return setSocketOption(SO_SNDTIMEO, (char *)&tv, sizeof(struct timeval));
+    }
+    else {
+        return 0;
+    }
+}
+int WiFiClientSecure::setSocketOption(int option, char* value, size_t len)
+{
+    int res = setsockopt(sslclient->socket, SOL_SOCKET, option, value, len);
+    if(res < 0) {
+        log_e("%X : %d", option, errno);
+    }
+    return res;
+}

Diff for: libraries/WiFiClientSecure/src/WiFiClientSecure.h

@@ -32,7 +32,7 @@ class WiFiClientSecure : public WiFiClient
 
     int _lastError = 0;
     int _peek = -1;
-    int _timeout = 0;
+    int _timeout;
     bool _use_insecure;
     const char *_CA_cert;
     const char *_cert;
@@ -79,7 +79,8 @@ class WiFiClientSecure : public WiFiClient
     void setAlpnProtocols(const char **alpn_protos);
     const mbedtls_x509_crt* getPeerCertificate() { return mbedtls_ssl_get_peer_cert(&sslclient->ssl_ctx); };
     bool getFingerprintSHA256(uint8_t sha256_result[32]) { return get_peer_fingerprint(sslclient, sha256_result); };
-    int setTimeout(uint32_t seconds){ return 0; }
+    int setTimeout(uint32_t seconds);
+    int setSocketOption(int option, char* value, size_t len);
 
     operator bool()
     {