ucb/source/ucp/webdav-curl/CurlSession.cxx |   32 +++++++++++------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

New commits:
commit 84b5de0e000372e46d7243873859fc03e114bde8
Author:     Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Thu Mar 14 15:47:28 2024 +0100
Commit:     Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Tue Mar 19 20:56:22 2024 +0100

    ucb: webdav-curl: always set CURLOPT_NOBODY for HEAD
    
    Otherwise there will be timeout that depends on when the server will
    close the connection, which varies by server but can be several minutes;
    getting a potential error document from the server for this one request
    when logging is enabled is less important.
    
    Change-Id: I505b014b148ba009c400d37d826c9edb8c3a6da2
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164838
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
    (cherry picked from commit c8400f5acc36d2cf0c007260bdc94534a53bba90)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164822
    Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>

diff --git a/ucb/source/ucp/webdav-curl/CurlSession.cxx 
b/ucb/source/ucp/webdav-curl/CurlSession.cxx
index 3b40d2c3082f..2f104d5388d7 100644
--- a/ucb/source/ucp/webdav-curl/CurlSession.cxx
+++ b/ucb/source/ucp/webdav-curl/CurlSession.cxx
@@ -166,15 +166,6 @@ struct CurlOption
     }
 };
 
-// NOBODY will prevent logging the response body in ProcessRequest() exception
-// handler, so only use it if logging is disabled
-const CurlOption g_NoBody{ CURLOPT_NOBODY,
-                           sal_detail_log_report(SAL_DETAIL_LOG_LEVEL_INFO, 
"ucb.ucp.webdav.curl")
-                                   == SAL_DETAIL_LOG_ACTION_IGNORE
-                               ? 1L
-                               : 0L,
-                           nullptr };
-
 /// combined guard class to ensure things are released in correct order,
 /// particularly in ProcessRequest() error handling
 class Guard
@@ -1829,7 +1820,11 @@ auto CurlSession::HEAD(OUString const& rURIReference, 
::std::vector<OUString> co
     CurlUri const uri(CurlProcessor::URIReferenceToURI(*this, rURIReference));
 
     ::std::vector<CurlOption> const options{
-        g_NoBody, { CURLOPT_CUSTOMREQUEST, "HEAD", "CURLOPT_CUSTOMREQUEST" }
+        // NOBODY will prevent logging the response body in ProcessRequest()
+        // exception, but omitting it here results in a long timeout until the
+        // server closes the connection, which is worse
+        { CURLOPT_NOBODY, 1L, nullptr },
+        { CURLOPT_CUSTOMREQUEST, "HEAD", "CURLOPT_CUSTOMREQUEST" }
     };
 
     ::std::pair<::std::vector<OUString> const&, DAVResource&> const 
headers(rHeaderNames,
commit 372ee9e4c24e628ce73ec0c6a6fee91abd2e6b94
Author:     Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Fri Mar 8 11:20:45 2024 +0100
Commit:     Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Tue Mar 19 20:56:14 2024 +0100

    ucb: webdav-curl: only set CURLOPT_NOBODY for HEAD
    
    Some testing with Apache httpd+mod_dav reveals that it usually sends a
    body with a 401 status, which causes the CURLE_WEIRD_SERVER_REPLY error
    code from curl.
    
    So we should either ignore this error in case there's a HTTP status too,
    or stop using CURLOPT_NOBODY.
    
    The latter seems to have no downside, except for HEAD requests, where
    strangely the server keeps the connection open and curl waits for 5
    seconds for no body to arrive, blocking the UI, so continue to use
    CURLOPT_NOBODY for HEAD.
    
    The other methods don't seem to block.
    
    It turns out that the SAL_LOG-dependent setting of g_NoBody turned HEAD
    into GET anyway if logging is enabled, so explicitly set the method.
    
    Change-Id: Ibe2eef8e7a827d4e356ba37c4b56bee0be3b9c13
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164569
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
    (cherry picked from commit e0259d4c0951c4dd77c74d08b9d905728d4c8dfd)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164507
    Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>

diff --git a/ucb/source/ucp/webdav-curl/CurlSession.cxx 
b/ucb/source/ucp/webdav-curl/CurlSession.cxx
index 18bd7ab61e2b..3b40d2c3082f 100644
--- a/ucb/source/ucp/webdav-curl/CurlSession.cxx
+++ b/ucb/source/ucp/webdav-curl/CurlSession.cxx
@@ -1828,7 +1828,9 @@ auto CurlSession::HEAD(OUString const& rURIReference, 
::std::vector<OUString> co
 
     CurlUri const uri(CurlProcessor::URIReferenceToURI(*this, rURIReference));
 
-    ::std::vector<CurlOption> const options{ g_NoBody };
+    ::std::vector<CurlOption> const options{
+        g_NoBody, { CURLOPT_CUSTOMREQUEST, "HEAD", "CURLOPT_CUSTOMREQUEST" }
+    };
 
     ::std::pair<::std::vector<OUString> const&, DAVResource&> const 
headers(rHeaderNames,
                                                                             
io_rResource);
@@ -2063,9 +2065,8 @@ auto CurlSession::MKCOL(OUString const& rURIReference, 
DAVRequestEnvironment con
 
     CurlUri const uri(CurlProcessor::URIReferenceToURI(*this, rURIReference));
 
-    ::std::vector<CurlOption> const options{
-        g_NoBody, { CURLOPT_CUSTOMREQUEST, "MKCOL", "CURLOPT_CUSTOMREQUEST" }
-    };
+    ::std::vector<CurlOption> const options{ { CURLOPT_CUSTOMREQUEST, "MKCOL",
+                                               "CURLOPT_CUSTOMREQUEST" } };
 
     CurlProcessor::ProcessRequest(*this, uri, "MKCOL", options, &rEnv, 
nullptr, nullptr, nullptr,
                                   nullptr);
@@ -2093,9 +2094,8 @@ auto CurlProcessor::MoveOrCopy(CurlSession& rSession, 
std::u16string_view rSourc
         throw uno::RuntimeException("curl_slist_append failed");
     }
 
-    ::std::vector<CurlOption> const options{
-        g_NoBody, { CURLOPT_CUSTOMREQUEST, pMethod, "CURLOPT_CUSTOMREQUEST" }
-    };
+    ::std::vector<CurlOption> const options{ { CURLOPT_CUSTOMREQUEST, pMethod,
+                                               "CURLOPT_CUSTOMREQUEST" } };
 
     CurlProcessor::ProcessRequest(rSession, uriSource, 
OUString::createFromAscii(pMethod), options,
                                   &rEnv, ::std::move(pList), nullptr, nullptr, 
nullptr);
@@ -2125,9 +2125,8 @@ auto CurlSession::DESTROY(OUString const& rURIReference, 
DAVRequestEnvironment c
 
     CurlUri const uri(CurlProcessor::URIReferenceToURI(*this, rURIReference));
 
-    ::std::vector<CurlOption> const options{
-        g_NoBody, { CURLOPT_CUSTOMREQUEST, "DELETE", "CURLOPT_CUSTOMREQUEST" }
-    };
+    ::std::vector<CurlOption> const options{ { CURLOPT_CUSTOMREQUEST, "DELETE",
+                                               "CURLOPT_CUSTOMREQUEST" } };
 
     CurlProcessor::ProcessRequest(*this, uri, "DESTROY", options, &rEnv, 
nullptr, nullptr, nullptr,
                                   nullptr);

Reply via email to