Skip to content

setSocketOption doesn't respect inheritance #7244

Closed
@bertmelis

Description

@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"

Activity

noorhaq

noorhaq commented on Sep 14, 2022

@noorhaq

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

bertmelis

bertmelis commented on Sep 14, 2022

@bertmelis
ContributorAuthor

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

The former is a fix, the latter unpractical.

noorhaq

noorhaq commented on Sep 14, 2022

@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

bertmelis commented on Sep 14, 2022

@bertmelis
ContributorAuthor

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

bertmelis commented on Sep 15, 2022

@bertmelis
ContributorAuthor

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

bertmelis

bertmelis commented on Sep 15, 2022

@bertmelis
ContributorAuthor

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

bertmelis

bertmelis commented on Sep 19, 2022

@bertmelis
ContributorAuthor

@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

VojtechBartoska commented on Sep 19, 2022

@VojtechBartoska
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      setSocketOption doesn't respect inheritance · Issue #7244 · espressif/arduino-esp32