From e25f5b89b22f68a95a4eb1c5c3e920d0033a3608 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Thu, 20 Jan 2022 12:59:20 +0100 Subject: ucb: webdav-curl: fix the debug logging more Unfortunately didn't work in the cases where it needed to work. Change-Id: Ia132ba38fb6f0be4481dce48da2d713d47fa8bf7 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128658 Tested-by: Jenkins Reviewed-by: Michael Stahl (cherry picked from commit f8ef102a82513233fb794109cecd599304e78407) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128670 Reviewed-by: Thorsten Behrens --- ucb/source/ucp/webdav-curl/CurlSession.cxx | 91 ++++++++++++++++-------------- 1 file changed, 50 insertions(+), 41 deletions(-) diff --git a/ucb/source/ucp/webdav-curl/CurlSession.cxx b/ucb/source/ucp/webdav-curl/CurlSession.cxx index ee21e88fc44d..720e566e2f30 100644 --- a/ucb/source/ucp/webdav-curl/CurlSession.cxx +++ b/ucb/source/ucp/webdav-curl/CurlSession.cxx @@ -163,6 +163,15 @@ 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 @@ -334,7 +343,8 @@ static size_t write_callback(char* const ptr, size_t const size, size_t const nm { return 0; // that is an error } - if (200 <= *oResponseCode && *oResponseCode < 300) + // always write, for exception handler in ProcessRequest() + // if (200 <= *oResponseCode && *oResponseCode < 300) { try { @@ -1273,49 +1283,48 @@ auto CurlProcessor::ProcessRequest( } ResponseHeaders headers(rSession.m_pCurl.get()); - uno::Reference xSeqOutStream; - uno::Reference xDebugOutStream; - if (!pxOutStream) - { - xSeqOutStream = io::SequenceOutputStream::create(rSession.m_xContext); - xDebugOutStream = xSeqOutStream; - } + // always pass a stream for debug logging, buffer the result body + uno::Reference const xSeqOutStream( + io::SequenceOutputStream::create(rSession.m_xContext)); + uno::Reference const xTempOutStream(xSeqOutStream); + assert(xTempOutStream.is()); try { - ProcessRequestImpl(rSession, rURI, pRequestHeaderList.get(), - pxOutStream ? pxOutStream : &xDebugOutStream, + ProcessRequestImpl(rSession, rURI, pRequestHeaderList.get(), &xTempOutStream, pxInStream ? &data : nullptr, pRequestedHeaders, headers); + if (pxOutStream) + { // only copy to result stream if transfer was successful + (*pxOutStream)->writeBytes(xSeqOutStream->getWrittenBytes()); + (*pxOutStream)->closeOutput(); // signal EOF + } } catch (DAVException const& rException) { - if (xDebugOutStream.is()) + // log start of request body if there was any + auto const bytes(xSeqOutStream->getWrittenBytes()); + auto const len(::std::min(bytes.getLength(), 10000)); + SAL_INFO("ucb.ucp.webdav.curl", + "DAVException; (first) " << len << " bytes of data received:"); + if (0 < len) { - auto const bytes(xSeqOutStream->getWrittenBytes()); - auto const len(::std::min(bytes.getLength(), 10000)); - SAL_INFO("ucb.ucp.webdav.curl", - "DAVException; (first) " << len << " bytes of data received:"); - if (0 < len) + OStringBuffer buf(len); + for (sal_Int32 i = 0; i < len; ++i) { - OStringBuffer buf(len); - for (sal_Int32 i = 0; i < len; ++i) + if (bytes[i] < 0x20) // also if negative { - if (bytes[i] < 0x20) // also if negative - { - static char const hexDigit[16] - = { '0', '1', '2', '3', '4', '5', '6', '7', - '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' }; - buf.append("\\x"); - buf.append(hexDigit[static_cast(bytes[i]) >> 4]); - buf.append(hexDigit[bytes[i] & 0x0F]); - } - else - { - buf.append(static_cast(bytes[i])); - } + static char const hexDigit[16] = { '0', '1', '2', '3', '4', '5', '6', '7', + '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' }; + buf.append("\\x"); + buf.append(hexDigit[static_cast(bytes[i]) >> 4]); + buf.append(hexDigit[bytes[i] & 0x0F]); + } + else + { + buf.append(static_cast(bytes[i])); } - SAL_INFO("ucb.ucp.webdav.curl", buf.makeStringAndClear()); } + SAL_INFO("ucb.ucp.webdav.curl", buf.makeStringAndClear()); } // error handling part 3: special HTTP status codes @@ -1453,9 +1462,9 @@ auto CurlSession::OPTIONS(OUString const& rURIReference, DAVResource result; ::std::pair<::std::vector const&, DAVResource&> const headers(headerNames, result); - ::std::vector const options{ { CURLOPT_NOBODY, 1L, nullptr }, - { CURLOPT_CUSTOMREQUEST, "OPTIONS", - "CURLOPT_CUSTOMREQUEST" } }; + ::std::vector const options{ + g_NoBody, { CURLOPT_CUSTOMREQUEST, "OPTIONS", "CURLOPT_CUSTOMREQUEST" } + }; CurlProcessor::ProcessRequest(*this, uri, options, &rEnv, nullptr, nullptr, nullptr, &headers); @@ -1760,7 +1769,7 @@ auto CurlSession::HEAD(OUString const& rURIReference, ::std::vector co CurlUri const uri(CurlProcessor::URIReferenceToURI(*this, rURIReference)); - ::std::vector const options{ { CURLOPT_NOBODY, 1L, nullptr } }; + ::std::vector const options{ g_NoBody }; ::std::pair<::std::vector const&, DAVResource&> const headers(rHeaderNames, io_rResource); @@ -1993,7 +2002,7 @@ auto CurlSession::MKCOL(OUString const& rURIReference, DAVRequestEnvironment con CurlUri const uri(CurlProcessor::URIReferenceToURI(*this, rURIReference)); ::std::vector const options{ - { CURLOPT_NOBODY, 1L, nullptr }, { CURLOPT_CUSTOMREQUEST, "MKCOL", "CURLOPT_CUSTOMREQUEST" } + g_NoBody, { CURLOPT_CUSTOMREQUEST, "MKCOL", "CURLOPT_CUSTOMREQUEST" } }; CurlProcessor::ProcessRequest(*this, uri, options, &rEnv, nullptr, nullptr, nullptr, nullptr); @@ -2022,7 +2031,7 @@ auto CurlProcessor::MoveOrCopy(CurlSession& rSession, OUString const& rSourceURI } ::std::vector const options{ - { CURLOPT_NOBODY, 1L, nullptr }, { CURLOPT_CUSTOMREQUEST, pMethod, "CURLOPT_CUSTOMREQUEST" } + g_NoBody, { CURLOPT_CUSTOMREQUEST, pMethod, "CURLOPT_CUSTOMREQUEST" } }; CurlProcessor::ProcessRequest(rSession, uriSource, options, &rEnv, ::std::move(pList), nullptr, @@ -2053,9 +2062,9 @@ auto CurlSession::DESTROY(OUString const& rURIReference, DAVRequestEnvironment c CurlUri const uri(CurlProcessor::URIReferenceToURI(*this, rURIReference)); - ::std::vector const options{ { CURLOPT_NOBODY, 1L, nullptr }, - { CURLOPT_CUSTOMREQUEST, "DELETE", - "CURLOPT_CUSTOMREQUEST" } }; + ::std::vector const options{ + g_NoBody, { CURLOPT_CUSTOMREQUEST, "DELETE", "CURLOPT_CUSTOMREQUEST" } + }; CurlProcessor::ProcessRequest(*this, uri, options, &rEnv, nullptr, nullptr, nullptr, nullptr); } -- cgit