Esp-idf: [TW#17306] libcurl fails with "Connection already in progress"

[paulreimer]

As noted in this issue:
https://github.com/nkolban/esp32-snippets/issues/219

I get a similar error with HTTPS using mbedTLS, as well:

* Rebuilt URL to: https://www.howsmyssl.com/
* timeout on name lookup is not supported
*   Trying 104.196.190.195...
* TCP_NODELAY set
* connect to 104.196.190.195 port 443 failed: Connection already in progress
* Failed to connect to www.howsmyssl.com port 443: Connection already in progress
* Closing connection 0
I (6156) curl_test: curl_easy_perform failed: Couldn't connect to server

Specifically these commits appear to break libcurl which worked before (e.g. on commit a320fed and before)
https://github.com/espressif/esp-idf/compare/a320fed...3f83914

I should note that libcurl does currently work on the latest release v2.1, but I ran git bisect to find the exact commits that broke it.

[loboris]

My ESP32 curl example builds and runs with the latest esp-idf without any error.
It runs http/https GET/POST, FTP tests, SFTP tests, SMTP - Email test (GMail), ...

[paulreimer]

Thanks @loboris, I can confirm that your curl component in the repo above does work for me using the latest esp-idf master (I can even use my curl_config.h that wasn't working above, but I have to comment out SIZEOF_CURL_OFF_T).

You also included libssh2, quickmail, and zlib to support more curl features, which I must say is awesome.

However I'm trying to figure out where you got the curl sources from. I see in your include/curl/curlver.h

#define LIBCURL_VERSION "7.54.1-DEV"

But there are significant diffs from the official curl v7.54.1 release sources. I'd prefer to be able to track upstream curl, at least know where the patches came from, etc.

[loboris]

The libcurl sources were cloned from curl repository.
Unfortunately, I don't remember the exact commit which was used.
I had to do some modifications to make it work with with ESP32.
I'll try to update the sources to the latest curl commit and document the needed changes when I find some time to do it.

[loboris]

The problem with Connection already in progress is caused by the fact that the lwip sockets starts at LWIP_SOCKET_OFFSET.

FD_SET/FD_GET macros take this into account when operating on fd sets: LWIP_SOCKET_OFFSET is subtracted from fd number, and the resulting value as used as bit offset in fd set.

In curl (file _lib/select.h_), the macro #define VALID_SOCK(s) (((s) >= 0) && ((s) < FD_SETSIZE)) checks the valid socket and fails with esp-idf lwip socket.

_This macro has to be changed to_:
#define VALID_SOCK(s) (((s) - LWIP_SOCKET_OFFSET >= 0) && ((s) - LWIP_SOCKET_OFFSET < FD_SETSIZE)),
then everything works as expected.

I've tested my ESP32 curl example (updated) with latest libcurl cloned from curl repository and it builds and runs without problem with this patch.

_Only directories lib and include from curl repository needs to be used._

You can download all the patches I'm using from my server.

=========
cURL INFO
=========

Curl version info
  version: 7.58.0-DEV - 473600
Host: ESP32
- IP V6 supported
- SSL supported
- LIBZ supported
- NTLM NOT supported
- DEBUG NOT supported
- UNIX sockets NOT supported
Protocols:
- dict
- file
- ftp
- ftps
- gopher
- http
- https
- imap
- imaps
- pop3
- pop3s
- rtsp
- scp
- sftp
- smtp
- smtps
- telnet
- tftp

I (6282) wifi: pm start, type:0

Starting in 1 seconds...   

--------------------------------------------------------
I (7458) [cURL Example]: === PASS 1; restart_cnt=0 ===
--------------------------------------------------------


#### HTTP GET
     Send GET request with parameters
* timeout on name lookup is not supported
*   Trying 82.196.4.208...
* TCP_NODELAY set
* Connected to loboris.eu (82.196.4.208) port 80 (#0)
> GET /ESP32/test.php?par1=765&par2=test HTTP/1.1
Host: loboris.eu
Accept: */*
Accept-Encoding: deflate, gzip

< HTTP/1.1 200 OK
< Date: Sun, 31 Dec 2017 12:34:58 GMT
< Server: Apache/2.4.7 (Ubuntu)
< X-Powered-By: PHP/5.5.9-1ubuntu4.22
< Vary: Accept-Encoding
< Content-Encoding: gzip
< Content-Length: 212
< Content-Type: text/html
< 
* Closing connection 0

____________ Response BODY: ____________

============
Method: GET
============

--------------------------
Number of uploaded files: 0
    Number of GET params: 2
   Number of POST params: 0
--------------------------

===========
Debug info:
===========

-----------
POST DATA: 
-----------
Array
(
)
-----------
 GET DATA:  
-----------
Array
(
    [par1] => 765
    [par2] => test
)
-----------
FILE DATA: 
-----------
Array
(
)
-----------

=====================================
LoBo test server, 2017/12/31 13:34:58
=====================================

^^^^^^^^^^^^ Response BODY: ^^^^^^^^^^^^


#### HTTP GET BIG FILE
     Get big file (~2.4 MB), simulate saving to file system
* timeout on name lookup is not supported
*   Trying 82.196.4.208...
* TCP_NODELAY set
* Connected to loboris.eu (82.196.4.208) port 80 (#0)
> GET /ESP32/bigimg.jpg HTTP/1.1
Host: loboris.eu
Accept: */*
Accept-Encoding: deflate, gzip

< HTTP/1.1 200 OK
< Date: Sun, 31 Dec 2017 12:35:10 GMT
< Server: Apache/2.4.7 (Ubuntu)
< Last-Modified: Sun, 29 May 2016 16:55:19 GMT
< ETag: "25090a-533fe013c6bc0"
< Accept-Ranges: bytes
< Content-Length: 2427146
< Content-Type: image/jpeg
< 
* Download: received 803924
* Download: received 1602340
* Download: received 2417988
* Closing connection 0
* Download: received 2427146 B; time=15.1 s; speed=156.9 KB/sec

____________ Response BODY: ____________
SIMULATED save to file; size=2427146
^^^^^^^^^^^^ Response BODY: ^^^^^^^^^^^^


#### HTTPS (SSL) GET
     Send GET request to secure (SSL) server
* timeout on name lookup is not supported
*   Trying 198.128.229.135...
* TCP_NODELAY set
* Connected to www-eng-x.llnl.gov (198.128.229.135) port 443 (#0)
* Error reading ca cert file /certs/ca-certificates.crt - mbedTLS: (-0x3E00) PK - Read/write of file failed
* mbedTLS: Connecting to www-eng-x.llnl.gov:443
* mbedTLS: Set min SSL version to TLS 1.0
* mbedTLS: Handshake complete, cipher is TLS-ECDHE-RSA-WITH-AES-256-GCM-SHA384
* Dumping cert info:
* cert. version     : 3
* serial number     : 03:F7:4B:3A:03:E8:45:AC:D1:EC:73:19:EE:0D:53:45:1A:DF
* issuer name       : C=US, O=Let's Encrypt, CN=Let's Encrypt Authority X3
* subject name      : CN=www-br.llnl.gov
* issued  on        : 2017-12-01 07:44:14
* expires on        : 2018-03-01 07:44:14
* signed using      : RSA with SHA-256
* RSA key size      : 2048 bits
* basic constraints : CA=false
* subject alt name  : www-br.llnl.gov, www-br.ucllnl.org, www-eng-x.llnl.gov
* key usage         : Digital Signature, Key Encipherment
* ext key usage     : TLS Web Server Authentication, TLS Web Client Authentication

* SSL connected
> GET /documents/a_document.txt HTTP/1.1
Host: www-eng-x.llnl.gov
Accept: */*
Accept-Encoding: deflate, gzip

< HTTP/1.1 200 OK
< Date: Sun, 31 Dec 2017 12:35:28 GMT
< Server: Apache
< Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
< Last-Modified: Thu, 20 Apr 1995 07:00:00 GMT
< ETag: "6e-2d61af3ba7c00"
< Accept-Ranges: bytes
< Content-Length: 110
< Connection: close
< Content-Type: text/plain; charset=UTF-8
< 
* Closing connection 0

____________ Response BODY: ____________
This is a test

The quick brown fox jumped over the lazy dogs.
1234567890!@#$%^&*()[]{}-=_+;:'",.<>/?`~\|

^^^^^^^^^^^^ Response BODY: ^^^^^^^^^^^^

[paulreimer]

@loboris Thank you! That macro change did resolve the issue for me, and I was also able to use the latest upstream curl-7_57_0 (I already have a curl_config.h, and I do not need your ssh protocol patch at this time).

I never would have found where/how to modify that curl macro, either, so thanks again! My immediate concerns (for libcurl) are resolved.

However, as far as esp-idf goes, is this just an expected breaking change for 3.0? If it breaks libcurl (in quite a mysterious way), is it going to break other ported software, too?

Ideally there is a way to avoid this that can be added to esp-idf, but I am also curious in how you (@loboris) debugged this issue? I can kinda see that it would impact socket/select/FD_ related things, and just hunt in the ported code for their usage (and re-adjust by the offset)?

[loboris]

I think the libcurl (and other libraries using lwip) should be aware of lwip's LWIP_SOCKET_OFFSET.
As libcurl is lwip aware (USE_LWIPSOCK), the conditional compile should be used where LWIP_SOCKET_OFFSET can alter the code behavior.
This should be probably submited to curl repository as PR.

In esp-idf, LWIP is a special case for VFS use, and LWIP_SOCKET_OFFSET is set dynamically at VFS registration time. I've had some other issues since this behaviour was implemented, so it was easy to find the solution for curl issue.

[projectgus]

However, as far as esp-idf goes, is this just an expected breaking change for 3.0? If it breaks libcurl (in quite a mysterious way), is it going to break other ported software, too?

Good point. I've added a note to the release notes here:
https://github.com/espressif/esp-idf/releases/tag/v3.0-rc1
(Search for LWIP_SOCKET_OFFSET.)

This wasn't intended to be a breaking change (POSIX functions may return non-negative int as a valid fd), but it seems quite a lot of code assumes fds will always be low numbered integers (reasonable, as they usually are).

Some good news, LWIP_SOCKET_OFFSET will probably be going back to zero (or close to zero) in IDF v3.1 when we plan to expand select() to work with other VFS drivers. (Code updated for v3.0, like the modification for libcurl mentioned here, should continue to work.)

[igrr]

Some news here, LWIP_SOCKET_OFFSET is not being used anymore. Lwip sockets once again use small numbers.

[paulreimer]

I find that is not that case, at least with latest master I still get this error.

At least, I still require this patch in libcurl:

//#define VALID_SOCK(s) (((s) >= 0) && ((s) < FD_SETSIZE))
// Adjust the fd # by the LWIP_SOCKET_OFFSET to make it 0-based again
#define VALID_SOCK(s) (((s) - LWIP_SOCKET_OFFSET >= 0) && ((s) - LWIP_SOCKET_OFFSET < FD_SETSIZE))

[igrr]

We had to revert VFS changes soon after they were done, due to a regression. Updated fix coming back soon.

[paulreimer]

Safe to test this out yet?