Sorry for the late reply!

>> Curl_socket_ready do return the correct value. Since openSSL want us to
>> write information to the socket after a SSL_ERROR_WANT_WRITE error code it
>> seems reasonable that Curl_socket_ready should return 2 (CURL_CSELECT_OUT).
>> But Curl_readwrite only calls the recieve function if we have a readable
>> socket. And therein lies the fault.
>
> Exactly.
>
> We need to separate what to wait for and what to do when the waited-for action
> occurs since in this case the socket gets writable and we should still call
> the read function to take care of it.
>
> When thinking about it, I think we also have the same error for SSH although
> we may not yet have found a case where it becomes visible.
>
>> Insted of using the return value from the Curl_socket_ready (which isn't a
>> good measure of determine what to do since both SSL_write and SSL_read might
>> use both readable and writeable sockets during a renegotiation ) we need
>> some other way of knowing what we are doing. But I don't really see an easy
>> way of doing this.
>
> Curl_socket_ready() is a rather low-level function that needs to stay working
> exactly like it does.
>
> I suggest we introduce a new internal struct member or two that allows the
> protocol-specific parts to set a forced action to assume, and the
> Curl_readwrite() function would then check that field first before it assumes
> that the event implies a specific action.
>
> It could work in a way similar to this (throwing out a suggestion):
>
> Index: lib/transfer.c
> ===================================================================
> RCS file: /cvsroot/curl/curl/lib/transfer.c,v
> retrieving revision 1.442
> diff -u -r1.442 transfer.c
> --- lib/transfer.c 12 Nov 2009 14:36:34 -0000 1.442
> +++ lib/transfer.c 15 Nov 2009 14:28:12 -0000
> @@ -1649,6 +1649,7 @@
> curl_socket_t fd_read;
> curl_socket_t fd_write;
> int select_res = conn->cselect_bits;
> + int act_data; /* how to act on the data */
>
> conn->cselect_bits = 0;
>
> @@ -1675,21 +1676,24 @@
> return CURLE_SEND_ERROR;
> }
>
> - /* We go ahead and do a read if we have a readable socket or if
> - the stream was rewound (in which case we have data in a
> - buffer) */
> + /* We go ahead and do a read if we have a readable socket or if the stream
> + was rewound (in which case we have data in a buffer) */
> if((k->keepon & KEEP_RECV) &&
> - ((select_res & CURL_CSELECT_IN) || conn->bits.stream_was_rewound)) {
> + ((select_res & CURL_CSELECT_IN) || conn->bits.stream_was_rewound))
> + act_data |= (conn->forcewrite?KEEP_SEND:KEEP_RECV);
>
> + /* If we still have writing to do, we check if we have a writable socket. */
> + if((k->keepon & KEEP_SEND) && (select_res & CURL_CSELECT_OUT))
> + act_data |= (conn->forceread?KEEP_RECV:KEEP_SEND);
> +
> + if(act_data & KEEP_RECV) {
> result = readwrite_data(data, conn, k, &didwhat, done);
> if(result || *done)
> return result;
> }
>
> - /* If we still have writing to do, we check if we have a writable socket. */
> - if((k->keepon & KEEP_SEND) && (select_res & CURL_CSELECT_OUT)) {
> + if(act_data & KEEP_SEND) {
> /* write */
> -
> result = readwrite_upload(data, conn, k, &didwhat);
> if(result)
> return result;
>
> It shouldn't be too hard to adapt the SSL stuff to use this concept methinks.
> What you others think?

Well to me it seems like a possible solution. I will try it out as soon as I can.
I't doesn't seem all that hard to make it work.

>
>> One way ( but a quite ugly way I would say ) of solving the problem is to
>> solve the problem in the Curl_ossl_recv function by creating a loop until we
>> don't have a SSL_ERROR_WANT_WRITE or SSL_ERROR_WANT_READ error code ( or
>> until things times out ).
>
> We're already working hard to decrease the amount of blocking places within
> the code and I think adding new blocks without very good reasons is a bad
> idea. I much rather prefer we go the route I suggest above and polish it to
> work...

Well. I also prefer to cure the disease rather than remove the symtoms. I will try your
fix and return the results to you in a couple of days.

>
> --
>
> / daniel.haxx.se
> -------------------------------------------------------------------
> List admin: http://cool.haxx.se/list/listinfo/curl-library
> Etiquette: http://curl.haxx.se/mail/etiquette.html
                                               
_________________________________________________________________
Windows Live: Keep your friends up to date with what you do online.
http://www.microsoft.com/middleeast/windows/windowslive/see-it-in-action/social-network-basics.aspx?ocid=PID23461::T:WLMTAGL:ON:WL:en-xm:SI_SB_1:092010
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2009-12-01