Merge pull request #9867 from hercules-ci/issue-912

#912 allow leading period
This commit is contained in:
Robert Hensing 2024-01-31 19:10:59 +01:00 committed by GitHub
commit 4072a8fea0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 146 additions and 47 deletions

View file

@ -0,0 +1,10 @@
---
synopsis: Store paths are allowed to start with `.`
issues: 912
prs: 9867 9091 9095 9120 9121 9122 9130 9219 9224
---
Leading periods were allowed by accident in Nix 2.4. The Nix team has considered this to be a bug, but this behavior has since been relied on by users, leading to unnecessary difficulties.
From now on, leading periods are officially, definitively supported. The names `.` and `..` are disallowed, as well as those starting with `.-` or `..-`.
Nix versions that denied leading periods are documented [in the issue](https://github.com/NixOS/nix/issues/912#issuecomment-1919583286).

View file

@ -3,6 +3,11 @@
namespace nix { namespace nix {
static constexpr std::string_view nameRegexStr = R"([0-9a-zA-Z\+\-_\?=][0-9a-zA-Z\+\-\._\?=]*)";
static constexpr std::string_view nameRegexStr =
// This uses a negative lookahead: (?!\.\.?(-|$))
// - deny ".", "..", or those strings followed by '-'
// - when it's not those, start again at the start of the input and apply the next regex, which is [0-9a-zA-Z\+\-\._\?=]+
R"((?!\.\.?(-|$))[0-9a-zA-Z\+\-\._\?=]+)";
} }

View file

@ -9,9 +9,20 @@ static void checkName(std::string_view path, std::string_view name)
if (name.size() > StorePath::MaxPathLen) if (name.size() > StorePath::MaxPathLen)
throw BadStorePath("store path '%s' has a name longer than %d characters", throw BadStorePath("store path '%s' has a name longer than %d characters",
path, StorePath::MaxPathLen); path, StorePath::MaxPathLen);
if (name[0] == '.')
throw BadStorePath("store path '%s' starts with illegal character '.'", path);
// See nameRegexStr for the definition // See nameRegexStr for the definition
if (name[0] == '.') {
// check against "." and "..", followed by end or dash
if (name.size() == 1)
throw BadStorePath("store path '%s' has invalid name '%s'", path, name);
if (name[1] == '-')
throw BadStorePath("store path '%s' has invalid name '%s': first dash-separated component must not be '%s'", path, name, ".");
if (name[1] == '.') {
if (name.size() == 2)
throw BadStorePath("store path '%s' has invalid name '%s'", path, name);
if (name[2] == '-')
throw BadStorePath("store path '%s' has invalid name '%s': first dash-separated component must not be '%s'", path, name, "..");
}
}
for (auto c : name) for (auto c : name)
if (!((c >= '0' && c <= '9') if (!((c >= '0' && c <= '9')
|| (c >= 'a' && c <= 'z') || (c >= 'a' && c <= 'z')

View file

@ -1,3 +1,4 @@
#include <rapidcheck/gen/Arbitrary.h>
#include <regex> #include <regex>
#include <rapidcheck.h> #include <rapidcheck.h>
@ -20,63 +21,60 @@ void showValue(const StorePath & p, std::ostream & os)
namespace rc { namespace rc {
using namespace nix; using namespace nix;
Gen<StorePathName> Arbitrary<StorePathName>::arbitrary() Gen<char> storePathChar()
{ {
auto len = *gen::inRange<size_t>( return rc::gen::apply([](uint8_t i) -> char {
1, switch (i) {
StorePath::MaxPathLen - StorePath::HashLen);
std::string pre;
pre.reserve(len);
for (size_t c = 0; c < len; ++c) {
switch (auto i = *gen::inRange<uint8_t>(0, 10 + 2 * 26 + 6)) {
case 0 ... 9: case 0 ... 9:
pre += '0' + i; return '0' + i;
case 10 ... 35: case 10 ... 35:
pre += 'A' + (i - 10); return 'A' + (i - 10);
break;
case 36 ... 61: case 36 ... 61:
pre += 'a' + (i - 36); return 'a' + (i - 36);
break;
case 62: case 62:
pre += '+'; return '+';
break;
case 63: case 63:
pre += '-'; return '-';
break;
case 64: case 64:
// names aren't permitted to start with a period, return '.';
// so just fall through to the next case here
if (c != 0) {
pre += '.';
break;
}
case 65: case 65:
pre += '_'; return '_';
break;
case 66: case 66:
pre += '?'; return '?';
break;
case 67: case 67:
pre += '='; return '=';
break;
default: default:
assert(false); assert(false);
} }
},
gen::inRange<uint8_t>(0, 10 + 2 * 26 + 6));
} }
return gen::just(StorePathName { Gen<StorePathName> Arbitrary<StorePathName>::arbitrary()
.name = std::move(pre), {
}); return gen::construct<StorePathName>(
gen::suchThat(
gen::container<std::string>(storePathChar()),
[](const std::string & s) {
return
!( s == ""
|| s == "."
|| s == ".."
|| s.starts_with(".-")
|| s.starts_with("..-")
);
}
)
);
} }
Gen<StorePath> Arbitrary<StorePath>::arbitrary() Gen<StorePath> Arbitrary<StorePath>::arbitrary()
{ {
return gen::just(StorePath { return
*gen::arbitrary<Hash>(), gen::construct<StorePath>(
(*gen::arbitrary<StorePathName>()).name, gen::arbitrary<Hash>(),
}); gen::apply([](StorePathName n){ return n.name; }, gen::arbitrary<StorePathName>())
);
} }
} // namespace rc } // namespace rc

View file

@ -39,7 +39,12 @@ TEST_DONT_PARSE(double_star, "**")
TEST_DONT_PARSE(star_first, "*,foo") TEST_DONT_PARSE(star_first, "*,foo")
TEST_DONT_PARSE(star_second, "foo,*") TEST_DONT_PARSE(star_second, "foo,*")
TEST_DONT_PARSE(bang, "foo!o") TEST_DONT_PARSE(bang, "foo!o")
TEST_DONT_PARSE(dotfile, ".gitignore") TEST_DONT_PARSE(dot, ".")
TEST_DONT_PARSE(dot_dot, "..")
TEST_DONT_PARSE(dot_dot_dash, "..-1")
TEST_DONT_PARSE(dot_dash, ".-1")
TEST_DONT_PARSE(dot_dot_dash_a, "..-a")
TEST_DONT_PARSE(dot_dash_a, ".-a")
#undef TEST_DONT_PARSE #undef TEST_DONT_PARSE
@ -63,6 +68,11 @@ TEST_DO_PARSE(underscore, "foo_bar")
TEST_DO_PARSE(period, "foo.txt") TEST_DO_PARSE(period, "foo.txt")
TEST_DO_PARSE(question_mark, "foo?why") TEST_DO_PARSE(question_mark, "foo?why")
TEST_DO_PARSE(equals_sign, "foo=foo") TEST_DO_PARSE(equals_sign, "foo=foo")
TEST_DO_PARSE(dotfile, ".gitignore")
TEST_DO_PARSE(triple_dot_a, "...a")
TEST_DO_PARSE(triple_dot_1, "...1")
TEST_DO_PARSE(triple_dot_dash, "...-")
TEST_DO_PARSE(triple_dot, "...")
#undef TEST_DO_PARSE #undef TEST_DO_PARSE
@ -84,6 +94,64 @@ RC_GTEST_FIXTURE_PROP(
RC_ASSERT(p == store->parseStorePath(store->printStorePath(p))); RC_ASSERT(p == store->parseStorePath(store->printStorePath(p)));
} }
RC_GTEST_FIXTURE_PROP(
StorePathTest,
prop_check_regex_eq_parse,
())
{
static auto nameFuzzer =
rc::gen::container<std::string>(
rc::gen::oneOf(
// alphanum, repeated to weigh heavier
rc::gen::oneOf(
rc::gen::inRange('0', '9'),
rc::gen::inRange('a', 'z'),
rc::gen::inRange('A', 'Z')
),
// valid symbols
rc::gen::oneOf(
rc::gen::just('+'),
rc::gen::just('-'),
rc::gen::just('.'),
rc::gen::just('_'),
rc::gen::just('?'),
rc::gen::just('=')
),
// symbols for scary .- and ..- cases, repeated for weight
rc::gen::just('.'), rc::gen::just('.'),
rc::gen::just('.'), rc::gen::just('.'),
rc::gen::just('-'), rc::gen::just('-'),
// ascii symbol ranges
rc::gen::oneOf(
rc::gen::inRange(' ', '/'),
rc::gen::inRange(':', '@'),
rc::gen::inRange('[', '`'),
rc::gen::inRange('{', '~')
),
// typical whitespace
rc::gen::oneOf(
rc::gen::just(' '),
rc::gen::just('\t'),
rc::gen::just('\n'),
rc::gen::just('\r')
),
// some chance of control codes, non-ascii or other garbage we missed
rc::gen::inRange('\0', '\xff')
));
auto name = *nameFuzzer;
std::string path = store->storeDir + "/575s52sh487i0ylmbs9pvi606ljdszr0-" + name;
bool parsed = false;
try {
store->parseStorePath(path);
parsed = true;
} catch (const BadStorePath &) {
}
RC_ASSERT(parsed == std::regex_match(std::string { name }, nameRegex));
}
#endif #endif
} }

View file

@ -11,10 +11,17 @@ using namespace nix;
Gen<Hash> Arbitrary<Hash>::arbitrary() Gen<Hash> Arbitrary<Hash>::arbitrary()
{ {
Hash prototype(HashAlgorithm::SHA1);
return
gen::apply(
[](const std::vector<uint8_t> & v) {
Hash hash(HashAlgorithm::SHA1); Hash hash(HashAlgorithm::SHA1);
for (size_t i = 0; i < hash.hashSize; ++i) assert(v.size() == hash.hashSize);
hash.hash[i] = *gen::arbitrary<uint8_t>(); std::copy(v.begin(), v.end(), hash.hash);
return gen::just(hash); return hash;
},
gen::container<std::vector<uint8_t>>(prototype.hashSize, gen::arbitrary<uint8_t>())
);
} }
} }