libexpr: Deprecate NUL bytes in strings

I absolutely want to go back to allowing them, I am strongly of the
opinion that nothing should be special about NUL in a string, but this
will have to wait until at least the GC rewrite, so for now the least I
can do is to disarm this gun.

Change-Id: Id14b6037bc8b274c6c60ad970b1c74d436fb62a7
This commit is contained in:
piegames
2025-02-28 16:17:17 +01:00
parent e38ad66ef7
commit 65ce95d381
12 changed files with 62 additions and 15 deletions
@@ -10,3 +10,4 @@ This release cycle features a new batch of deprecated (anti-)features.
You can opt in into the old behavior with `--extra-deprecated-features` or any equivalent configuration option.
- `cr-line-endings`: Current handling of CR (`\r`) or CRLF (`\r\n`) line endings in Nix is inconsistent and broken, and will lead to unexpected evaluation results with certain strings. Given that fixing the semantics might silently alter the evaluation result of derivations, the only option at the moment is to disallow them alltogether. More proper support for CRLF is planned to be added back again in the future. Until then, all files must use `\n` exclusively.
- `nul-bytes`: Currently the Nix grammar allows NUL bytes (`\0`) in strings, and thus indirectly also in identifiers. Unfortunately, several core parts of the code base still work with NUL-terminated strings and cannot easily be migrated. Also note that it is still possible to introduce NUL bytes and thus problematic behavior via other means, those are tracked separately.
+6 -4
View File
@@ -249,6 +249,7 @@ struct _ind_string {
/* Marker for non-empty lines */
struct has_content : semantic, seq<> {};
struct cr : semantic, one<'\r'> {};
struct nul : semantic, one<'\0'> {};
};
struct ind_string : _ind_string, seq<
TAO_PEGTL_STRING("''"),
@@ -266,13 +267,14 @@ struct ind_string : _ind_string, seq<
_ind_string::literal<
plus<
sor<
not_one<'$', '\'', '\n', '\r'>,
not_one<'$', '\'', '\n', '\r', '\0'>,
// TODO probably factor this out like the others for performance
seq<one<'$'>, not_one<'{', '\'', '\n', '\r'>>,
seq<one<'$'>, not_one<'{', '\'', '\n', '\r', '\0'>>,
seq<one<'$'>, at<one<'\n'>>>,
seq<one<'\''>, not_one<'\'', '$', '\n', '\r'>>,
seq<one<'\''>, not_one<'\'', '$', '\n', '\r', '\0'>>,
seq<one<'\''>, at<one<'\n'>>>,
_ind_string::cr
_ind_string::cr,
_ind_string::nul
>
>
>,
+28 -11
View File
@@ -508,7 +508,7 @@ struct StringState : SubexprState {
// FIXME this truncates strings on NUL for compat with the old parser. ideally
// we should use the decomposition the g gives us instead of iterating over
// the entire string again.
static void unescapeStr(std::string & str)
void unescapeStr(std::string & str, State & ps)
{
char * s = str.data();
char * t = s;
@@ -529,24 +529,26 @@ struct StringState : SubexprState {
else *t = c;
t++;
}
if (!ps.featureSettings.isEnabled(Dep::NulBytes) && size_t(s - str.data() - 1) != str.size())
ps.nulFound(currentPos);
str.resize(t - str.data());
}
void endLiteral()
void endLiteral(State & ps)
{
if (!currentLiteral.empty()) {
unescapeStr(currentLiteral);
unescapeStr(currentLiteral, ps);
parts.emplace_back(currentPos, std::make_unique<ExprString>(currentPos, std::move(currentLiteral)));
}
}
std::unique_ptr<Expr> finish()
std::unique_ptr<Expr> finish(State & ps)
{
if (parts.empty()) {
unescapeStr(currentLiteral);
unescapeStr(currentLiteral, ps);
return std::make_unique<ExprString>(currentPos, std::move(currentLiteral));
} else {
endLiteral();
endLiteral(ps);
auto pos = parts[0].first;
return std::make_unique<ExprConcatStrings>(pos, true, std::move(parts));
}
@@ -570,21 +572,23 @@ template<> struct BuildAST<grammar::v1::string::cr_crlf> {
template<> struct BuildAST<grammar::v1::string::interpolation> {
static void apply(const auto & in, StringState & s, State & ps) {
s.endLiteral();
s.endLiteral(ps);
s.parts.emplace_back(ps.at(in), s->popExprOnly());
}
};
template<> struct BuildAST<grammar::v1::string::escape> {
static void apply(const auto & in, StringState & s, State & ps) {
if (!ps.featureSettings.isEnabled(Dep::NulBytes) && *in.begin() == '\0')
ps.nulFound(ps.at(in));
s.append(ps.at(in), "\\"); // FIXME compat with old parser
s.append(ps.at(in), in.string_view());
}
};
template<> struct BuildAST<grammar::v1::string> : change_head<StringState> {
static void success0(StringState & s, ExprState & e, State &) {
e.pushExpr(noPos, s.finish());
static void success0(StringState & s, ExprState & e, State & ps) {
e.pushExpr(noPos, s.finish(ps));
}
};
@@ -619,6 +623,12 @@ template<> struct BuildAST<grammar::v1::ind_string::escape> {
case 'n': s.lines.back().parts.emplace_back(ps.at(in), "\n"); break;
case 'r': s.lines.back().parts.emplace_back(ps.at(in), "\r"); break;
case 't': s.lines.back().parts.emplace_back(ps.at(in), "\t"); break;
case 0:
if (!ps.featureSettings.isEnabled(Dep::NulBytes)) {
ps.nulFound(ps.at(in));
break;
}
KJ_FALLTHROUGH;
default: s.lines.back().parts.emplace_back(ps.at(in), in.string_view()); break;
}
}
@@ -637,6 +647,13 @@ template<> struct BuildAST<grammar::v1::ind_string::cr> {
}
};
template<> struct BuildAST<grammar::v1::ind_string::nul> {
static void apply(const auto & in, IndStringState & s, State & ps) {
if (!ps.featureSettings.isEnabled(Dep::NulBytes))
ps.nulFound(ps.at(in));
}
};
template<> struct BuildAST<grammar::v1::ind_string> : change_head<IndStringState> {
static void success(const auto & in, IndStringState & s, ExprState & e, State & ps) {
e.pushExpr(noPos, ps.stripIndentation(ps.at(in), std::move(s.lines)));
@@ -646,7 +663,7 @@ template<> struct BuildAST<grammar::v1::ind_string> : change_head<IndStringState
template<typename... Content> struct BuildAST<grammar::v1::path::literal<Content...>> {
static void apply(const auto & in, StringState & s, State & ps) {
s.append(ps.at(in), in.string_view());
s.endLiteral();
s.endLiteral(ps);
}
};
@@ -708,7 +725,7 @@ template<> struct BuildAST<grammar::v1::path> : change_head<StringState> {
}
static void success(const auto & in, StringState & s, ExprState & e, State & ps) {
s.endLiteral();
s.endLiteral(ps);
check_slash<ExprPath>(ps.atEnd(in), s, ps);
check_slash<ExprString>(ps.atEnd(in), s, ps);
if (s.parts.size() == 1) {
+12
View File
@@ -37,6 +37,7 @@ struct State
void dupAttr(Symbol attr, const PosIdx pos, const PosIdx prevPos);
void overridesFound(const PosIdx pos);
void badLineEndingFound(const PosIdx pos, bool warnOnly);
void nulFound(const PosIdx pos);
void addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ptr<Expr> e, const PosIdx pos);
void validateLambdaAttrs(AttrsPattern & pattern, PosIdx pos = noPos);
std::unique_ptr<Expr> stripIndentation(const PosIdx pos, std::vector<IndStringLine> && line);
@@ -114,6 +115,17 @@ inline void State::badLineEndingFound(const PosIdx pos, bool warnOnly)
.pos = positions[pos],
});
}
// Added 2025-02-05.
inline void State::nulFound(const PosIdx pos)
{
throw ParseError({
.msg = HintFmt(
"NUL bytes (`\\0`) are currently not well supported, because internally strings are NUL-terminated, which may lead to unexpected truncation. Use %s to disable this error.",
"--extra-deprecated-features nul-bytes"
),
.pos = positions[pos],
});
}
inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ptr<Expr> e, const PosIdx pos)
{
@@ -0,0 +1,6 @@
---
name: nul-bytes
internalName: NulBytes
---
Allow NUL bytes (`\0`) in Nix strings.
Note however that due to Nix using NUL-terminated strings internally, this may cause undefined behavior.
+1
View File
@@ -156,6 +156,7 @@ experimental_feature_definitions = files(
deprecated_feature_definitions = files(
'deprecated-features/ancient-let.md',
'deprecated-features/cr-line-endings.md',
'deprecated-features/nul-bytes.md',
'deprecated-features/rec-set-overrides.md',
'deprecated-features/url-literals.md',
'deprecated-features/shadow-internal-symbols.md',
+1
View File
@@ -0,0 +1 @@
"foo"
@@ -0,0 +1 @@
--extra-deprecated-features nul-bytes
Binary file not shown.
@@ -0,0 +1 @@
foo
@@ -0,0 +1,4 @@
error: NUL bytes (`\0`) are currently not well supported, because internally strings are NUL-terminated, which may lead to unexpected truncation. Use --extra-deprecated-features nul-bytes to disable this error.
at «stdin»:1:2:
1| "foo
| ^
+1
View File
@@ -0,0 +1 @@
eval-okay-nul.nix