Mongoose update
Issue
Solves #757. The webserver should use mongoose 7 instead of mongoose 6, for not using deprecated OpenSSL functions. Furthermore, the test cases simulating slow connections should be fixed.
Tasks
- [x] Update code, so that it supports the API of mongoose 7
- [x] Find a way to simulate slow connections
- [x] Update certificates to make the HTTPS tests work (see #773)
- [x] Fix failing tests
- [x] Fix remaining merge conflicts
- [x] ~~Fix failing windows tests~~
API Changes in mongoose 7
Building
- Parameter
MG_ENABLE_SSLis nowMG_ENABLE_OPENSSL
Structural changes
A few functionalities got outsourced to the callback
- The context (in our case a pointer to an
HttpServerorHttpsServerobject) is passed as a callback parameter - The TLS connection gets initialized inside the callback for each connection, via the
mg_tls_initmethod
Methods -- Renames and changes
mg_send_headandmg_printfare nowmg_http_reply. For each header,\r\nneeds to be appended manuallymg_http_send_errordoes not exist anymore. We replace it with a staticSendErrormethod inAbstractServermg_http_listenis now used for binding and listening instead ofmg_bind
Structures -- Renames and changes
- Trivial renaming of attributes, e.g. pointer attributes formerly named
pare nowptr http_messageis nowmg_http_message. HTTP headers are now stored in an array ofstruct mg_http_header
Further details
This list of changes is not complete. There are some further minor changes. For further information about mongoose 7, see the mongoose documentation.
Simulation of slow connections
TLDR; It works with mg_timer structs provided by mongoose (also see timer documentation, timer example). For further information, also see the comment below.
Since I have come up with a solution to simulate slow connections, a few tests are failing. One problem is that if only the header of the response is received, libcurl stores the status code regardless of whether the request succeeded or not. All tests with timeouts expect the response code to be 0, but it is 200, as the header gets transmitted before the body.
My question now is: Is this an issue at the implementation of libcurl, libcpr, or the tests of libcpr?
In my eyes this is a libcurl bug. Would you like to create an issue for that in libcurl?
Yes, I created an issue here.
Update: The issue I created at libcurl was marked not-a-bug and closed. The reasoning was that libcurl's behavior reflects the behavior from the documentation, which is that the response code is only 0 if no response was received at all.
My suggestion would be to leave the implementation of libcpr as it is and to remove the failing assertions in the tests. This way we stay consistent with libcurl. To be honest, I agree with the response I got: a user can and should check for specific errors with the error codes provided by libcurl or the response.error attribute in our case.
Aha, OK. Interesting. Yes, then simply remove the assertion and keep it like that. Please add a note to this issue here inside the code, why we don't assert for the status code here.
Simulating slow connections with mongoose
I finally came up with a way to simulate slow connections with mongoose without nested calls of mg_mgr_poll which lead to a lot of problems. Here is a short summary:
The naive approach
- Manually construct an HTTP header inside the
mg_printfmethod - Drain header with
mg_mgr_poll - Wait some time
- Send the remaining part(s) of the message
The problem with this approach is: We have nested calls of mg_mgr_poll: The response handling method is called from mg_mgr_poll and we call mg_mgr_poll from within the response handling method. This leads to two kinds of issues:
- The linked list that stores the connections is traversed in a loop. If the list gets updated by nested
mg_mgr_pollcalls, this confuses the formermg_mgr_pollcalls and potentially leads to segfaults and/or memory leaks - We stay in the response method of a specific endpoint while handling a request to another endpoint. This makes the two methods interfere with each other
Using mg_timer structs
- Register timers with (lambda) functions that simply send the message we want to send
- Set an according timeout
- As a parameter to the timer, pass a helper structure containing necessary information about the connection
- All helper structures are stored in a vector of
unique_ptrs in the AbstractServer object
- On each iteration of
mg_mgr_poll, all registered timers are called - Once a message has been fully sent, the counter is set to a value that doesn't fulfill the sending condition anymore, thus deactivating the timer
- In the end, all timer arguments are deleted by deleting the aforementioned vector
Update. Just when I thought my work was finally coming to an end, the tests on windows cause issues...
Windows Port issues
Debugging with Wireshark
As you can see from the CI checks, the tests setting the port of the server as source port are failing. For some reason, binding a socket to the port that is occupied by the server works under windows. This leads to the following network communication:
We can see messages sent from port 61936 to exactly the same port 61936. This should definitely not happen.
Curl Request on the test server
This is very unlikely a problem of libcpr, since the same happens when performing the request with curl:
$ curl -v --local-port 61936 127.0.0.1:61936/hello.html
* Trying 127.0.0.1:61936...
* Local port: 61936
* Connected to 127.0.0.1 (127.0.0.1) port 61936 (#0)
> GET /hello.html HTTP/1.1
> Host: 127.0.0.1:61936
> User-Agent: curl/7.84.0
> Accept: */*
>
* Received HTTP/0.9 when not allowed
* Closing connection 0
curl: (1) Received HTTP/0.9 when not allowed
When performing the same request again, I get:
$ curl -v --local-port 61936 127.0.0.1:61936/not-existing.html
* Trying 127.0.0.1:61936...
* Local port: 61936
* connect to 127.0.0.1 port 61936 failed: Address already in use
* Failed to connect to 127.0.0.1 port 61936 after 12 ms: Address already in use
* Closing connection 0
curl: (7) Failed to connect to 127.0.0.1 port 61936 after 12 ms: Address already in use
This is still not fully the expected behaviour, since the curl error code is different (for the expected behaviour see below).
Curl Request on a simple Python Server
In my eyes, this seems to be a problem of mongoose under windows, since I have tried performing the requests on a simple python server, which instantly showed the expected behaviour:
$ curl -v --local-port 8080 127.0.0.1:8080/index.html
* Trying 127.0.0.1:8080...
* bind failed with errno 10048: Address already in use
* Closing connection 0
curl: (45) bind failed with errno 10048: Address already in use
Update
Giving up on this. I tried some simple fixes, and I even tried to write a dedicated test for windows where I create and bind a socket manually and then use this port as a local port. It seemed like this didn't work as well and it even made another test fail. I would simply suggest skipping these tests for windows.
After I gave up on the tests LocalPortTests.SetLocalPortTestOccupied and LocalPortTests.SetOptionTestOccupied (which I commented out for windows, see previous comment), and solved the remaining merge conflicts, this PR should finally be ready to be reviewed and merged :)
What is the state of this PR?
Now that I fixed all the merge conflicts again, this PR is again ready to be merged. In one pipeline for macOS MultiperformGetTests.MultiperformTenSessionsGetTest fails, but I have no idea why.
Ready to go. Thanks for your great work!