mbedtls: handle WANT_WRITE from mbedtls_ssl_read() #18682
Conversation
|
This one fixes the test problems: diff --git a/lib/vtls/mbedtls.c b/lib/vtls/mbedtls.c
index a8abd0fe09..0a62da2b58 100644
--- a/lib/vtls/mbedtls.c
+++ b/lib/vtls/mbedtls.c
@@ -1113,8 +1113,9 @@ static CURLcode mbed_send(struct Curl_cfilter *cf, struct Curl_easy *data,
int nwritten;
(void)data;
- *pnwritten = 0;
DEBUGASSERT(backend);
+ *pnwritten = 0;
+ connssl->io_need = CURL_SSL_IO_NEED_NONE;
/* mbedtls is picky when a mbedtls_ssl_write) was previously blocked.
* It requires to be called with the same amount of bytes again, or it
* will lose bytes, e.g. reporting all was sent but they were not.
@@ -1135,11 +1136,22 @@ static CURLcode mbed_send(struct Curl_cfilter *cf, struct Curl_easy *data,
else {
CURL_TRC_CF(data, cf, "mbedtls_ssl_write(len=%zu) -> -0x%04X",
len, -nwritten);
- result = ((nwritten == MBEDTLS_ERR_SSL_WANT_WRITE)
+ switch(nwritten) {
#ifdef MBEDTLS_SSL_PROTO_TLS1_3
- || (nwritten == MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET)
+ case MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET:
#endif
- ) ? CURLE_AGAIN : CURLE_SEND_ERROR;
+ case MBEDTLS_ERR_SSL_WANT_READ:
+ connssl->io_need = CURL_SSL_IO_NEED_RECV;
+ result = CURLE_AGAIN;
+ break;
+ case MBEDTLS_ERR_SSL_WANT_WRITE:
+ connssl->io_need = CURL_SSL_IO_NEED_SEND;
+ result = CURLE_AGAIN;
+ break;
+ default:
+ result = CURLE_SEND_ERROR;
+ break;
+ }
if((result == CURLE_AGAIN) && !backend->send_blocked) {
backend->send_blocked = TRUE;
backend->send_blocked_len = len;
@@ -1280,6 +1292,7 @@ static CURLcode mbed_recv(struct Curl_cfilter *cf, struct Curl_easy *data,
(void)data;
DEBUGASSERT(backend);
*pnread = 0;
+ connssl->io_need = CURL_SSL_IO_NEED_NONE;
nread = mbedtls_ssl_read(&backend->ssl, (unsigned char *)buf, buffersize);
if(nread > 0)
@@ -1294,6 +1307,11 @@ static CURLcode mbed_recv(struct Curl_cfilter *cf, struct Curl_easy *data,
FALLTHROUGH();
#endif
case MBEDTLS_ERR_SSL_WANT_READ:
+ connssl->io_need = CURL_SSL_IO_NEED_RECV;
+ result = CURLE_AGAIN;
+ break;
+ case MBEDTLS_ERR_SSL_WANT_WRITE:
+ connssl->io_need = CURL_SSL_IO_NEED_SEND;
result = CURLE_AGAIN;
break;
case MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY: |
|
Analysis of PR #18682 at 1d644082: Test ../../tests/http/test_07_upload.py::TestUpload::test_07_13_upload_seq_large[http/1.1] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Test ../../tests/http/test_07_upload.py::TestUpload::test_07_31_put_10m[http/1.1] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Test ../../tests/http/test_07_upload.py::TestUpload::test_07_50_put_speed_limit[http/1.1] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Test ../../tests/http/test_07_upload.py::TestUpload::test_07_32_issue_10591[http/1.1] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Test ../../tests/http/test_40_socks.py::TestSocks::test_40_04_ul_serial[http/1.1-socks4] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Test ../../tests/http/test_07_upload.py::TestUpload::test_07_60_upload_exp100[http/1.1] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Test ../../tests/http/test_40_socks.py::TestSocks::test_40_04_ul_serial[http/1.1-socks5] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). There are more failures, but that's enough from Gha. Generated by Testclutch |
icing
left a comment
There was a problem hiding this comment.
the need flag needs clearing. See comment with patch.
|
Thanks! |
The mbedtls_ssl_read() function is documented to be able to also return MBEDTLS_ERR_SSL_WANT_WRITE, so act on that accordingly instead of returning error for it. Fixed-by: Stefan Eissing Reported in Joshua's sarif data
1d64408 to
a1e4827
Compare
The mbedtls_ssl_read() function is documented to be able to also return MBEDTLS_ERR_SSL_WANT_WRITE, so act on that accordingly instead of returning error for it.
Reported in Joshua's sarif data