cui/Library_cui.mk                            |    2 
 cui/source/dialogs/AdditionsDialog.cxx        |  130 ++++++++------------------
 cui/source/inc/AdditionsDialog.hxx            |    4 
 include/unotools/ucbstreamhelper.hxx          |    2 
 unotools/source/ucbhelper/ucbstreamhelper.cxx |    5 +
 vcl/source/graphic/GraphicLoader.cxx          |    1 
 6 files changed, 53 insertions(+), 91 deletions(-)

New commits:
commit db6c7a486395304f38e9ea52951f576f34749cbc
Author:     Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Wed Oct 28 13:25:25 2020 +0100
Commit:     Stephan Bergmann <sberg...@redhat.com>
CommitDate: Wed Oct 28 19:15:28 2020 +0100

    Use UCB instead of cURL to download https files
    
    Since 7dc6fc32eb618da6defb8a9f330978fa019677b8 "uitest: Check the more icons
    dialog opens" started to test the AdditionsDialog code, some ASan builds 
started
    to report use-after-poison from within SearchAndParseThread::execute ->
    curlGet -> curl_easy_perform -> ... -> https_connecting -> ... ->
    secmod_ModuleInit -> pemC_Initialize -> ..., see the comments starting at
    <https://gerrit.libreoffice.org/c/core/+/98226/
    10#message-2a55980ab2477e41dc7515e4aeabc7234afc2053> "tdf#133026: Tight
    integration of extensions - Adding thread structure".
    
    The exact cause for that ASan use-after-poison is not quite clear to me.  
First,
    I thought it was due to liberal use of curl_easy_init (without a central
    curl_global_init) in a multi-threaded process; see
    <https://curl.haxx.se/libcurl/c/curl_easy_init.html> for this being 
considered
    "lethal".  But then, another issue could be that most of the nss libraries 
like
    instdir/program/libnss3.so (implementing the "... -> secmod_ModuleInit" 
part)
    come from LO's bundled --without-system-nss while /lib64/libnsspem.so
    (implementing the "pemC_Initialize -> ..." part) comes from the system, and
    there could be some mismatch when mixing these (esp. if the former are built
    with ASan).
    
    So whatever the actual cause, the right fix should be to use LO's UCB 
instead of
    directly calling into cURL anyway.
    
    That required a version of utl::UcbStreamHelper::CreateStream that uses a 
given
    XInteractionHandler (which may be null to indicate no interaction handling,
    instead reporting all interaction requests as exceptions) instead of 
internally
    creating an XInteractionHandler using the GUI, and which would cause 
deadlock
    in 7dc6fc32eb618da6defb8a9f330978fa019677b8's UITest_sw_options.  (And
    introducing that additional utl::UcbStreamHelper::CreateStream overload 
required
    css::awt::XWindow to be a complete type now in
    vcl/source/graphic/GraphicLoader.cxx, for
    
    > include/com/sun/star/uno/Reference.h:277:18: note: in instantiation of 
