Ensure PATH components are handled consistently

Author: falconindy

src/filter.cc

@@ -81,11 +81,8 @@ Exact::Exact(std::string match, bool case_sensitive) {
     };
   } else {
     predicate_ = [m = std::move(match)](std::string_view line) {
-      if (line.size() != m.size()) {
-        return false;
-      }
-
-      return strncasecmp(line.data(), m.data(), m.size()) == 0;
+      return line.size() == m.size() &&
+             strncasecmp(line.data(), m.data(), m.size()) == 0;
     };
   }
 }

src/filter.hh

@@ -4,6 +4,7 @@
 
 #include <functional>
 #include <memory>
+#include <span>
 #include <string>
 #include <string_view>
 
@@ -52,15 +53,15 @@ class Directory : public Filter {
 
 class Bin : public Filter {
  public:
-  explicit Bin(const std::vector<std::string>& bins) : bins_(bins) {}
+  explicit Bin(std::span<const std::string> bins) : bins_(bins) {}
 
   bool Matches(std::string_view line) const override;
 
  private:
   // We use this as an optimization for throwing out things early, i.e.
   // directories can't be binaries.
   Directory directory_filter_;
-  const std::vector<std::string>& bins_;
+  std::span<const std::string> bins_;
 };
 
 class Regex : public Filter {

src/pkgfile.cc

@@ -9,6 +9,7 @@
 
 #include <format>
 #include <iostream>
+#include <map>
 #include <optional>
 #include <set>
 #include <vector>
@@ -38,6 +39,44 @@ std::string WeaklyCanonicalizeBin(std::string_view path) {
   return fs::weakly_canonical(canonical);
 }
 
+bool IsValidBin(std::string_view path) {
+  if (path.empty() || path.starts_with("/home") || !path.starts_with('/')) {
+    return false;
+  }
+
+  return true;
+}
+
+std::vector<std::string> ParsePathVarToBins(const char* var) {
+  if (var == nullptr) {
+    return {};
+  }
+
+  std::set<std::string> bins;
+  std::string_view psv(var);
+
+  while (!psv.empty()) {
+    auto pos = psv.find(':');
+    if (pos == psv.npos) {
+      if (IsValidBin(psv)) {
+        // then the remainder goes in the vector
+        bins.emplace(WeaklyCanonicalizeBin(psv));
+      }
+      break;
+    }
+
+    std::string_view component(psv.data(), pos);
+    if (IsValidBin(component)) {
+      bins.emplace(WeaklyCanonicalizeBin(component));
+    }
+
+    psv.remove_prefix(pos + 1);
+  }
+
+  return {std::make_move_iterator(bins.begin()),
+          std::make_move_iterator(bins.end())};
+}
+
 }  // namespace
 
 Pkgfile::Pkgfile(Options options) : options_(options) {
@@ -64,32 +103,7 @@ Pkgfile::Pkgfile(Options options) : options_(options) {
       break;
   }
 
-  if (const char* p = getenv("PATH"); p) {
-    std::string_view psv(p);
-
-    while (!psv.empty()) {
-      auto pos = psv.find(':');
-      if (pos == psv.npos) {
-        // then the remainder goes in the vector
-        bins_.emplace_back(WeaklyCanonicalizeBin(psv));
-        break;
-      }
-
-      // Remove any trailing slashes from the PATH component and normalize it.
-      std::string_view component = {psv.data(), pos};
-      while (!component.empty() && component.ends_with('/')) {
-        component.remove_suffix(1);
-      }
-
-      // No relative paths
-      if (!component.empty() && component.starts_with('/') &&
-          !component.starts_with("/home")) {
-        bins_.emplace_back(WeaklyCanonicalizeBin(component));
-      }
-
-      psv.remove_prefix(pos + 1);
-    }
-  }
+  bins_ = ParsePathVarToBins(getenv("PATH"));
 }
 
 std::string Pkgfile::FormatSearchResult(const std::string& repo,

src/pkgfile.hh

@@ -1,8 +1,6 @@
 #pragma once
 
 #include <filesystem>
-#include <map>
-#include <optional>
 
 #include "archive_reader.hh"
 #include "db.hh"
@@ -63,8 +61,6 @@ class Pkgfile {
     std::string_view version;
   };
 
-  using RepoMap = std::multimap<std::string, std::filesystem::path>;
-
   using ArchiveEntryCallback = std::function<int(
       const std::string& repo, const filter::Filter& filter,
       const ParsedPkgname& pkg, Result* result, ArchiveReader* reader)>;