feat: improve glob handling

include/flox/core/types.hh

@@ -126,13 +126,18 @@ struct Subtree
   }
 
   /** @brief Implicitly convert a @a flox::Subtree to a string. */
-  constexpr explicit operator std::string_view() const
+  constexpr explicit
+  operator std::string_view() const
   {
     return to_string( *this );
   }
 
   // NOLINTNEXTLINE
-  constexpr operator subtree_type() const { return this->subtree; }
+  constexpr
+  operator subtree_type() const
+  {
+    return this->subtree;
+  }
 
   /** @brief Compare two @a flox::Subtree for equality. */
   [[nodiscard]] constexpr bool

include/flox/core/util.hh

@@ -22,6 +22,7 @@
 #include <nlohmann/json.hpp>
 
 #include "flox/core/exceptions.hh"
+#include "flox/core/types.hh"
 
 
 /* -------------------------------------------------------------------------- */
@@ -421,6 +422,14 @@ vectorMapOptional( const std::vector<T> & orig )
 
 /* -------------------------------------------------------------------------- */
 
+/**
+ * @brief Convert a @a AttrPathGlob to a string for display.
+ */
+std::string
+displayableGlobbedPath( const AttrPathGlob & attrs );
+
+/* -------------------------------------------------------------------------- */
+
 }  // namespace flox
 
 

include/flox/resolver/descriptor.hh

