diff --git a/tests/repo/RepoVariables_test.cc b/tests/repo/RepoVariables_test.cc index 25c88f4d3c..9dc159a022 100644 --- a/tests/repo/RepoVariables_test.cc +++ b/tests/repo/RepoVariables_test.cc @@ -14,6 +14,23 @@ using std::endl; using namespace zypp; using namespace boost::unit_test; +// This is kind of ugly for testing: Whenever a zypp lock is +// acquired the repo variables are cleared and re-read from disk, +// which is ok (by ZYppFactory). +// The VarReplacer itself however acquires a lock to resolve the +// Target variables (like $releasever), which is ok as well. +// For testing however - if we want to manipulate the variable +// definitions - we must hold a lock ourselves. Otherwise the +// VarReplacer will acquired one and so discard our custom +// settings. +#include +auto guard { getZYpp() }; +namespace zyppintern { + std::map repoVariablesGet(); + void repoVariablesSwap( std::map & val_r ); +} +// --- + #define DATADIR (Pathname(TESTS_SRC_DIR) + "/repo/yum/data") typedef std::list ListType; @@ -249,4 +266,77 @@ BOOST_AUTO_TEST_CASE(uncached) ::setenv( "ZYPP_REPO_RELEASEVER", "13.3", 1 ); BOOST_CHECK_EQUAL( replacer1("${releasever}"), "13.3" ); } + +BOOST_AUTO_TEST_CASE(replace_rawurl) +{ + std::string s; + + s = "http://$host/repositories"; // lowercase, so it works for both (Url stores host component lowercased) + { + Url u { s }; + RawUrl r { s }; + // check replacing... + repo::RepoVariablesUrlReplacer replacer; + std::map vardef { ::zyppintern::repoVariablesGet() }; + + vardef["host"] = ""; // make sure it's not defined + ::zyppintern::repoVariablesSwap( vardef ); + BOOST_CHECK_THROW( replacer( u ).asCompleteString(), zypp::url::UrlNotAllowedException ); // Url scheme requires a host component + BOOST_CHECK_THROW( replacer( r ).asCompleteString(), zypp::url::UrlNotAllowedException ); // Url scheme requires a host component + + vardef["host"] = "cdn.opensuse.org"; + ::zyppintern::repoVariablesSwap( vardef ); + BOOST_CHECK_EQUAL( replacer( u ).asCompleteString(), "http://cdn.opensuse.org/repositories" ); + BOOST_CHECK_EQUAL( replacer( r ).asCompleteString(), "http://cdn.opensuse.org/repositories" ); + + vardef["host"] = "cdn.opensuse.org/pathadded"; + ::zyppintern::repoVariablesSwap( vardef ); + BOOST_CHECK_EQUAL( replacer( u ).asCompleteString(), "http://cdn.opensuse.org/pathadded/repositories" ); + BOOST_CHECK_EQUAL( replacer( r ).asCompleteString(), "http://cdn.opensuse.org/pathadded/repositories" ); + + vardef["host"] = "cdn.opensuse.org:1234/pathadded"; + ::zyppintern::repoVariablesSwap( vardef ); + BOOST_CHECK_EQUAL( replacer( u ).asCompleteString(), "http://cdn.opensuse.org:1234/pathadded/repositories" ); + BOOST_CHECK_EQUAL( replacer( r ).asCompleteString(), "http://cdn.opensuse.org:1234/pathadded/repositories" ); + + vardef["host"] = "//something making the Url invalid//"; + ::zyppintern::repoVariablesSwap( vardef ); + BOOST_CHECK_THROW( replacer( u ).asCompleteString(), zypp::url::UrlNotAllowedException ); // Url scheme requires a host component + BOOST_CHECK_THROW( replacer( r ).asCompleteString(), zypp::url::UrlNotAllowedException ); // Url scheme requires a host component + } + + // If embedded vars do not form a valid Url, RawUrl must be used to carry them. + // But one should make sure the expanded string later forms a valid Url. + s = "${OPENSUSE_DISTURL:-http://cdn.opensuse.org/repositories/}leap/repo"; + BOOST_CHECK_THROW( Url{ s }, zypp::url::UrlBadComponentException ); + { + RawUrl r { s }; + BOOST_CHECK_EQUAL( r.getScheme(), "zypp-rawurl" ); + BOOST_CHECK_EQUAL( r.getFragment(), s ); + // Make sure a RawUrl (as Url or String) is not re-evaluated when fed + // back into a RawUrl. I.e. no zypp-rawurl as payload of a zypp-rawurl. + BOOST_CHECK_EQUAL( r, RawUrl( r ) ); + BOOST_CHECK_EQUAL( r, RawUrl( r.asCompleteString() ) ); + // Of course you can always do Url("zypp-rawurl:#zypp-rawurl:#$VAR"), + // but then you probably know what you are doing. + BOOST_CHECK_EQUAL( RawUrl( "zypp-rawurl:#zypp-rawurl:%23$VAR" ).asCompleteString(), "zypp-rawurl:#zypp-rawurl:%23$VAR" ); + BOOST_CHECK_EQUAL( Url( "zypp-rawurl:#zypp-rawurl:%23$VAR" ).asCompleteString(), "zypp-rawurl:#zypp-rawurl:%23$VAR" ); + + // check replacing... + repo::RepoVariablesUrlReplacer replacer; + std::map vardef { ::zyppintern::repoVariablesGet() }; + + vardef["OPENSUSE_DISTURL"] = ""; // make sure it's not defined + ::zyppintern::repoVariablesSwap( vardef ); + BOOST_CHECK_EQUAL( replacer( r ).asCompleteString(), "http://cdn.opensuse.org/repositories/leap/repo" ); + + vardef["OPENSUSE_DISTURL"] = "https://mymirror.org/"; // custom value + ::zyppintern::repoVariablesSwap( vardef ); + BOOST_CHECK_EQUAL( replacer( r ).asCompleteString(), "https://mymirror.org/leap/repo" ); + + vardef["OPENSUSE_DISTURL"] = "//something making the Url invalid//"; + ::zyppintern::repoVariablesSwap( vardef ); + BOOST_CHECK_THROW( replacer( r ).asCompleteString(), zypp::url::UrlBadComponentException ); // Url scheme is a required component + } +} // vim: set ts=2 sts=2 sw=2 ai et: diff --git a/tests/zypp/Url_test.cc b/tests/zypp/Url_test.cc index fe9d8ea972..08bb9221a6 100644 --- a/tests/zypp/Url_test.cc +++ b/tests/zypp/Url_test.cc @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -317,4 +318,61 @@ BOOST_AUTO_TEST_CASE(plugin_querystring_args) BOOST_CHECK_EQUAL( pm["o"], "" ); } +BOOST_AUTO_TEST_CASE(rawurls) +{ + std::string s; + // Both throw if constructed from an empty string (which is a legacy decision). + BOOST_CHECK_THROW( Url{ s }, zypp::url::UrlBadComponentException ); + BOOST_CHECK_THROW( RawUrl{ s }, zypp::url::UrlBadComponentException ); + + // Without embedded vars both forms are the same + s = "http://cdn.opensuse.org/repositories"; + BOOST_CHECK_EQUAL( Url{ s }, RawUrl{ s } ); + + // Without embedded vars both forms throw if Url is not valid + s = "no url"; + BOOST_CHECK_THROW( Url{ s }, zypp::url::UrlBadComponentException ); + BOOST_CHECK_THROW( RawUrl{ s }, zypp::url::UrlBadComponentException ); + + // Embedded vars may form a valid Url (apparently). RawUrl however always forms the + // zypp-rawurl: schema if vars are embedded because it resolves some issues. RawUrl + // is always able to restore the original unexpanded string. Url is not. + s = "http://$HOST/repositories"; + { + Url u { s }; + BOOST_CHECK_EQUAL( u.getScheme(), "http" ); + // OOps: A variable in the host part of an Url must be defined lowercased, + // because the Url ctor stores the lowercased 'hostname'. + BOOST_CHECK_EQUAL( u.asCompleteString(), "http://$host/repositories" ); // not "http://$HOST/repositories + + // RawUrl is able to restore the original unexpanded string... + RawUrl r { s }; + BOOST_CHECK_EQUAL( r.getScheme(), "zypp-rawurl" ); + BOOST_CHECK_EQUAL( r.getFragment(), s ); + + // OOps: Conditional vars in the host part don't work at all for Url. + s = "http://${HOST:-defaulthost}/repositories"; + BOOST_CHECK_THROW( Url{ s }, zypp::url::UrlBadComponentException ); // Invalid port component '-defaulthost} + r = s; + BOOST_CHECK_EQUAL( r.getScheme(), "zypp-rawurl" ); + BOOST_CHECK_EQUAL( r.getFragment(), s ); + } + + // If embedded vars do not form a valid Url, RawUrl must be used to carry them. + // But one should make sure the expanded string later forms a valid Url. + s = "${OPENSUSE_DISTURL:-http://cdn.opensuse.org/repositories/}leap/repo"; + BOOST_CHECK_THROW( Url{ s }, zypp::url::UrlBadComponentException ); + { + RawUrl r { s }; + BOOST_CHECK_EQUAL( r.getScheme(), "zypp-rawurl" ); + BOOST_CHECK_EQUAL( r.getFragment(), s ); + + // The RawUrl can then be used and shipped as Url. + // The schema however stays zypp-rawurl: until a RepoVariablesUrlReplacer + // is used to form the intended Url. + BOOST_CHECK_EQUAL( r, Url{ r } ); + BOOST_CHECK_EQUAL( r, Url{ r.asCompleteString() } ); + } +} + // vim: set ts=2 sts=2 sw=2 ai et: diff --git a/zypp/repo/RepoVariables.cc b/zypp/repo/RepoVariables.cc index 4b4a219db7..1221d6c10f 100644 --- a/zypp/repo/RepoVariables.cc +++ b/zypp/repo/RepoVariables.cc @@ -560,26 +560,51 @@ namespace zypp Url RepoVariablesUrlReplacer::operator()( const Url & value ) const { - Url::ViewOptions toReplace = value.getViewOptions() - url::ViewOption::WITH_USERNAME - url::ViewOption::WITH_PASSWORD; - // Legacy: Not 100% correct because it substitutes inside the 'proxypass=' value, - // but this was done before as well. The final fix will have to keep the proxypasswd - // out side the url in a credential file. - Url tmpurl { value }; - tmpurl.setViewOptions( toReplace ); - const std::string & replaced( RepoVarExpand()( hotfix1050625::asString( tmpurl ), RepoVarsMap::lookup ) ); - Url newurl; - if ( !replaced.empty() ) - { - newurl = replaced; - newurl.setUsername( value.getUsername( url::E_ENCODED ), url::E_ENCODED ); - newurl.setPassword( value.getPassword( url::E_ENCODED ), url::E_ENCODED ); - newurl.setViewOptions( value.getViewOptions() ); + if ( value.isValid() ) { + // The zypp-rawurl: schema is replaced by extracting and replacing the raw stringvalue. + // Other schmemata are replaced the traditional way. + if ( value.schemeIsRawUrl() ) { + const std::string & replaced { RepoVarExpand()( value.getFragment(), RepoVarsMap::lookup ) }; + newurl = Url( replaced ); + } else { + // The traditional + Url::ViewOptions toReplace = value.getViewOptions() - url::ViewOption::WITH_USERNAME - url::ViewOption::WITH_PASSWORD; + // Legacy: Not 100% correct because it substitutes inside the 'proxypass=' value, + // but this was done before as well. The final fix will have to keep the proxypasswd + // out side the url in a credential file. + Url tmpurl { value }; + tmpurl.setViewOptions( toReplace ); + const std::string & replaced( RepoVarExpand()( hotfix1050625::asString( tmpurl ), RepoVarsMap::lookup ) ); + + if ( !replaced.empty() ) + { + newurl = replaced; + newurl.setUsername( value.getUsername( url::E_ENCODED ), url::E_ENCODED ); + newurl.setPassword( value.getPassword( url::E_ENCODED ), url::E_ENCODED ); + newurl.setViewOptions( value.getViewOptions() ); + } + } } return newurl; } } // namespace repo /////////////////////////////////////////////////////////////////// + + //////////////////////////////////////////////////////////////////// + // class RawUrl + //////////////////////////////////////////////////////////////////// + + RawUrl::RawUrl( const std::string & encodedUrl_r ) + { + if ( str::startsWith( encodedUrl_r, "zypp-rawurl:" ) || not repo::hasRepoVarsEmbedded( encodedUrl_r ) ) { + *this = Url( encodedUrl_r ); + } else { + *this = Url( "zypp-rawurl:" ); + setFragment( encodedUrl_r ); + } + } + } // namespace zypp /////////////////////////////////////////////////////////////////// /////////////////////////////////////////////////////////////////// @@ -589,6 +614,11 @@ namespace zyppintern // internal helper called when re-acquiring the lock void repoVariablesReset() { repo::RepoVarsMap::instance().clear(); } + // Direct inspection and manipulation of the var-set for debugging and testcases + std::map repoVariablesGet() + { return repo::RepoVarsMap::instance(); } + void repoVariablesSwap( std::map & val_r ) + { return repo::RepoVarsMap::instance().swap( val_r ); } } // namespace zyppintern /////////////////////////////////////////////////////////////////// diff --git a/zypp/repo/RepoVariables.h b/zypp/repo/RepoVariables.h index a67b22dce1..d155786954 100644 --- a/zypp/repo/RepoVariables.h +++ b/zypp/repo/RepoVariables.h @@ -116,8 +116,8 @@ namespace zypp /** * \short Functor replacing repository variables * - * Replaces repository variables in the URL (except for user/pass inside authority) - * \see RepoVariablesStringReplacer + * Replaces repository variables in the URL + * \see \ref RepoVariablesStringReplacer and \ref RawUrl */ struct RepoVariablesUrlReplacer { @@ -138,6 +138,59 @@ namespace zypp /** \relates RepoVariablesUrlReplacer Helper managing repo variables replaced url lists */ typedef base::ContainerTransform, repo::RepoVariablesUrlReplacer> RepoVariablesReplacedUrlList; + //////////////////////////////////////////////////////////////////// + /// \class RawUrl + /// \brief Convenience type to create \c zypp-rawurl: Urls. + /// + /// Strings containing embedded RepoVariables (\see \ref repo::RepoVarExpand) + /// may not form a valid \ref Url until the variables are actually expanded. + /// The \c zypp-rawurl: schema was introduced to allow shipping such strings + /// through legacy APIs expecting a valid \ref Url. Prior to that even the + /// unexpanded string has to form a valid \ref Url which limited the use of + /// repo variables in URLs. + /// + /// The unexpanded string is shipped as payload encoded in the Url's fragment + /// part. Later a \ref RepoVariablesStringReplacer can be used to form the + /// intended \ref Url by substituting the variables. + /// + /// The typical use case are url strings read from a .repo (or .service) file. + /// They are stored as\c zypp-rawurl: within a \ref RepoInfo (or \ref ServiceInfo) + /// in case they have to written back to the file. The application usually refers + /// to the expanded \ref Url, but those classes offer to retrieve the raw Urls + /// as well - as type \ref Url. + /// + /// If the string does not contain any variables at all, the native \u Url is + /// constructed. + /// + /// \code + /// std::string s { "$DISTURL/leap/15.6/repo/oss" } + /// Url raw { s }; // Throws because s does not form a valid Url + /// RawUrl raw { s }; // OK; creates a zypp-rawurl: with s as payload + /// + /// // with DISTURL=https://cdn.opensuse.org/distribution + /// Url url { RepoVariablesUrlReplacer()( raw ) }; + /// // forms https://cdn.opensuse.org/distribution/leap/15.6/repo/oss + /// \endcode + /// + /// \note Support for repo variables forming valid URIs after expansion only + /// requires a libzypp provinding 'libzypp(repovarexpand) >= 1.2' + /// (\see \ref feature-test). + /// + /// \note Methods returning RawUrls are often deprecated because printing them + /// may accidentally expose credentials (password,proxypassword) embedded in the + /// raw url-string. An url-string with embedded RepoVariables, forms a valid + /// \ref Url after variables have been expanded. In the expanded Url passwords + /// are protected. The unexpanded url-string however does not even need to form + /// a valid Url, so sensitive data in the string can not be detected and protected. + /// This is why the \c zypp-rawurl: schema, which is used to store the unexpanded + /// url-strings, per default prints just it's schema, but not it's content. + struct RawUrl : public Url + { + RawUrl() : Url() {} + RawUrl( const Url & url_r ) : Url( url_r ) {} + RawUrl( const std::string & encodedUrl_r ); + }; + } // namespace zypp ///////////////////////////////////////////////////////////////////