curl-library
RE: further schannel improvements
Date: Fri, 15 Jun 2012 00:24:02 +0000
>> I'd appreciate any feedback or questions anyone has on my patches.
>The biggest nits I spotted are the loops you introduce in 0004 and 0005.
>First, since you loop on EAGAIN it will make the code busy-loop like crazy during periods and waste a lot of CPU. Any loop on EAGAIN *must* wait for the proper socket activity until it retries the operation.
Daniel,
Good point, how does this look?
*err = Curl_socket_ready(CURL_SOCKET_BAD, conn->sock[sockindex],
timeout_ms);
if(*err < 0) {
/* fatal error */
failf(conn->data, "select/poll on SSL socket, errno: %d",
SOCKERRNO);
break;
}
Marc, Yang,
Thanks for your excellent feedback. I think I've incorporated all suggestions. These patches should be a little easier to read; after rebasing some of the things I was doing have been done already.
Marc, I believe the intermittent problem connecting was most likely due to a missing "break" statement in the send function. At first glance it seems that the break statement might not be necessary since the loop continues while bytes written < len. However, len is the number of encrypted bytes to send (larger that the number of unencrypted), and written is the number of encrypted bytes sent. Before returning from the function "written" should be back to the number of un-encrypted bytes requested to be sent. I did this but then it's necessary to force a break out of the loop with the break statement.
Another change here is that I made the shutdown method delete the security context and buffers. I think this is the right thing to do; it says on MSDN that we should delete the security context when shutting down the SSL connection. This turns the close function into nothing more than a check that we've shutdown already. I haven't actually tried to downgrade an SSL connection back to a regular connection. I think the only place in libcurl that does this is FTP. It would be good to know if it actually works.
Unfortunately as I went through the patches numerous times with an interactive rebase I lost one of my patches, the one that fixed the connect hang I saw on WinCE that was caused by checking the socket while we have additional data at the end of our handshake in the 'extra' buffer. I'll get this patch back out tomorrow.
Mark
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
- application/octet-stream attachment: 0002-schannel-SSL-Use-standard-Curl-read-write-methods.patch
- application/octet-stream attachment: 0003-schannel-SSL-Added-helper-methods-to-simplify-code.patch
- application/octet-stream attachment: 0004-schannel-SSL-Made-send-method-handle-unexpected-case.patch
- application/octet-stream attachment: 0005-schannel-SSL-certificate-validation-on-WinCE.patch
- application/octet-stream attachment: 0006-schannel-SSL-Implemented-SSL-shutdown.patch
- application/octet-stream attachment: 0001-SSPI-related-code-Unicode-support-for-WinCE.patch
- application/x-zip-compressed attachment: patches.zip