desktop/source/app/crashreport.cxx | 6 ++---
include/ucbhelper/proxydecider.hxx | 6 ++---
ucb/source/ucp/cmis/cmis_content.cxx | 5 ----
ucb/source/ucp/cmis/cmis_repo_content.cxx | 5 ----
ucb/source/ucp/webdav-curl/CurlSession.cxx | 34 +++++++++++++++--------------
ucb/source/ucp/webdav-curl/CurlSession.hxx | 4 +--
ucbhelper/source/client/proxydecider.cxx | 17 ++++++++++----
7 files changed, 40 insertions(+), 37 deletions(-)
New commits:
commit f88af95552f9b46e1714964d84c447327b50ed40
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Fri Aug 18 15:09:43 2023 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Fri Aug 18 18:33:27 2023 +0200
ucbhelper,ucb,desktop: InternetProxyServer is problematic
It turns out that every single client of InternetProxyDecider simply
concatenates the 2 members of InternetProxyServer into a single string
and passes it on to curl_easy_setopt(CURLOPT_PROXY), which will happily
take a URL including scheme and everything.
It turns out that the awful GetUnixSystemProxy() tries to cut off the
scheme in a terrible way, but GetPACProxy() does no such thing and
WINHTTP_PROXY_INFO::lpszProxy may or may not contain scheme in its
entries; fix this to only separate the port and leave the rest alone.
So why do we need a InternetProxyServer struct? Because officecfg has
separate entries that correspond to its members, and so
InternetProxyDecider gets separate events on its listener interface when
any of them changes, which is easiest to handle if it stores these
separately.
So just return a concatenated URL with or without scheme in getProxy().
Change-Id: I43c696471c8bec90667b5930fa00975adb432fe1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/155840
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/desktop/source/app/crashreport.cxx
b/desktop/source/app/crashreport.cxx
index 59a2aa771b32..680492b3b80a 100644
--- a/desktop/source/app/crashreport.cxx
+++ b/desktop/source/app/crashreport.cxx
@@ -139,7 +139,7 @@ void CrashReporter::writeCommonInfo()
static constexpr OUStringLiteral url = u"crashreport.libreoffice.org";
const sal_Int32 port = 443;
- const ucbhelper::InternetProxyServer proxy_server =
proxy_decider.getProxy(protocol, url, port);
+ const OUString proxy_server = proxy_decider.getProxy(protocol, url, port);
// save the new Keys
vmaKeyValues atlast = maKeyValues;
@@ -152,9 +152,9 @@ void CrashReporter::writeCommonInfo()
addKeyValue("BuildID", utl::Bootstrap::getBuildIdData(""), AddItem);
addKeyValue("URL", protocol + "://" + url + "/submit/", AddItem);
- if (!proxy_server.aName.isEmpty())
+ if (!proxy_server.isEmpty())
{
- addKeyValue("Proxy", proxy_server.aName + ":" +
OUString::number(proxy_server.nPort), AddItem);
+ addKeyValue("Proxy", proxy_server, AddItem);
}
// write the new keys at the end
diff --git a/include/ucbhelper/proxydecider.hxx
b/include/ucbhelper/proxydecider.hxx
index 9a1abad3264a..3fe3025833ce 100644
--- a/include/ucbhelper/proxydecider.hxx
+++ b/include/ucbhelper/proxydecider.hxx
@@ -116,10 +116,10 @@ public:
* If host is not empty this parameter must always contain a valid
* port number, for instance the default port for the requested
* protocol(i.e. 80 or http).
- * @return a InternetProxyServer struct. If member aName of the
- * InternetProxyServer is empty no proxy server is to be used.
+ * @return an URL, with or without scheme.
+ * If empty no proxy server is to be used.
*/
- InternetProxyServer
+ OUString
getProxy( const OUString & rProtocol,
const OUString & rHost,
sal_Int32 nPort ) const;
diff --git a/ucb/source/ucp/cmis/cmis_content.cxx
b/ucb/source/ucp/cmis/cmis_content.cxx
index 815d89502f2c..b5ad2cb738ce 100644
--- a/ucb/source/ucp/cmis/cmis_content.cxx
+++ b/ucb/source/ucp/cmis/cmis_content.cxx
@@ -304,11 +304,8 @@ namespace cmis
// Set the proxy if needed. We are doing that all times as the proxy
data shouldn't be cached.
ucbhelper::InternetProxyDecider aProxyDecider( m_xContext );
INetURLObject aBindingUrl( m_aURL.getBindingUrl( ) );
- const ucbhelper::InternetProxyServer& rProxy = aProxyDecider.getProxy(
+ const OUString sProxy = aProxyDecider.getProxy(
INetURLObject::GetScheme( aBindingUrl.GetProtocol( ) ),
aBindingUrl.GetHost(), aBindingUrl.GetPort() );
- OUString sProxy = rProxy.aName;
- if ( rProxy.nPort > 0 )
- sProxy += ":" + OUString::number( rProxy.nPort );
libcmis::SessionFactory::setProxySettings( OUSTR_TO_STDSTR( sProxy ),
std::string(), std::string(), std::string() );
// Look for a cached session, key is binding url + repo id
diff --git a/ucb/source/ucp/cmis/cmis_repo_content.cxx
b/ucb/source/ucp/cmis/cmis_repo_content.cxx
index 56d11b5283a7..455df22f8640 100644
--- a/ucb/source/ucp/cmis/cmis_repo_content.cxx
+++ b/ucb/source/ucp/cmis/cmis_repo_content.cxx
@@ -134,11 +134,8 @@ namespace cmis
// Set the proxy if needed. We are doing that all times as the proxy
data shouldn't be cached.
ucbhelper::InternetProxyDecider aProxyDecider( m_xContext );
INetURLObject aBindingUrl( m_aURL.getBindingUrl( ) );
- const ucbhelper::InternetProxyServer& rProxy = aProxyDecider.getProxy(
+ const OUString sProxy = aProxyDecider.getProxy(
INetURLObject::GetScheme( aBindingUrl.GetProtocol( ) ),
aBindingUrl.GetHost(), aBindingUrl.GetPort() );
- OUString sProxy = rProxy.aName;
- if ( rProxy.nPort > 0 )
- sProxy += ":" + OUString::number( rProxy.nPort );
libcmis::SessionFactory::setProxySettings( OUSTR_TO_STDSTR( sProxy ),
std::string(), std::string(), std::string() );
if ( !m_aRepositories.empty() )
diff --git a/ucb/source/ucp/webdav-curl/CurlSession.cxx
b/ucb/source/ucp/webdav-curl/CurlSession.cxx
index ac924beb2e17..a881a1703dec 100644
--- a/ucb/source/ucp/webdav-curl/CurlSession.cxx
+++ b/ucb/source/ucp/webdav-curl/CurlSession.cxx
@@ -693,18 +693,15 @@
CurlSession::CurlSession(uno::Reference<uno::XComponentContext> xContext,
rc = curl_easy_setopt(m_pCurl.get(), CURLOPT_HTTPAUTH, CURLAUTH_ANY);
assert(rc == CURLE_OK); // ANY is always available
// always set CURLOPT_PROXY to suppress proxy detection in libcurl
- OString const utf8Proxy(OUStringToOString(m_Proxy.aName,
RTL_TEXTENCODING_UTF8));
+ OString const utf8Proxy(OUStringToOString(m_Proxy, RTL_TEXTENCODING_UTF8));
rc = curl_easy_setopt(m_pCurl.get(), CURLOPT_PROXY, utf8Proxy.getStr());
if (rc != CURLE_OK)
{
SAL_WARN("ucb.ucp.webdav.curl", "CURLOPT_PROXY failed: " <<
GetErrorString(rc));
- throw DAVException(DAVException::DAV_SESSION_CREATE,
- ConnectionEndPointString(m_Proxy.aName,
m_Proxy.nPort));
+ throw DAVException(DAVException::DAV_SESSION_CREATE, m_Proxy);
}
- if (!m_Proxy.aName.isEmpty())
+ if (!m_Proxy.isEmpty())
{
- rc = curl_easy_setopt(m_pCurl.get(), CURLOPT_PROXYPORT,
static_cast<long>(m_Proxy.nPort));
- assert(rc == CURLE_OK);
// set this initially, may be overwritten during authentication
rc = curl_easy_setopt(m_pCurl.get(), CURLOPT_PROXYAUTH, CURLAUTH_ANY);
assert(rc == CURLE_OK); // ANY is always available
@@ -749,7 +746,7 @@ auto CurlSession::CanUse(OUString const& rURI,
uno::Sequence<beans::NamedValue>
auto CurlSession::UsesProxy() -> bool
{
assert(m_URI.GetScheme() == "http" || m_URI.GetScheme() == "https");
- return !m_Proxy.aName.isEmpty();
+ return !m_Proxy.isEmpty();
}
auto CurlSession::abort() -> void
@@ -967,9 +964,7 @@ auto CurlProcessor::ProcessRequestImpl(
case CURLE_UNSUPPORTED_PROTOCOL:
throw DAVException(DAVException::DAV_UNSUPPORTED);
case CURLE_COULDNT_RESOLVE_PROXY:
- throw DAVException(
- DAVException::DAV_HTTP_LOOKUP,
- ConnectionEndPointString(rSession.m_Proxy.aName,
rSession.m_Proxy.nPort));
+ throw DAVException(DAVException::DAV_HTTP_LOOKUP,
rSession.m_Proxy);
case CURLE_COULDNT_RESOLVE_HOST:
throw DAVException(
DAVException::DAV_HTTP_LOOKUP,
@@ -1214,12 +1209,12 @@ auto CurlProcessor::ProcessRequest(
};
::std::optional<Auth> oAuth;
::std::optional<Auth> oAuthProxy;
- if (pEnv && !rSession.m_isAuthenticatedProxy &&
!rSession.m_Proxy.aName.isEmpty())
+ if (pEnv && !rSession.m_isAuthenticatedProxy &&
!rSession.m_Proxy.isEmpty())
{
try
{
// the hope is that this must be a URI
- CurlUri const uri(rSession.m_Proxy.aName);
+ CurlUri const uri(rSession.m_Proxy);
if (!uri.GetUser().isEmpty() || !uri.GetPassword().isEmpty())
{
oAuthProxy.emplace(uri.GetUser(), uri.GetPassword(),
CURLAUTH_ANY);
@@ -1452,7 +1447,7 @@ auto CurlProcessor::ProcessRequest(
auto const ret =
pEnv->m_xAuthListener->authenticate(
oRealm ? *oRealm : "",
statusCode == SC_UNAUTHORIZED ?
rSession.m_URI.GetHost()
- :
rSession.m_Proxy.aName,
+ :
rSession.m_Proxy,
userName, passWord, isSystemCredSupported);
if (ret == 0)
diff --git a/ucb/source/ucp/webdav-curl/CurlSession.hxx
b/ucb/source/ucp/webdav-curl/CurlSession.hxx
index 3c3df3c26d76..2b714128781a 100644
--- a/ucb/source/ucp/webdav-curl/CurlSession.hxx
+++ b/ucb/source/ucp/webdav-curl/CurlSession.hxx
@@ -31,8 +31,8 @@ private:
CurlUri const m_URI;
/// buffer for libcurl detailed error messages
char m_ErrorBuffer[CURL_ERROR_SIZE];
- /// proxy is used if aName is non-empty
- ::ucbhelper::InternetProxyServer const m_Proxy;
+ /// proxy is used if non-empty
+ OUString const m_Proxy;
/// once authentication was successful, rely on m_pCurl's data
bool m_isAuthenticated = false;
bool m_isAuthenticatedProxy = false;
diff --git a/ucbhelper/source/client/proxydecider.cxx
b/ucbhelper/source/client/proxydecider.cxx
index a46cfa268971..a2abf5a80cdd 100644
--- a/ucbhelper/source/client/proxydecider.cxx
+++ b/ucbhelper/source/client/proxydecider.cxx
@@ -585,8 +585,9 @@ InternetProxyServer GetUnixSystemProxy(const OUString &
rProtocol)
OUString tmp = OUString::createFromAscii(pEnvProxy);
if (tmp.getLength() < (rProtocol.getLength() + 3))
return aProxy;
- tmp = tmp.copy(rProtocol.getLength() + 3);
- sal_Int32 x = tmp.indexOf(':');
+ sal_Int32 x = tmp.indexOf("://");
+ sal_Int32 at = tmp.indexOf('@', x == -1 ? 0 : x + 3);
+ x = tmp.indexOf(':', at == -1 ? x == -1 ? 0 : x + 3 : at + 1);
if (x == -1)
return aProxy;
int nPort = o3tl::toInt32(tmp.subView(x + 1));
@@ -937,13 +938,19 @@ bool InternetProxyDecider::shouldUseProxy( const OUString
& rProtocol,
return !rData.aName.isEmpty();
}
-
-InternetProxyServer InternetProxyDecider::getProxy(
+OUString InternetProxyDecider::getProxy(
const OUString & rProtocol,
const OUString & rHost,
sal_Int32 nPort ) const
{
- return m_xImpl->getProxy( rProtocol, rHost, nPort );
+ InternetProxyServer ret(m_xImpl->getProxy(rProtocol, rHost, nPort));
+
+ if (ret.aName.isEmpty() || ret.nPort == -1)
+ {
+ return ret.aName;
+ }
+
+ return ret.aName + ":" + OUString::number(ret.nPort);
}
} // namespace ucbhelper
commit 9b30b4b1678e8be15ba51d236bd9a3e693d8d3d6
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Fri Aug 18 13:49:54 2023 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Fri Aug 18 18:33:13 2023 +0200
Fix curl proxy access for non-authenticated proxy
If rSession.m_Proxy.aName is a simple host-name, the CurlUri constructor
will fail with CURLUE_BAD_SCHEME, so just ignore the error here,
we only care about parsing out the username/password
Change-Id: Iec2d6e7315a5899ddddf6120a43199b75bf62db2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/155834
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/ucb/source/ucp/webdav-curl/CurlSession.cxx
b/ucb/source/ucp/webdav-curl/CurlSession.cxx
index b9161767c829..ac924beb2e17 100644
--- a/ucb/source/ucp/webdav-curl/CurlSession.cxx
+++ b/ucb/source/ucp/webdav-curl/CurlSession.cxx
@@ -1216,11 +1216,18 @@ auto CurlProcessor::ProcessRequest(
::std::optional<Auth> oAuthProxy;
if (pEnv && !rSession.m_isAuthenticatedProxy &&
!rSession.m_Proxy.aName.isEmpty())
{
- // the hope is that this must be a URI
- CurlUri const uri(rSession.m_Proxy.aName);
- if (!uri.GetUser().isEmpty() || !uri.GetPassword().isEmpty())
+ try
+ {
+ // the hope is that this must be a URI
+ CurlUri const uri(rSession.m_Proxy.aName);
+ if (!uri.GetUser().isEmpty() || !uri.GetPassword().isEmpty())
+ {
+ oAuthProxy.emplace(uri.GetUser(), uri.GetPassword(),
CURLAUTH_ANY);
+ }
+ }
+ catch (DAVException&)
{
- oAuthProxy.emplace(uri.GetUser(), uri.GetPassword(), CURLAUTH_ANY);
+ // ignore any parsing failure here
}
}
decltype(CURLAUTH_ANY) const authSystem(CURLAUTH_NEGOTIATE | CURLAUTH_NTLM
| CURLAUTH_NTLM_WB);
