[Libreoffice-commits] core.git: cui/Library_cui.mk cui/source include/unotools unotools/source vcl/source
Stephan Bergmann (via logerrit)
logerrit at kemper.freedesktop.org
Wed Oct 28 18:16:13 UTC 2020
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 <sbergman at redhat.com>
AuthorDate: Wed Oct 28 13:25:25 2020 +0100
Commit: Stephan Bergmann <sbergman at 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 <sbergman at 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>
More information about the Libreoffice-commits
mailing list