Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Url varexpand issues #497

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Url varexpand issues #497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3d8667d
fix typo in comment
mlandres Nov 17, 2023
cf3d9ab
Url: Fix bug when parsing empty components like query or fragment
mlandres Nov 17, 2023
9d4853e
Url: Fix regexes; '.' inside a character class must not be \-escaped
mlandres Nov 22, 2023
9ce8905
RepoVarExpand: Fix expansion of embedded words with invalid variable …
mlandres Nov 22, 2023
1faba2a
RepoVarExpand: VarRetriever callback is not needed for backslash esca…
mlandres Nov 22, 2023
82d408f
Add hasRepoVarsEmbedded test for strings
mlandres Nov 22, 2023
3219d97
Url: Add builtin "zypp-rawurl" schema for strings with unexpanded rep…
mlandres Nov 22, 2023
b889e22
Add RawUrl for constructing the zypp-rawurl: schema and handle it in …
mlandres Nov 24, 2023
a7d1a12
WIP
mlandres Nov 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/autoinclude/FeatureTest.doc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ Packages requiring a feature may use the corresponding \c Requires: in their .sp
<DD>Also support user defined repo variables in /etc/zypp/vars.d.</DD>
<DT>version 1.1</DT>
<DD>Support repo variables in an URIs \c host and \c port component.</DD>
<DT>version 1.2</DT>
<DD>Support repo variables forming valid URIs after expansion - but not before.</DD>
</DL></DD>
</DL>

Expand Down
5 changes: 3 additions & 2 deletions doc/autoinclude/RepoVariables.doc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ The variable expander also supports shell like definition of default and alterna
\see \ref zypp::repo::RepoVarExpand Variable expander


\subsection zypp-repoars-builtin Builtin repository variables
\subsection zypp-repovars-builtin Builtin repository variables

\li \c $arch -
The system's CPU architecture.
Expand All @@ -31,10 +31,11 @@ The variable expander also supports shell like definition of default and alterna
\c $releasever_major will be set to the leading portion up to (but not including) the 1st dot; \c $releasever_minor to the trailing portion after the 1st dot. If there's no dot in \c $releasever, \c $releasever_major is the same as \c $releasever and \c $releasever_minor is empty.


\subsection zypp-repoars-userdefined User defined repository variables [requires 'libzypp(repovarexpand) >= 1']
\subsection zypp-repovars-userdefined User defined repository variables [requires 'libzypp(repovarexpand) >= 1']

A custom repository variable is defined by creating a file in \c /etc/zypp/vars.d. The variable name equals the file name. The files first line (up to but not including the newline character) defines the variables value. Valid variable(file) names consist of alphanumeric chars and '_' only.

Variable substitution within an URIs authority [requires 'libzypp(repovarexpand) >= 1.1'] is limited to \c host and \c port. Bash style definition of default and alternate values is not supported. No variables can be used in an URIs \c scheme, \c user and \c password.

The use of arbitrary strings which form a valid url after variable substitution - but not before - requires 'libzypp(repovarexpand) >= 1.2'. The \c zypp-rawurl: schema is used to wrap the unexpanded strings into a valid \ref Url until they get expanded (\see \ref zypp::RawUrl).
*/
2 changes: 1 addition & 1 deletion libzypp.spec.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ Provides: libzypp(plugin:services) = 1
Provides: libzypp(plugin:system) = 1
Provides: libzypp(plugin:urlresolver) = 0
Provides: libzypp(plugin:repoverification) = 0
Provides: libzypp(repovarexpand) = 1.1
Provides: libzypp(repovarexpand) = 1.2

%if 0%{?suse_version}
Recommends: logrotate
Expand Down
92 changes: 92 additions & 0 deletions tests/parser/RepoFileReader_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,23 @@ using std::stringstream;
using std::string;
using namespace zypp;

// 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 <zypp/ZYppFactory.h>
auto guard { getZYpp() };
namespace zyppintern {
std::map<std::string,std::string> repoVariablesGet();
void repoVariablesSwap( std::map<std::string,std::string> & val_r );
}
// ---

static std::string suse_repo = "[factory-oss]\n"
"name=factory-oss $releasever - $basearch\n"
"failovermethod=priority\n"
Expand Down Expand Up @@ -56,3 +73,78 @@ BOOST_AUTO_TEST_CASE(read_repo_file)
BOOST_CHECK_EQUAL( Url("http://serv.er/loc1"), repo.mirrorListUrl() );
}
}

