curl (linux/gdk) networking improvements
Created attachment 13653 [details] curl networking improvements Curl (linux/gdk) networking improvements: a) POST support b) don't use CURL in a re-entrant way. c) for easier debugging, in debug build if env var DEBUG_CURL is set, turn on curl's internal debbuging
Comment on attachment 13653 [details] curl networking improvements + if ((CURLE_OK == err) && ((301 == httpCode) || (302 == httpCode))) + return totalSize; + You have unneeded parenthesis here, and in several other places. + CString cstring = elements[i].m_filename.utf8(); An extra space after the =. + // Fill in the file upload field + curl_formadd(&post, + &lastItem, + CURLFORM_COPYNAME, "sendfile", + CURLFORM_FILE, d->m_fileName, + CURLFORM_END); + + // Fill in the filename field + curl_formadd(&post, + &lastItem, + CURLFORM_COPYNAME, "filename", + CURLFORM_COPYCONTENTS, d->m_fileName, + CURLFORM_END); + + // Fill in the submit field too, even if this is rarely needed + curl_formadd(&post, + &lastItem, + CURLFORM_COPYNAME, "submit", + CURLFORM_COPYCONTENTS, "send", + CURLFORM_END); This looks to be based on curl example code. It seems incorrect to be adding a form field named "submit" for each file that is uploaded. I think the indentation style of this code block could be cleaned up a great deal too. + if (job == node->job()) { + node->setRemoved(true); + return true; + node = next; + } I think "node = next;" should be outside the braces. +class ResourceHandleList +{ Brace should be on same line as class statement.
Created attachment 14268 [details] curl cleanup, take two This patch attempts to address some of the issues in the review. I'm not entirely familiar with what it's doing, but I have pending patches for this area of code and would prefer to get Krzysztof's patch merged while it still applies. Having this patch open in bugzilla is a blocker for new work being done on the curl backend, so we should try to get this patch applied or WONTFIX if necessary.
Created attachment 14401 [details] Curl re-entrancy problem: tries to reuse semi-active handle Segfault when libcurl tries to share handle which is not finished yet. man curl_easy_handle: "If the curl handles are used simultaneously, you MUST use the locking methods in the share handle." When libcurl-gnutls (ubuntu package) is concerned, "simultaneously" means not calling add when you have received callback from that originates from curl_multi_perform. Temp fix: -- Index: platform/network/gdk/ResourceHandleManager.cpp =================================================================== --- platform/network/gdk/ResourceHandleManager.cpp (revision 21288) +++ platform/network/gdk/ResourceHandleManager.cpp (working copy) @@ -188,7 +188,6 @@ void ResourceHandleManager::add(Resource curl_easy_setopt(d->m_handle, CURLOPT_FOLLOWLOCATION, 1); curl_easy_setopt(d->m_handle, CURLOPT_MAXREDIRS, 10); curl_easy_setopt(d->m_handle, CURLOPT_HTTPAUTH, CURLAUTH_ANY); - curl_easy_setopt(d->m_handle, CURLOPT_SHARE, m_curlShareHandle); // enable gzip and deflate through Accept-Encoding: curl_easy_setopt(d->m_handle, CURLOPT_ENCODING, "");
Code changes look ok to me, though I am not a curl expert. It's been reported these make curl networking more stable, so let us get them landed for now. They'll be more thoroughly validated once we get http layout tests running for gdk and other ports that use curl.
Comment on attachment 14268 [details] curl cleanup, take two r=me
Landed in r21885.