Created attachment 2382 [details] gtk2-socket-handle-interrupted-transmission-in-ssl_write.patch According to the gnutls documentation, gnutls_record_send may indicate an interrupted write. In this case the caller should call the function again with the exact same parameters. Simplify the function and return the value verbatim to the callers. A return code of zero is not an error. Other code paths in sock_write do not consider this as an error either.
Created attachment 2383 [details] master-socket-handle-interrupted-transmission-in-ssl_write.patch
You're changing the value returned when ret=0, aren't you?
I mean, isn't it needed to check everywhere this function is called to reflect the new return code pattern?
It looks like the do loop in the patch has the potential for creating an infinite loop unless gnutls_record_send maintains state to avoid repeatedly returning GNUTLS_E_AGAIN after repeated calls. Agreed, it needs a check on where the return values is being used. Looking at the man page for gnutls_record_send there's an alternative approach: "If you wish to discard the previous data instead of retrying, you must call gnutls_record_discard_queued() before calling this function with different parameters". So adding gnutls_record_discard_queued() prior to returning an error from the existing ssl_write function would be another approach. I guess this cleans up after the error which is currently not being handled.
Actually - gnutls_record_send(): "This function can only be used with transports where send() is an all-or-nothing operation (e.g., UDP). When partial writes are allowed this function will cause session errors." So this is not appropriate for a TCP connection. Currently a gnutls_record_send failure would just trigger an error pop-up, and the user could press Send or Get again. Given a program driven re-try is not a requirement of gnutls_record_send maybe that's good enough?
regarding write size being zero: It can not be the task of ssl_write to decide if a short write is an error. It may be handled by sock_write as an error. session_write_buf handles short writes (but it lacks support for EINTR). oauth2_contact_server lacks support for short writes. In other words, handling of short writes needs a separate bug and a separate patch. regarding interrupted writes: there is plenty of usage of gnutls_record_send in gnutls.git. Some lack error handling, many use the suggested loop. Therefore I think the suggested loop is the preferred way of dealing with the condition. I have not checked other usage of gnutls_record_send outside of gnutls.git. It is unclear why EAGAIN/INTERRUPTED was considered success in commit 81fd89ef633cec2d54b3cff170127cd8d88f544d. Like most other commits, it lacks details about the implementation. I would not bother the user with details that can be dealt with automatically.