Fix GC failures on bad store path names

It failed on names like '/nix/store/9ip48nkc9rfy0a4yaw98lp6gipqlib1a-'.
This commit is contained in:
Eelco Dolstra 2020-02-28 18:07:10 +01:00
parent 2e953b567e
commit 22a754c091
No known key found for this signature in database
GPG key ID: 8170B4726D7198DE
4 changed files with 35 additions and 29 deletions

View file

@ -255,12 +255,11 @@ void LocalStore::findTempRoots(FDs & fds, Roots & tempRoots, bool censor)
void LocalStore::findRoots(const Path & path, unsigned char type, Roots & roots) void LocalStore::findRoots(const Path & path, unsigned char type, Roots & roots)
{ {
auto foundRoot = [&](const Path & path, const Path & target) { auto foundRoot = [&](const Path & path, const Path & target) {
Path storePath = toStorePath(target); auto storePath = maybeParseStorePath(toStorePath(target));
// FIXME if (storePath && isValidPath(*storePath))
if (isStorePath(storePath) && isValidPath(parseStorePath(storePath))) roots[std::move(*storePath)].emplace(path);
roots[parseStorePath(storePath)].emplace(path);
else else
printInfo("skipping invalid root from '%1%' to '%2%'", path, storePath); printInfo("skipping invalid root from '%1%' to '%2%'", path, target);
}; };
try { try {
@ -296,10 +295,9 @@ void LocalStore::findRoots(const Path & path, unsigned char type, Roots & roots)
} }
else if (type == DT_REG) { else if (type == DT_REG) {
Path storePath = storeDir + "/" + std::string(baseNameOf(path)); auto storePath = maybeParseStorePath(storeDir + "/" + std::string(baseNameOf(path)));
// FIXME if (storePath && isValidPath(*storePath))
if (isStorePath(storePath) && isValidPath(parseStorePath(storePath))) roots[std::move(*storePath)].emplace(path);
roots[parseStorePath(storePath)].emplace(path);
} }
} }
@ -523,14 +521,14 @@ void LocalStore::deletePathRecursive(GCState & state, const Path & path)
unsigned long long size = 0; unsigned long long size = 0;
// FIXME auto storePath = maybeParseStorePath(path);
if (isStorePath(path) && isValidPath(parseStorePath(path))) { if (storePath && isValidPath(*storePath)) {
StorePathSet referrers; StorePathSet referrers;
queryReferrers(parseStorePath(path), referrers); queryReferrers(*storePath, referrers);
for (auto & i : referrers) for (auto & i : referrers)
if (printStorePath(i) != path) deletePathRecursive(state, printStorePath(i)); if (printStorePath(i) != path) deletePathRecursive(state, printStorePath(i));
size = queryPathInfo(parseStorePath(path))->narSize; size = queryPathInfo(*storePath)->narSize;
invalidatePathChecked(parseStorePath(path)); invalidatePathChecked(*storePath);
} }
Path realPath = realStoreDir + "/" + std::string(baseNameOf(path)); Path realPath = realStoreDir + "/" + std::string(baseNameOf(path));
@ -593,8 +591,7 @@ bool LocalStore::canReachRoot(GCState & state, StorePathSet & visited, const Sto
visited.insert(path.clone()); visited.insert(path.clone());
//FIXME if (!isValidPath(path)) return false;
if (!isStorePath(printStorePath(path)) || !isValidPath(path)) return false;
StorePathSet incoming; StorePathSet incoming;
@ -637,8 +634,9 @@ void LocalStore::tryToDelete(GCState & state, const Path & path)
//Activity act(*logger, lvlDebug, format("considering whether to delete '%1%'") % path); //Activity act(*logger, lvlDebug, format("considering whether to delete '%1%'") % path);
// FIXME auto storePath = maybeParseStorePath(path);
if (!isStorePath(path) || !isValidPath(parseStorePath(path))) {
if (!storePath || !isValidPath(*storePath)) {
/* A lock file belonging to a path that we're building right /* A lock file belonging to a path that we're building right
now isn't garbage. */ now isn't garbage. */
if (isActiveTempFile(state, path, ".lock")) return; if (isActiveTempFile(state, path, ".lock")) return;
@ -655,7 +653,7 @@ void LocalStore::tryToDelete(GCState & state, const Path & path)
StorePathSet visited; StorePathSet visited;
if (canReachRoot(state, visited, parseStorePath(path))) { if (storePath && canReachRoot(state, visited, *storePath)) {
debug("cannot delete '%s' because it's still reachable", path); debug("cannot delete '%s' because it's still reachable", path);
} else { } else {
/* No path we visited was a root, so everything is garbage. /* No path we visited was a root, so everything is garbage.
@ -818,8 +816,8 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
string name = dirent->d_name; string name = dirent->d_name;
if (name == "." || name == "..") continue; if (name == "." || name == "..") continue;
Path path = storeDir + "/" + name; Path path = storeDir + "/" + name;
// FIXME auto storePath = maybeParseStorePath(path);
if (isStorePath(path) && isValidPath(parseStorePath(path))) if (storePath && isValidPath(*storePath))
entries.push_back(path); entries.push_back(path);
else else
tryToDelete(state, path); tryToDelete(state, path);

View file

@ -55,6 +55,20 @@ StorePath Store::parseStorePath(std::string_view path) const
return StorePath::make(path, storeDir); return StorePath::make(path, storeDir);
} }
std::optional<StorePath> Store::maybeParseStorePath(std::string_view path) const
{
try {
return parseStorePath(path);
} catch (Error &) {
return {};
}
}
bool Store::isStorePath(const Path & path) const
{
return (bool) maybeParseStorePath(path);
}
StorePathSet Store::parseStorePathSet(const PathSet & paths) const StorePathSet Store::parseStorePathSet(const PathSet & paths) const
{ {
StorePathSet res; StorePathSet res;

View file

@ -19,14 +19,6 @@ bool Store::isInStore(const Path & path) const
} }
bool Store::isStorePath(const Path & path) const
{
return isInStore(path)
&& path.size() >= storeDir.size() + 1 + storePathHashLen
&& path.find('/', storeDir.size() + 1) == Path::npos;
}
Path Store::toStorePath(const Path & path) const Path Store::toStorePath(const Path & path) const
{ {
if (!isInStore(path)) if (!isInStore(path))

View file

@ -281,6 +281,8 @@ public:
StorePath parseStorePath(std::string_view path) const; StorePath parseStorePath(std::string_view path) const;
std::optional<StorePath> maybeParseStorePath(std::string_view path) const;
std::string printStorePath(const StorePath & path) const; std::string printStorePath(const StorePath & path) const;
// FIXME: remove // FIXME: remove