From 5a1824ebe1fcdeff86b57a13a33d6428e89e4bce Mon Sep 17 00:00:00 2001 From: Pierre Bourdon Date: Fri, 17 May 2024 02:02:48 +0200 Subject: [PATCH] derived-path: refuse built derived path with a non-derivation base Example: /nix/store/dr53sp25hyfsnzjpm8mh3r3y36vrw3ng-neovim-0.9.5^out This is nonsensical since selecting outputs can only be done for a buildable derivation, not for a realised store path. The build worker side of things ends up crashing with an assertion when trying to handle such malformed paths. Change-Id: Ia3587c71fe3da5bea45d4e506e1be4dd62291ddf --- src/libstore/derived-path.cc | 46 +++++++++++++---------------- tests/unit/libstore/derived-path.cc | 9 ++++++ 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/libstore/derived-path.cc b/src/libstore/derived-path.cc index 214caab54..22977c7b1 100644 --- a/src/libstore/derived-path.cc +++ b/src/libstore/derived-path.cc @@ -185,21 +185,32 @@ DerivedPath::Built DerivedPath::Built::parse( }; } -static SingleDerivedPath parseWithSingle( +template +static DerivedPathT parseDerivedPath( const Store & store, std::string_view s, std::string_view separator, const ExperimentalFeatureSettings & xpSettings) { size_t n = s.rfind(separator); - return n == s.npos - ? (SingleDerivedPath) SingleDerivedPath::Opaque::parse(store, s) - : (SingleDerivedPath) SingleDerivedPath::Built::parse(store, - make_ref(parseWithSingle( + if (n == s.npos) { + return DerivedPathT::Opaque::parse(store, s); + } else { + auto path = DerivedPathT::Built::parse(store, + make_ref(parseDerivedPath( store, s.substr(0, n), separator, xpSettings)), s.substr(n + 1), xpSettings); + + const auto& basePath = path.getBaseStorePath(); + if (!basePath.isDerivation()) { + throw InvalidPath("cannot use output selection ('%s') on non-derivation store path '%s'", + separator, basePath.to_string()); + } + + return path; + } } SingleDerivedPath SingleDerivedPath::parse( @@ -207,7 +218,7 @@ SingleDerivedPath SingleDerivedPath::parse( std::string_view s, const ExperimentalFeatureSettings & xpSettings) { - return parseWithSingle(store, s, "^", xpSettings); + return parseDerivedPath(store, s, "^", xpSettings); } SingleDerivedPath SingleDerivedPath::parseLegacy( @@ -215,24 +226,7 @@ SingleDerivedPath SingleDerivedPath::parseLegacy( std::string_view s, const ExperimentalFeatureSettings & xpSettings) { - return parseWithSingle(store, s, "!", xpSettings); -} - -static DerivedPath parseWith( - const Store & store, std::string_view s, std::string_view separator, - const ExperimentalFeatureSettings & xpSettings) -{ - size_t n = s.rfind(separator); - return n == s.npos - ? (DerivedPath) DerivedPath::Opaque::parse(store, s) - : (DerivedPath) DerivedPath::Built::parse(store, - make_ref(parseWithSingle( - store, - s.substr(0, n), - separator, - xpSettings)), - s.substr(n + 1), - xpSettings); + return parseDerivedPath(store, s, "!", xpSettings); } DerivedPath DerivedPath::parse( @@ -240,7 +234,7 @@ DerivedPath DerivedPath::parse( std::string_view s, const ExperimentalFeatureSettings & xpSettings) { - return parseWith(store, s, "^", xpSettings); + return parseDerivedPath(store, s, "^", xpSettings); } DerivedPath DerivedPath::parseLegacy( @@ -248,7 +242,7 @@ DerivedPath DerivedPath::parseLegacy( std::string_view s, const ExperimentalFeatureSettings & xpSettings) { - return parseWith(store, s, "!", xpSettings); + return parseDerivedPath(store, s, "!", xpSettings); } DerivedPath DerivedPath::fromSingle(const SingleDerivedPath & req) diff --git a/tests/unit/libstore/derived-path.cc b/tests/unit/libstore/derived-path.cc index c62d79a78..ffa541e9f 100644 --- a/tests/unit/libstore/derived-path.cc +++ b/tests/unit/libstore/derived-path.cc @@ -77,6 +77,15 @@ TEST_F(DerivedPathTest, built_built_xp) { MissingExperimentalFeature); } +/** + * Built paths with a non-derivation base should fail parsing. + */ +TEST_F(DerivedPathTest, non_derivation_base) { + ASSERT_THROW( + DerivedPath::parse(*store, "/nix/store/g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x^foo"), + InvalidPath); +} + #ifndef COVERAGE RC_GTEST_FIXTURE_PROP(