From bdd239b91a76238fdba96409c72f347ed0c964a6 Mon Sep 17 00:00:00 2001 From: Michael Andres Date: Fri, 5 Mar 2021 17:38:58 +0100 Subject: [PATCH 1/4] MediaCurl: remove dead DETECT_DIR_INDEX code --- zypp/media/CurlHelper.h | 1 - zypp/media/MediaCurl.cc | 50 ----------------------------------------- zypp/media/MediaCurl.h | 2 -- 3 files changed, 53 deletions(-) diff --git a/zypp/media/CurlHelper.h b/zypp/media/CurlHelper.h index 61f1f92c22..eb07e76e26 100644 --- a/zypp/media/CurlHelper.h +++ b/zypp/media/CurlHelper.h @@ -20,7 +20,6 @@ #define CONNECT_TIMEOUT 60 #define TRANSFER_TIMEOUT_MAX 60 * 60 -#define DETECT_DIR_INDEX 0 #define EXPLICITLY_NO_PROXY "_none_" diff --git a/zypp/media/MediaCurl.cc b/zypp/media/MediaCurl.cc index 2947c31439..cc31aeb003 100644 --- a/zypp/media/MediaCurl.cc +++ b/zypp/media/MediaCurl.cc @@ -971,49 +971,6 @@ bool MediaCurl::doGetDoesFileExist( const Pathname & filename ) const /////////////////////////////////////////////////////////////////// - -#if DETECT_DIR_INDEX -bool MediaCurl::detectDirIndex() const -{ - if(_url.getScheme() != "http" && _url.getScheme() != "https") - return false; - // - // try to check the effective url and set the not_a_file flag - // if the url path ends with a "/", what usually means, that - // we've received a directory index (index.html content). - // - // Note: This may be dangerous and break file retrieving in - // case of some server redirections ... ? - // - bool not_a_file = false; - char *ptr = NULL; - CURLcode ret = curl_easy_getinfo( _curl, - CURLINFO_EFFECTIVE_URL, - &ptr); - if ( ret == CURLE_OK && ptr != NULL) - { - try - { - Url eurl( ptr); - std::string path( eurl.getPathName()); - if( !path.empty() && path != "/" && *path.rbegin() == '/') - { - DBG << "Effective url (" - << eurl - << ") seems to provide the index of a directory" - << endl; - not_a_file = true; - } - } - catch( ... ) - {} - } - return not_a_file; -} -#endif - -/////////////////////////////////////////////////////////////////// - void MediaCurl::doGetFileCopy(const Pathname & filename , const Pathname & target, callback::SendReport & report, const ByteCount &expectedFileSize_r, RequestOptions options ) const { Pathname dest = target.absolutename(); @@ -1220,13 +1177,6 @@ void MediaCurl::doGetFileCopyFile(const Pathname & filename , const Pathname & d ZYPP_RETHROW(e); } } - -#if DETECT_DIR_INDEX - if (!ret && detectDirIndex()) - { - ZYPP_THROW(MediaNotAFileException(_url, filename)); - } -#endif // DETECT_DIR_INDEX } /////////////////////////////////////////////////////////////////// diff --git a/zypp/media/MediaCurl.h b/zypp/media/MediaCurl.h index de3ab5cbae..6b99945278 100644 --- a/zypp/media/MediaCurl.h +++ b/zypp/media/MediaCurl.h @@ -162,8 +162,6 @@ class MediaCurl : public MediaHandler bool authenticate(const std::string & availAuthTypes, bool firstTry) const; - bool detectDirIndex() const; - private: long _curlDebug; From 8086bba639788eb262cf4002ac7f64ed9d3817a1 Mon Sep 17 00:00:00 2001 From: Michael Andres Date: Fri, 5 Mar 2021 17:42:26 +0100 Subject: [PATCH 2/4] MediaCurl: hide TRANSFER_TIMEOUT_MAX define --- zypp/media/CurlHelper.cc | 2 ++ zypp/media/CurlHelper.h | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/zypp/media/CurlHelper.cc b/zypp/media/CurlHelper.cc index cfa8746fba..d9f30c73ca 100644 --- a/zypp/media/CurlHelper.cc +++ b/zypp/media/CurlHelper.cc @@ -10,6 +10,8 @@ #include #include +#define TRANSFER_TIMEOUT_MAX 60 * 60 + using std::endl; using namespace zypp; diff --git a/zypp/media/CurlHelper.h b/zypp/media/CurlHelper.h index eb07e76e26..2daeb97089 100644 --- a/zypp/media/CurlHelper.h +++ b/zypp/media/CurlHelper.h @@ -19,7 +19,6 @@ #include #define CONNECT_TIMEOUT 60 -#define TRANSFER_TIMEOUT_MAX 60 * 60 #define EXPLICITLY_NO_PROXY "_none_" From 42d5ec107744098dc025cc57293836080288ee5c Mon Sep 17 00:00:00 2001 From: Michael Andres Date: Fri, 5 Mar 2021 17:52:06 +0100 Subject: [PATCH 3/4] Cleanup TransferSettings usage TransferSettings schould be initialized with config defaults, so there is no need to explicitely set them twice after construction. --- zypp/media/CurlHelper.cc | 16 -------------- zypp/media/CurlHelper.h | 8 ------- zypp/media/MediaCurl.cc | 5 ----- zypp/media/TransferSettings.cc | 32 ++++++++++++++++++++-------- zypp/media/TransferSettings.h | 2 +- zypp/zyppng/media/network/request.cc | 5 ----- 6 files changed, 24 insertions(+), 44 deletions(-) diff --git a/zypp/media/CurlHelper.cc b/zypp/media/CurlHelper.cc index d9f30c73ca..1339c1427c 100644 --- a/zypp/media/CurlHelper.cc +++ b/zypp/media/CurlHelper.cc @@ -321,22 +321,6 @@ const char * distributionFlavorHeader() return _value.c_str(); } -const char * agentString() -{ - // we need to add the release and identifier to the - // agent string. - // The target could be not initialized, and then this information - // is guessed. - static const std::string _value( - str::form( - "ZYpp " LIBZYPP_VERSION_STRING " (curl %s) %s" - , curl_version_info(CURLVERSION_NOW)->version - , Target::targetDistribution( Pathname()/*guess root*/ ).c_str() - ) - ); - return _value.c_str(); -} - void curlEscape( std::string & str_r, const char char_r, const std::string & escaped_r ) { for ( std::string::size_type pos = str_r.find( char_r ); diff --git a/zypp/media/CurlHelper.h b/zypp/media/CurlHelper.h index 2daeb97089..280a36f7c2 100644 --- a/zypp/media/CurlHelper.h +++ b/zypp/media/CurlHelper.h @@ -18,8 +18,6 @@ #include #include -#define CONNECT_TIMEOUT 60 - #define EXPLICITLY_NO_PROXY "_none_" #undef CURLVERSION_AT_LEAST @@ -58,12 +56,6 @@ const char * anonymousIdHeader(); */ const char * distributionFlavorHeader(); -/** - * initialized only once, this gets the agent string - * which also includes the curl version - */ -const char * agentString(); - void curlEscape( std::string & str_r, const char char_r, const std::string & escaped_r ); std::string curlEscapedPath( std::string path_r ); std::string curlUnEscape( std::string text_r ); diff --git a/zypp/media/MediaCurl.cc b/zypp/media/MediaCurl.cc index cc31aeb003..b03717ee56 100644 --- a/zypp/media/MediaCurl.cc +++ b/zypp/media/MediaCurl.cc @@ -292,11 +292,6 @@ void MediaCurl::setupEasy() } vol_settings.addHeader("Pragma:"); - _settings.setTimeout(ZConfig::instance().download_transfer_timeout()); - _settings.setConnectTimeout(CONNECT_TIMEOUT); - - _settings.setUserAgentString(agentString()); - // fill some settings from url query parameters try { diff --git a/zypp/media/TransferSettings.cc b/zypp/media/TransferSettings.cc index 5247b55dbb..11f2b57202 100644 --- a/zypp/media/TransferSettings.cc +++ b/zypp/media/TransferSettings.cc @@ -1,14 +1,12 @@ +#include // for useragent version info + #include -#include #include #include -#include -#include -#include -#include #include #include +#include using std::endl; @@ -18,6 +16,22 @@ namespace zypp { namespace media { + namespace { + const std::string & defaultUserAgent() + { + // we need to add the release and identifier to the + // agent string. + // The target could be not initialized, and then this information + // is guessed. + static const std::string _value { + str::form( "ZYpp " LIBZYPP_VERSION_STRING " (curl %s) %s", + curl_version_info(CURLVERSION_NOW)->version, + Target::targetDistribution( Pathname()/*guess root*/ ).c_str() ) + }; + return _value; + } + } + class TransferSettings::Impl { public: @@ -53,7 +67,7 @@ namespace zypp public: std::vector _headers; - std::string _useragent; + std::optional _useragent; std::string _username; std::string _password; bool _useproxy; @@ -100,10 +114,10 @@ namespace zypp void TransferSettings::setUserAgentString( std::string && val_r ) - { _impl->_useragent = std::move(val_r); } + { if ( val_r.empty() ) _impl->_useragent.reset(); else _impl->_useragent = std::move(val_r); } - std::string TransferSettings::userAgentString() const - { return _impl->_useragent; } + const std::string & TransferSettings::userAgentString() const + { return _impl->_useragent.has_value() ? *(_impl->_useragent) : defaultUserAgent(); } void TransferSettings::setUsername( std::string && val_r ) diff --git a/zypp/media/TransferSettings.h b/zypp/media/TransferSettings.h index 7217ebf193..071bd16afc 100644 --- a/zypp/media/TransferSettings.h +++ b/zypp/media/TransferSettings.h @@ -43,7 +43,7 @@ namespace zypp void setUserAgentString( std::string && val_r ); /** user agent string */ - std::string userAgentString() const; + const std::string & userAgentString() const; /** sets the auth username */ diff --git a/zypp/zyppng/media/network/request.cc b/zypp/zyppng/media/network/request.cc index 2bc1c1628e..b98f27e45a 100644 --- a/zypp/zyppng/media/network/request.cc +++ b/zypp/zyppng/media/network/request.cc @@ -123,11 +123,6 @@ namespace zyppng { locSet.addHeader("Pragma:"); - locSet.setTimeout( zypp::ZConfig::instance().download_transfer_timeout() ); - locSet.setConnectTimeout( CONNECT_TIMEOUT ); - - locSet.setUserAgentString( internal::agentString() ); - { char *ptr = getenv("ZYPP_MEDIA_CURL_DEBUG"); _curlDebug = (ptr && *ptr) ? zypp::str::strtonum( ptr) : 0L; From 3e3be1a750f54e301e2a1cc15d2799ee2f77120f Mon Sep 17 00:00:00 2001 From: Michael Andres Date: Mon, 8 Mar 2021 14:30:04 +0100 Subject: [PATCH 4/4] zypp.conf: Allow to define a custom user-agent string (fixes openSUSE/zypper#382) New zypp.conf option download.user_agent allowas to define a custom user agent string passed down to libcurl when downloading files. If not defined or empty we use the builtin default telling the zypp version, distribution and architecture. --- zypp.conf | 10 ++++++++++ zypp/ZConfig.cc | 8 ++++++++ zypp/ZConfig.h | 6 ++++++ zypp/media/TransferSettings.cc | 13 ++++++++----- 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/zypp.conf b/zypp.conf index de9e41c257..99b16e8eeb 100644 --- a/zypp.conf +++ b/zypp.conf @@ -151,6 +151,16 @@ ## # repo.refresh.locales = en, de +## +## Custom user agent string to send to a HTTP server. +## +## Valid values: String suitable as user agent HTTP header +## +## If not defined or empty the builtin default telling the zypp +## version, distribution and architecture is used. +## +# download.user_agent = + ## ## Maximum number of concurrent connections to use per transfer ## diff --git a/zypp/ZConfig.cc b/zypp/ZConfig.cc index 8c371e2cbe..6e92a5ed5c 100644 --- a/zypp/ZConfig.cc +++ b/zypp/ZConfig.cc @@ -458,6 +458,10 @@ namespace zypp download_mediaMountdir.restoreToDefault( Pathname(value) ); } + else if ( entry == "download.user_agent" ) + { + download_user_agent = value; + } else if ( entry == "download.max_concurrent_connections" ) { str::strtonum(value, download_max_concurrent_connections); @@ -668,6 +672,7 @@ namespace zypp DefaultOption download_media_prefer_download; DefaultOption download_mediaMountdir; + std::string download_user_agent; int download_max_concurrent_connections; int download_min_download_speed; int download_max_download_speed; @@ -1055,6 +1060,9 @@ namespace zypp void ZConfig::set_default_download_media_prefer_download() { _pimpl->download_media_prefer_download.restoreToDefault(); } + std::string ZConfig::download_user_agent() const + { return _pimpl->download_user_agent; } + long ZConfig::download_max_concurrent_connections() const { return _pimpl->download_max_concurrent_connections; } diff --git a/zypp/ZConfig.h b/zypp/ZConfig.h index 39ee33f816..e23920c020 100644 --- a/zypp/ZConfig.h +++ b/zypp/ZConfig.h @@ -261,6 +261,12 @@ namespace zypp */ void repoLabelIsAlias( bool yesno_r ); + + /** + * A custom user agent string to send to a HTTP server. + */ + std::string download_user_agent() const; + /** * Maximum number of concurrent connections for a single transfer */ diff --git a/zypp/media/TransferSettings.cc b/zypp/media/TransferSettings.cc index 11f2b57202..b9116dadb8 100644 --- a/zypp/media/TransferSettings.cc +++ b/zypp/media/TransferSettings.cc @@ -23,11 +23,14 @@ namespace zypp // agent string. // The target could be not initialized, and then this information // is guessed. - static const std::string _value { - str::form( "ZYpp " LIBZYPP_VERSION_STRING " (curl %s) %s", - curl_version_info(CURLVERSION_NOW)->version, - Target::targetDistribution( Pathname()/*guess root*/ ).c_str() ) - }; + static const std::string _value {[](){ + std::string ret { ZConfig::instance().download_user_agent() }; + if ( ret.empty() ) + ret = str::form( "ZYpp " LIBZYPP_VERSION_STRING " (curl %s) %s", + curl_version_info(CURLVERSION_NOW)->version, + Target::targetDistribution( Pathname()/*guess root*/ ).c_str() ); + return ret; + }()}; return _value; } }