variable template specialization 
'std::is_base_of_v<com::sun::star::task::XInteractionHandler, 
com::sun::star::awt::XWindow>' requested here
    >             std::is_base_of_v<interface_type, derived_type>
    >                  ^
    
    )
    
    (The removed code in cui/source/dialogs/AdditionsDialog.cxx should have 
been the
    last remaining use of curl in Library_cui.  Apparently,
    e1e9e2aa16f08a8fd9e38db706d63e8eceeda8d1 "Kill Mozilla personas" had 
forgotten
    to remove it from cui/Library_cui.mk the last time Library_cui had become
    curl-free, before the introduction of the AdditionsDialog code.)
    
    (I did not remove the #undef ABSOLUTE FIXME from
    cui/source/dialogs/AdditionsDialog.cxx.
    c4bee547b02fbe3d07b1e9ee203c73e48f86e6bf "tdf#133026: Additions: Better 
Search
    Function" does not tell whether it had been added to mitigate a macro 
definition
    from the (now removed) #include <curl/curl.h>.)
    
    Change-Id: I1fec7488d36df81c3543d12d97da1291f77712fc
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/104938
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sberg...@redhat.com>

diff --git a/cui/Library_cui.mk b/cui/Library_cui.mk
index 1d1f0feb9ea8..ff221a9cc55a 100644
--- a/cui/Library_cui.mk
+++ b/cui/Library_cui.mk
@@ -68,8 +68,6 @@ $(eval $(call gb_Library_use_externals,cui,\
        boost_headers \
        $(call gb_Helper_optional,OPENCL,\
                clew) \
-    $(call gb_Helper_optional,DESKTOP,\
-               curl) \
     icuuc \
     icu_headers \
     libxml2 \
diff --git a/cui/source/dialogs/AdditionsDialog.cxx 
b/cui/source/dialogs/AdditionsDialog.cxx
index dd994839681a..39f841d03996 100644
--- a/cui/source/dialogs/AdditionsDialog.cxx
+++ b/cui/source/dialogs/AdditionsDialog.cxx
@@ -15,6 +15,7 @@
 
 #include <com/sun/star/graphic/GraphicProvider.hpp>
 #include <com/sun/star/graphic/XGraphicProvider.hpp>
+#include <com/sun/star/ucb/NameClash.hpp>
 #include <com/sun/star/ucb/SimpleFileAccess.hpp>
 #include <osl/file.hxx>
 #include <rtl/bootstrap.hxx>
@@ -30,7 +31,7 @@
 #include <com/sun/star/util/SearchFlags.hpp>
 #include <com/sun/star/util/SearchAlgorithms2.hpp>
 #include <unotools/textsearch.hxx>
-
+#include <unotools/ucbstreamhelper.hxx>
 #include <ucbhelper/content.hxx>
 
 #include <com/sun/star/deployment/DeploymentException.hpp>
@@ -40,8 +41,6 @@
 
 #include <com/sun/star/task/XInteractionApprove.hpp>
 
-//cURL
-#include <curl/curl.h>
 #include <orcus/json_document_tree.hpp>
 #include <orcus/config.hpp>
 #include <orcus/pstring.hpp>
@@ -70,97 +69,54 @@ using namespace ::com::sun::star::uno;
 using namespace ::com::sun::star::ucb;
 using namespace ::com::sun::star::beans;
 
-#ifdef UNX
-const char kUserAgent[] = "LibreOffice AdditionsDownloader/1.0 (Linux)";
-#else
-const char kUserAgent[] = "LibreOffice AdditionsDownloader/1.0 (unknown 
platform)";
-#endif
-
 namespace
 {
-size_t WriteCallback(void* ptr, size_t size, size_t nmemb, void* userp)
-{
-    if (!userp)
-        return 0;
-
-    std::string* response = static_cast<std::string*>(userp);
-    size_t real_size = size * nmemb;
-    response->append(static_cast<char*>(ptr), real_size);
-    return real_size;
-}
-
-// Callback to get the response data from server to a file.
-size_t WriteCallbackFile(void* ptr, size_t size, size_t nmemb, void* userp)
-{
-    if (!userp)
-        return 0;
-
-    SvStream* response = static_cast<SvStream*>(userp);
-    size_t real_size = size * nmemb;
-    response->WriteBytes(ptr, real_size);
-    return real_size;
-}
-
 // Gets the content of the given URL and returns as a standard string
-std::string curlGet(const OString& rURL)
+std::string ucbGet(const OUString& rURL)
 {
-    CURL* curl = curl_easy_init();
-
-    if (!curl)
-        return std::string();
-
-    curl_easy_setopt(curl, CURLOPT_URL, rURL.getStr());
-
-    std::string response_body;
-
-    curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, WriteCallback);
-    curl_easy_setopt(curl, CURLOPT_WRITEDATA, 
static_cast<void*>(&response_body));
-
-    CURLcode cc = curl_easy_perform(curl);
-    long http_code = 0;
-    curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
-
-    if (http_code != 200)
+    try
     {
-        SAL_WARN("cui.dialogs", "Download failed. Error code: " << http_code);
-        response_body.clear();
+        auto const s = utl::UcbStreamHelper::CreateStream(
+            rURL, StreamMode::STD_READ, 
css::uno::Reference<css::task::XInteractionHandler>());
+        if (!s)
+        {
+            SAL_WARN("cui.dialogs", "CreateStream <" << rURL << "> failed");
+            return {};
+        }
+        std::string response_body;
+        do
+        {
+            char buf[4096];
+            auto const n = s->ReadBytes(buf, sizeof buf);
+            response_body.append(buf, n);
+        } while (s->good());
+        if (s->bad())
+        {
+            SAL_WARN("cui.dialogs", "Reading <" << rURL << "> failed with " << 
s->GetError());
+            return {};
+        }
+        return response_body;
     }
-
-    if (cc != CURLE_OK)
+    catch (css::uno::Exception&)
     {
-        SAL_WARN("cui.dialogs", "curl error: " << cc);
+        TOOLS_WARN_EXCEPTION("cui.dialogs", "Download failed");
+        return {};
     }
-
-    return response_body;
 }
 
-// Downloads and saves the file at the given rURL to a local path (sFileURL)
-void curlDownload(const OString& rURL, const OUString& sFileURL)
+// Downloads and saves the file at the given rURL to a local path 
(sFolderURL/fileName)
+void ucbDownload(const OUString& rURL, const OUString& sFolderURL, const 
OUString& fileName)
 {
-    CURL* curl = curl_easy_init();
-    SvFileStream aFile(sFileURL, StreamMode::WRITE);
-
-    if (!curl)
-        return;
-
-    curl_easy_setopt(curl, CURLOPT_URL, rURL.getStr());
-    curl_easy_setopt(curl, CURLOPT_USERAGENT, kUserAgent);
-
-    curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, WriteCallbackFile);
-    curl_easy_setopt(curl, CURLOPT_WRITEDATA, static_cast<void*>(&aFile));
-
-    CURLcode cc = curl_easy_perform(curl);
-    long http_code = 0;
-    curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
-
-    if (http_code != 200)
+    try
     {
-        SAL_WARN("cui.dialogs", "Download failed. Error code: " << http_code);
+        ucbhelper::Content(sFolderURL, {}, 
comphelper::getProcessComponentContext())
+            .transferContent(ucbhelper::Content(rURL, {}, 
comphelper::getProcessComponentContext()),
+                             ucbhelper::InsertOperation::Copy, fileName,
+                             css::ucb::NameClash::OVERWRITE);
     }
-
-    if (cc != CURLE_OK)
+    catch (css::uno::Exception&)
     {
-        SAL_WARN("cui.dialogs", "curl error: " << cc);
+        TOOLS_WARN_EXCEPTION("cui.dialogs", "Download failed");
     }
 }
 
@@ -264,14 +220,14 @@ bool getPreviewFile(const AdditionInfo& aAdditionInfo, 
OUString& sPreviewFile)
     userFolder += "/user/additions/" + aAdditionInfo.sExtensionID + "/";
 
     OUString 
aPreviewFile(INetURLObject(aAdditionInfo.sScreenshotURL).getName());
-    OString aPreviewURL = OUStringToOString(aAdditionInfo.sScreenshotURL, 
RTL_TEXTENCODING_UTF8);
+    OUString aPreviewURL = aAdditionInfo.sScreenshotURL;
 
     try
     {
         osl::Directory::createPath(userFolder);
 
         if (!xFileAccess->exists(userFolder + aPreviewFile))
-            curlDownload(aPreviewURL, userFolder + aPreviewFile);
+            ucbDownload(aPreviewURL, userFolder, aPreviewFile);
     }
     catch (const uno::Exception&)
     {
@@ -454,7 +410,7 @@ void SearchAndParseThread::execute()
 
     if (m_bIsFirstLoading)
     {
-        std::string sResponse = curlGet(m_pAdditionsDialog->m_sURL);
+        std::string sResponse = ucbGet(m_pAdditionsDialog->m_sURL);
         parseResponse(sResponse, m_pAdditionsDialog->m_aAllExtensionsVector);
         std::sort(m_pAdditionsDialog->m_aAllExtensionsVector.begin(),
                   m_pAdditionsDialog->m_aAllExtensionsVector.end(),
@@ -496,7 +452,7 @@ AdditionsDialog::AdditionsDialog(weld::Window* pParent, 
const OUString& sAdditio
     m_xEntrySearch->connect_focus_out(LINK(this, AdditionsDialog, 
FocusOut_Impl));
     m_xButtonClose->connect_clicked(LINK(this, AdditionsDialog, 
CloseButtonHdl));
 
-    m_sTag = OUStringToOString(sAdditionsTag, RTL_TEXTENCODING_UTF8);
+    m_sTag = sAdditionsTag;
     m_nMaxItemCount = PAGE_SIZE; // Dialog initialization item count
     m_nCurrentListItemCount = 0; // First, there is no item on the list.
 
@@ -511,7 +467,7 @@ AdditionsDialog::AdditionsDialog(weld::Window* pParent, 
const OUString& sAdditio
         m_sTag = "allextensions"; // Means empty parameter
     }
     //FIXME: Temporary URL
-    OString rURL = "https://yusufketen.com/api/"; + m_sTag + ".json";
+    OUString rURL = "https://yusufketen.com/api/"; + m_sTag + ".json";
     m_sURL = rURL;
 
     m_xExtensionManager
@@ -722,14 +678,14 @@ bool AdditionsItem::getExtensionFile(OUString& 
sExtensionFile)
     userFolder += "/user/additions/" + m_sExtensionID + "/";
 
     OUString aExtesionsFile(INetURLObject(m_sDownloadURL).getName());
-    OString aExtesionsURL = OUStringToOString(m_sDownloadURL, 
RTL_TEXTENCODING_UTF8);
+    OUString aExtesionsURL = m_sDownloadURL;
 
     try
     {
         osl::Directory::createPath(userFolder);
 
         if (!xFileAccess->exists(userFolder + aExtesionsFile))
-            curlDownload(aExtesionsURL, userFolder + aExtesionsFile);
+            ucbDownload(aExtesionsURL, userFolder, aExtesionsFile);
     }
     catch (const uno::Exception&)
     {
diff --git a/cui/source/inc/AdditionsDialog.hxx 
b/cui/source/inc/AdditionsDialog.hxx
index 9b7ced9c75f4..c550034a16db 100644
--- a/cui/source/inc/AdditionsDialog.hxx
+++ b/cui/source/inc/AdditionsDialog.hxx
@@ -90,8 +90,8 @@ public:
 
     ::rtl::Reference<SearchAndParseThread> m_pSearchThread;
 
-    OString m_sURL;
-    OString m_sTag;
+    OUString m_sURL;
+    OUString m_sTag;
     size_t
         m_nMaxItemCount; // Max number of item which will appear on the list 
before the press to the show more button.
     size_t m_nCurrentListItemCount; // Current number of item on the list
diff --git a/include/unotools/ucbstreamhelper.hxx 
b/include/unotools/ucbstreamhelper.hxx
index 69bae538b316..9f16bb8fc0de 100644
--- a/include/unotools/ucbstreamhelper.hxx
+++ b/include/unotools/ucbstreamhelper.hxx
@@ -33,12 +33,14 @@ namespace com::sun::star::io
             }
 
 namespace com::sun::star::awt { class XWindow; }
+namespace com::sun::star::task { class XInteractionHandler; }
 
 namespace utl
 {
     class UNOTOOLS_DLLPUBLIC UcbStreamHelper
     {
     public:
+        static std::unique_ptr<SvStream> CreateStream(const OUString& 
rFileName, StreamMode eOpenMode, 
css::uno::Reference<css::task::XInteractionHandler> const & handler);
         static std::unique_ptr<SvStream> CreateStream(const OUString& 
rFileName, StreamMode eOpenMode, css::uno::Reference<css::awt::XWindow> 
xParentWin = nullptr);
         static std::unique_ptr<SvStream> CreateStream(const OUString& 
rFileName, StreamMode eOpenMode,
                                                       bool bFileExists, 
css::uno::Reference<css::awt::XWindow> xParentWin = nullptr);
diff --git a/unotools/source/ucbhelper/ucbstreamhelper.cxx 
b/unotools/source/ucbhelper/ucbstreamhelper.cxx
index 6f72c00bc985..7c95e7ef9078 100644
--- a/unotools/source/ucbhelper/ucbstreamhelper.cxx
+++ b/unotools/source/ucbhelper/ucbstreamhelper.cxx
@@ -138,6 +138,11 @@ static std::unique_ptr<SvStream> lcl_CreateStream( const 
OUString& rFileName, St
     return pStream;
 }
 
+std::unique_ptr<SvStream> UcbStreamHelper::CreateStream(const OUString& 
rFileName, StreamMode eOpenMode, 
css::uno::Reference<css::task::XInteractionHandler> const & handler)
+{
+    return lcl_CreateStream( rFileName, eOpenMode, handler, true /* 
bEnsureFileExists */ );
+}
+
 std::unique_ptr<SvStream> UcbStreamHelper::CreateStream(const OUString& 
rFileName, StreamMode eOpenMode, css::uno::Reference<css::awt::XWindow> 
xParentWin)
 {
     // related tdf#99312
diff --git a/vcl/source/graphic/GraphicLoader.cxx 
b/vcl/source/graphic/GraphicLoader.cxx
index c26b2b9d0cfe..5c8ccc87d26c 100644
--- a/vcl/source/graphic/GraphicLoader.cxx
+++ b/vcl/source/graphic/GraphicLoader.cxx
@@ -10,6 +10,7 @@
 
 #include <vcl/GraphicLoader.hxx>
 
+#include <com/sun/star/awt/XWindow.hpp>
 #include <unotools/ucbstreamhelper.hxx>
 #include <vcl/graphicfilter.hxx>
 #include <vcl/weld.hxx>
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to