@@ -165,6 +165,49 @@ FLOX_DEFINE_EXCEPTION( ParseManifestDescriptorRawException,
 /** @} */
 
 
+/* -------------------------------------------------------------------------- */
+
+/**
+ * @brief Validates a single attribute name, `pname`, etc from a globbed
+ * @a flox::AttrPathGlob, returning the attribute name if it is suitable for
+ * appearing as a single attribute in a descriptor.
+ */
+std::optional<std::string>
+validatedSingleAttr( const AttrPathGlob & attrs );
+
+/**
+ * @brief Returns true if any component in the attribute path contains a glob
+ * but is not itself entirely a glob. For example, this would return `true` for
+ * `foo.b*ar.baz`, but not for `foo.*.baz` since `b*ar` contains a glob, but is
+ * not itself entirely a glob.
+ */
+bool
+globInAttrName( const AttrPathGlob & attrs );
+
+/**
+ * @brief Validates a relative attribute path from a globbed
+ * @a flox::AttrPathGlob, returning the string form of the relative path for
+ * use in the @a flox::resolver::ManifestDescriptorRaw.
+ */
+std::vector<std::string>
+validatedRelativePath( const AttrPathGlob &             attrs,
+                       const std::vector<std::string> & strings );
+
+/**
+ * @brief Validates an absolute path from a globbed
+ * @a flox::AttrPathGlob, returning the attribute path if it is suitable for
+ * an absolute path appearing in a descriptor.
+ */
+AttrPathGlob
+validatedAbsolutePath( const AttrPathGlob & attrs );
+
+/**
+ * @brief Returns `true` if the attribute path has enough path components and
+ * begins with one of the allowed prefixes (`legacyPackages` or `packages`).
+ */
+bool
+isAbsolutePath( const AttrPathGlob & attrs );
+
 /* -------------------------------------------------------------------------- */
 
 /**

src/resolver/descriptor.cc

@@ -490,40 +490,41 @@ ManifestDescriptorRaw::ManifestDescriptorRaw(
         "descriptor was missing a package name" );
     }
   auto attrsSubstr = descriptor.substr( cursor, attrsEndIdx - cursor );
-  std::vector<std::string> attrs = splitAttrPath( attrsSubstr );
+  auto attrsWithHandledGlobs
+    = maybeSplitAttrPathGlob( std::string( attrsSubstr ) );
+  auto attrsMaybeContainingGlobs = splitAttrPath( attrsSubstr );
   // Guard against `input:.` e.g. attrpaths with empty components
-  for ( auto & attr : attrs )
+  for ( auto & attr : attrsWithHandledGlobs )
     {
-      if ( attr.empty() )
+      if ( attr.has_value() && attr.value().empty() )
         {
           throw InvalidManifestDescriptorException(
             "descriptor attribute name was malformed: `"
             + std::string( attrsSubstr ) + "'" );
         }
     }
-  switch ( attrs.size() )
+  // Handle the attribute path provided by the user
+  switch ( attrsWithHandledGlobs.size() )
     {
       case 1:
         // Match against `name`, `pname`, or `attrName`
-        this->name = attrs[0];
+        this->name = validatedSingleAttr( attrsWithHandledGlobs );
         break;
       case 2:
         // We were definitely given a relative path
-        this->path = attrs;
+        this->path = validatedRelativePath( attrsWithHandledGlobs,
+                                            attrsMaybeContainingGlobs );
         break;
       default:
         // Could be a relative or absolute path depending on the prefix
-        if ( ( attrs[0] == "legacyPackages" ) || ( attrs[0] == "packages" ) )
+        if ( isAbsolutePath( attrsWithHandledGlobs ) )
           {
-            this->absPath = vectorMapOptional( attrs );
+            this->absPath = validatedAbsolutePath( attrsWithHandledGlobs );
           }
-        else if ( attrs.size() > 0 ) { this->path = attrs; }
         else
           {
-            // Someone gave us a path that's malformed e.g. it has zero length
-            // Note that this _should_ be unreachable
-            throw InvalidManifestDescriptorException(
-              "invalid attribute path: `" + std::string( attrsSubstr ) + "'" );
+            this->path = validatedRelativePath( attrsWithHandledGlobs,
+                                                attrsMaybeContainingGlobs );
           }
         break;
     }
@@ -536,6 +537,88 @@ ManifestDescriptorRaw::ManifestDescriptorRaw(
 
 /* -------------------------------------------------------------------------- */
 
+std::optional<std::string>
+validatedSingleAttr( const AttrPathGlob & attrs )
+{
+  if ( attrs[0].has_value() ) { return attrs[0]; }
+  throw InvalidManifestDescriptorException(
+    "globs are only allowed to replace entire system names: `"
+    + displayableGlobbedPath( attrs ) + "'" );
+}
+
+bool
+globInAttrName( const AttrPathGlob & attrs )
+{
+  for ( const std::optional<std::string> & attr : attrs )
+    {
+      if ( attr.has_value()
+           && ( std::find( attr.value().begin(), attr.value().end(), '*' )
+                != attr.value().end() ) )
+        {
+          return true;
+        }
+    }
+  return false;
+}
+
+std::vector<std::string>
+validatedRelativePath( const AttrPathGlob &             attrs,
+                       const std::vector<std::string> & strings )
+{
+  // Don't allow globs in relative paths. Relative paths don't contain
+  // the system name, so there's no reason for a glob to be here at all.
+  bool containsGlobbedAttr
+    = std::find( attrs.begin(), attrs.end(), std::nullopt ) != attrs.end();
+  if ( containsGlobbedAttr )
+    {
+      throw InvalidManifestDescriptorException(
+        "globs are only allowed to replace entire system names: `"
+        + displayableGlobbedPath( attrs ) + "'" );
+    }
+  else if ( globInAttrName( attrs ) )
+    {
+      throw InvalidManifestDescriptorException(
+        "globs are only allowed to replace entire system names: `"
+        + displayableGlobbedPath( attrs ) + "'" );
+    }
+  else if ( attrs.size() < 2 )
+    {
+      throw InvalidManifestDescriptorException(
+        "relative paths must contain at least 2 attributes" );
+    }
+  return strings;
+}
+
+AttrPathGlob
+validatedAbsolutePath( const AttrPathGlob & attrs )
+{
+  // We've already checked that the path is absolute, don't need to check the
+  // prefix
+  auto nGlobs       = std::count( attrs.begin(), attrs.end(), std::nullopt );
+  bool tooManyGlobs = nGlobs > 1;
+  bool systemNotGlobbed = attrs[1] != std::nullopt;
+  if ( tooManyGlobs || globInAttrName( attrs )
+       || ( ( nGlobs == 1 ) && systemNotGlobbed ) )
+    {
+      throw InvalidManifestDescriptorException(
+        "globs are only allowed to replace entire system names: `"
+        + displayableGlobbedPath( attrs ) + "'" );
+    }
+  return attrs;
+}
+
+bool
+isAbsolutePath( const AttrPathGlob & attrs )
+{
+  if ( attrs.size() < 3 ) { return false; }
+  auto                  maybePrefix      = attrs[0];
+  std::set<std::string> absolutePrefixes = { "legacyPackages", "packages" };
+  return ( maybePrefix.has_value()
+           && absolutePrefixes.contains( maybePrefix.value() ) );
+}
+
+/* -------------------------------------------------------------------------- */
+
 ManifestDescriptor::ManifestDescriptor( const ManifestDescriptorRaw & raw )
   : name( raw.name )
   , optional( raw.optional.value_or( false ) )

src/util.cc

@@ -21,6 +21,7 @@
 #include <nlohmann/json.hpp>
 
 #include "flox/core/exceptions.hh"
+#include "flox/core/types.hh"
 #include "flox/core/util.hh"
 
 
@@ -302,6 +303,26 @@ extract_json_errmsg( nlohmann::json::exception & err )
   return userFriendly;
 }
 