BOOST_AUTO_TEST_CASE(rawurl2repoinfo)
{
// Set up repo variables...
std::map<std::string,std::string> vardef { ::zyppintern::repoVariablesGet() };
vardef["releasever"] = "myversion";
vardef["basearch"] = "myarch";
vardef["OPENSUSE_DISTURL"] = "https://cdn.opensuse.org/repositories/";
::zyppintern::repoVariablesSwap( vardef );

{
std::stringstream input(
"[leap-repo]\n"
"name=leap-repo $releasever - $basearch\n"
"baseurl= ${OPENSUSE_DISTURL}leap/repo\n"
"gpgkey= ${OPENSUSE_DISTURL}leap/repo\n"
"mirrorlist= ${OPENSUSE_DISTURL}leap/repo\n"
"metalink= ${OPENSUSE_DISTURL}leap/repo\n"
);
RepoCollector collector;
parser::RepoFileReader parser( input, bind( &RepoCollector::collect, &collector, _1 ) );
BOOST_CHECK_EQUAL(1, collector.repos.size());

const RepoInfo & repo( collector.repos.front() );
BOOST_CHECK_EQUAL( repo.name(), "leap-repo myversion - myarch" );

BOOST_CHECK_EQUAL( repo.url().asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo" );
BOOST_CHECK_EQUAL( repo.gpgKeyUrl().asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo" );
BOOST_CHECK_EQUAL( repo.mirrorListUrl().asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo" );

BOOST_CHECK_EQUAL( repo.rawUrl().asCompleteString(), "zypp-rawurl:#${OPENSUSE_DISTURL}leap/repo" );
BOOST_CHECK_EQUAL( repo.rawGpgKeyUrl().asCompleteString(), "zypp-rawurl:#${OPENSUSE_DISTURL}leap/repo" );
BOOST_CHECK_EQUAL( repo.rawMirrorListUrl().asCompleteString(), "zypp-rawurl:#${OPENSUSE_DISTURL}leap/repo" );
}

// RepoFileReader unfortunately encodes "proxy=" into the URLs
// query part (by now just for the baseurl). For RawUrls the
// VarReplacer needs to take this into account.
{
std::stringstream input(
"[leap-repo]\n"
"name=leap-repo $releasever - $basearch\n"
"proxy=myproxy.host:1234\n"
"baseurl=${OPENSUSE_DISTURL}leap/repo\n"
"baseurl=https://cdn.opensuse.org/repositories/leap/repo\n"
"baseurl=https://cdn.opensuse.org/repositories/leap/repo?proxy=otherproxy.host\n"
"gpgkey=${OPENSUSE_DISTURL}leap/repo\n"
"mirrorlist=${OPENSUSE_DISTURL}leap/repo\n"
"metalink=${OPENSUSE_DISTURL}leap/repo\n"
);
RepoCollector collector;
parser::RepoFileReader parser( input, bind( &RepoCollector::collect, &collector, _1 ) );
BOOST_CHECK_EQUAL(1, collector.repos.size());

const RepoInfo & repo( collector.repos.front() );
BOOST_CHECK_EQUAL( repo.name(), "leap-repo myversion - myarch" );

BOOST_CHECK_EQUAL( repo.baseUrlsSize(), 3 );
const auto & baseurls = repo.baseUrls();
auto iter = baseurls.begin();
BOOST_CHECK_EQUAL( (iter++)->asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo?proxy=myproxy.host&proxyport=1234" );
BOOST_CHECK_EQUAL( (iter++)->asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo?proxy=myproxy.host&proxyport=1234" );
BOOST_CHECK_EQUAL( (iter++)->asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo?proxy=otherproxy.host" );
BOOST_CHECK_EQUAL( repo.gpgKeyUrl().asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo" );
BOOST_CHECK_EQUAL( repo.mirrorListUrl().asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo" );

const auto & rawbaseurls = repo.rawBaseUrls();
iter = rawbaseurls.begin();
BOOST_CHECK_EQUAL( (iter++)->asCompleteString(), "zypp-rawurl:?proxy=myproxy.host&proxyport=1234#${OPENSUSE_DISTURL}leap/repo" );
BOOST_CHECK_EQUAL( (iter++)->asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo?proxy=myproxy.host&proxyport=1234" );
BOOST_CHECK_EQUAL( (iter++)->asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo?proxy=otherproxy.host" );
BOOST_CHECK_EQUAL( repo.rawGpgKeyUrl().asCompleteString(), "zypp-rawurl:#${OPENSUSE_DISTURL}leap/repo" );
BOOST_CHECK_EQUAL( repo.rawMirrorListUrl().asCompleteString(), "zypp-rawurl:#${OPENSUSE_DISTURL}leap/repo" );
}
}
107 changes: 99 additions & 8 deletions tests/repo/RepoVariables_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <zypp/ZYppFactory.h>
auto guard { getZYpp() };
namespace zyppintern {
std::map<std::string,std::string> repoVariablesGet();
void repoVariablesSwap( std::map<std::string,std::string> & val_r );
}
// ---

#define DATADIR (Pathname(TESTS_SRC_DIR) + "/repo/yum/data")

typedef std::list<std::string> ListType;
Expand Down Expand Up @@ -73,7 +90,7 @@ void helperGenRepVarExpandResults()
{
// Generate test result strings for RepVarExpand:
// ( STRING, REPLACED_all_vars_undef, REPLACED_all_vars_defined )
// Crefully check whether new stings are correct before
// Carefully check whether new stings are correct before
// adding them to the testccse.
std::map<std::string,std::string> vartable;
std::map<std::string,std::pair<std::string,std::string>> result;
Expand Down Expand Up @@ -125,7 +142,7 @@ void helperGenRepVarExpandResults()
}
}

void RepVarExpandTest( const std::string & string_r, const std::string & allUndef_r, const std::string & allDef_r )
void RepVarExpandTest( const std::string & string_r, const std::string & allUndef_r, const std::string & allDef_r, bool hasVars=true )
{
std::map<std::string,std::string> vartable;
bool varsoff = true;
Expand All @@ -140,30 +157,31 @@ void RepVarExpandTest( const std::string & string_r, const std::string & allUnde
return &val;
};

BOOST_CHECK_EQUAL( repo::hasRepoVarsEmbedded( string_r ), hasVars );
varsoff = true;
BOOST_CHECK_EQUAL( repo::RepoVarExpand()( string_r, varLookup ), allUndef_r );
varsoff = false;
BOOST_CHECK_EQUAL( repo::RepoVarExpand()( string_r, varLookup ), allDef_r );
}

BOOST_AUTO_TEST_CASE(RepVarExpand)
{ // ( STRING , REPLACED_all_vars_undef , REPLACED_all_vars_defined )
RepVarExpandTest( "" , "" , "" );
RepVarExpandTest( "$" , "$" , "$" );
RepVarExpandTest( "$${}" , "$${}" , "$${}" );
{ // ( STRING , REPLACED_all_vars_undef , REPLACED_all_vars_defined STRING has Vars)
RepVarExpandTest( "" , "" , "" , false );
RepVarExpandTest( "$" , "$" , "$" , false );
RepVarExpandTest( "$${}" , "$${}" , "$${}" , false );
RepVarExpandTest( "$_:" , "$_:" , "[_]:" );
RepVarExpandTest( "$_A:" , "$_A:" , "[_A]:" );
RepVarExpandTest( "$_A_:" , "$_A_:" , "[_A_]:" );
RepVarExpandTest( "$_A_B:" , "$_A_B:" , "[_A_B]:" );
RepVarExpandTest( "${C:+a$Bba\\}" , "${C:+a$Bba\\}" , "${C:+a[Bba]\\}" );
RepVarExpandTest( "${C:+a$Bba}" , "" , "a[Bba]" );
RepVarExpandTest( "${C:+a${B\\}ba}" , "${C:+a${B\\}ba}" , "${C:+a${B\\}ba}" );
RepVarExpandTest( "${C:+a${B\\}ba}" , "" , "a${B}ba" );
RepVarExpandTest( "${C:+a${B}ba}" , "" , "a[B]ba" );
RepVarExpandTest( "${C:+a\\$Bba}" , "" , "a$Bba" );
RepVarExpandTest( "${C:+a\\${B\\}ba}" , "" , "a${B}ba" );
RepVarExpandTest( "${C:+a\\${B}ba}" , "ba}" , "a${Bba}" );
RepVarExpandTest( "${C:-a$Bba}" , "a$Bba" , "[C]" );
RepVarExpandTest( "${_A_B\\}" , "${_A_B\\}" , "${_A_B\\}" );
RepVarExpandTest( "${_A_B\\}" , "${_A_B\\}" , "${_A_B\\}" , false );
RepVarExpandTest( "${_A_B}" , "${_A_B}" , "[_A_B]" );
RepVarExpandTest( "\\${_A_B}" , "\\${_A_B}" , "\\[_A_B]" );
RepVarExpandTest( "__${D:+\\$X--{${E:-==\\$X{o\\}==} }--}__\\${B}${}__", "__--}__\\${B}${}__" , "__$X--{[E] --}__\\[B]${}__" );
Expand Down Expand Up @@ -248,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<std::string,std::string> 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<std::string,std::string> 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:
Loading