setSocketOption doesn't respect inheritance

[bertmelis]

Core version

latest master (checkout manually)

Issue

https://github.com/espressif/arduino-esp32/blob/master/libraries/WiFi/src/WiFiClient.h#L89

setSocketOption isn't virtual so when calling for a WiFiClientSecure object through a WiFiClient pointer, you'll get an error.
This happens for example when you try to disable Nagle (setNoDelay).

Debug Message

[  8002][V][ssl_client.cpp:62] start_ssl_client(): Free internal heap before TLS 241696
[  8010][V][ssl_client.cpp:68] start_ssl_client(): Starting socket
[  8058][V][ssl_client.cpp:149] start_ssl_client(): Seeding the random number generator
[  8062][V][ssl_client.cpp:158] start_ssl_client(): Setting up the SSL/TLS structure...
[  8066][V][ssl_client.cpp:181] start_ssl_client(): Loading CA cert
[  8083][V][ssl_client.cpp:257] start_ssl_client(): Setting hostname for TLS session...
[  8085][V][ssl_client.cpp:272] start_ssl_client(): Performing the SSL/TLS handshake...
[  9556][V][ssl_client.cpp:293] start_ssl_client(): Verifying peer X.509 certificate...
[  9558][V][ssl_client.cpp:301] start_ssl_client(): Certificate verified.
[  9561][V][ssl_client.cpp:316] start_ssl_client(): Free internal heap after TLS 198284
[  9569][E][WiFiClient.cpp:313] setSocketOption(): fail on -1, errno: 9, "Bad file number"

[noorhaq]

@bertmelis I am facing the same issue. Can you suggest any workaround or fix for it? It would be really helpful

[bertmelis]

Make it virtual (in WiFiClient) or cast your pointer to WiFiClientSecure before calling setSocketOption.

The former is a fix, the latter unpractical.

[noorhaq]

Hi @bertmelis,
I am a newbie on this side and tried by adding virtual keyword just now. No change
Can you add the code example if possible or guide me to a repo where you might have incorporated these changes

[bertmelis]

In WiFiClient.h change lines 89 and 90 from:

    int setSocketOption(int option, char* value, size_t len);
    int setSocketOption(int level, int option, const void* value, size_t len);

to:

    virtual int setSocketOption(int option, char* value, size_t len);
    virtual int setSocketOption(int level, int option, const void* value, size_t len);

setTimeout is discussed in #6676

[bertmelis]

Extra: getNoDelay doesn't work at all because getOption calls raw getsockopt directly. There's no getSocketOption to make virtual.

[bertmelis]

As an alternative, one could implement fd() in WiFiClientSecure and make that virtual.

[bertmelis]

@VojtechBartoska Unless you find this not an issue, I'm happy to create a PR. But there are many roads to Rome and I can only pick one. So guide me in the desired direction.

Imho, we should remove the setSocketOption and getSocketOption from WiFiClientSecure and implement a virtual int fd(). setSocketOption and getSocketOption are duplicate and are just working on a different fd. With a virtual int fd(), also getOption will work for WiFiclientSecure.

[VojtechBartoska]

Hi @bertmelis, thanks for the PR. We will review it and we will see if this is a good option :)