+/* -------------------------------------------------------------------------- */
+
+std::string
+displayableGlobbedPath( const flox::AttrPathGlob & attrs )
+{
+  std::vector<std::string> globbed;
+  for ( const std::optional<std::string> & attr : attrs )
+    {
+      if ( attr.has_value() ) { globbed.emplace_back( *attr ); }
+      else { globbed.emplace_back( "*" ); }
+    }
+  auto fold
+    = []( std::string a, std::string b ) { return std::move( a ) + '.' + b; };
+
+  std::string s = std::accumulate( std::next( globbed.begin() ),
+                                   globbed.end(),
+                                   globbed[0],
+                                   fold );
+  return s;
+}
 
 /* -------------------------------------------------------------------------- */
 

tests/descriptor.bats

@@ -346,3 +346,47 @@ setup_file() {
   assert_failure;
   assert_output --partial "descriptor attribute name was malformed";
 }
+
+@test "parse descriptor 'nixpkgs:*.foo'" {
+  query="nixpkgs:*.foo";
+  run "$PKGDB" parse descriptor --to manifest "$query";
+  assert_failure;
+  assert_output --partial "globs are only allowed to replace entire system names";
+  unset output;
+  run "$PKGDB" parse descriptor --to query "$query";
+  assert_failure;
+  assert_output --partial "globs are only allowed to replace entire system names";
+}
+
+@test "parse descriptor 'nixpkgs:*.foo.bar'" {
+  query="nixpkgs:*.foo.bar";
+  run "$PKGDB" parse descriptor --to manifest "$query";
+  assert_failure;
+  assert_output --partial "globs are only allowed to replace entire system names";
+  unset output;
+  run "$PKGDB" parse descriptor --to query "$query";
+  assert_failure;
+  assert_output --partial "globs are only allowed to replace entire system names";
+}
+
+@test "parse descriptor 'nixpkgs:packages.foo.*'" {
+  query="nixpkgs:packages.foo.*";
+  run "$PKGDB" parse descriptor --to manifest "$query";
+  assert_failure;
+  assert_output --partial "globs are only allowed to replace entire system names";
+  unset output;
+  run "$PKGDB" parse descriptor --to query "$query";
+  assert_failure;
+  assert_output --partial "globs are only allowed to replace entire system names";
+}
+
+@test "parse descriptor 'nixpkgs:packages.foo.b*ar'" {
+  query='nixpkgs:packages.foo.b*ar';
+  run "$PKGDB" parse descriptor --to manifest "$query";
+  assert_failure;
+  assert_output --partial "globs are only allowed to replace entire system names";
+  unset output;
+  run "$PKGDB" parse descriptor --to query "$query";
+  assert_failure;
+  assert_output --partial "globs are only allowed to replace entire system names";
+}