From b72bc4a972fe568744d98b89d63adcd504cb586c Mon Sep 17 00:00:00 2001 From: Alex Ameen Date: Tue, 9 May 2023 09:45:12 -0500 Subject: [PATCH 1/3] libexpr: quote reserved keys when printing This fixes a bug in commands like `nix eval' which would emit invalid attribute sets if they contained reserved keywords such as "assert", "let", etc. These keywords will not be quoted when printed, making them valid expressions. All keywords recognized by the lexer are quoted except "or", which does not require quotation. --- src/libexpr/eval.cc | 9 +++++++-- src/libexpr/print.cc | 20 ++++++++++++++++++-- src/libexpr/print.hh | 6 ++++++ tests/eval.sh | 2 ++ 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index e2b455b91..af9d037ec 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -94,7 +94,6 @@ RootValue allocRootValue(Value * v) #endif } - void Value::print(const SymbolTable & symbols, std::ostream & str, std::set * seen) const { @@ -122,7 +121,13 @@ void Value::print(const SymbolTable & symbols, std::ostream & str, else { str << "{ "; for (auto & i : attrs->lexicographicOrder(symbols)) { - str << symbols[i->name] << " = "; + // Quote reserved keywords so that the output can be + // re-evaluated later without upsetting the lexer. + if (isReservedKeyword(symbols[i->name])) { + str << "\"" << symbols[i->name] << "\" = "; + } else { + str << symbols[i->name] << " = "; + } i->value->print(symbols, str, seen); str << "; "; } diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index d08672cfc..53ba70bdd 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -1,4 +1,5 @@ #include "print.hh" +#include namespace nix { @@ -25,11 +26,26 @@ printLiteralBool(std::ostream & str, bool boolean) return str; } +// Returns `true' is a string is a reserved keyword which requires quotation +// when printing attribute set field names. +// +// This list should generally be kept in sync with `./lexer.l'. +// You can test if a keyword needs to be added by running: +// $ nix eval --expr '{ = 1; }' +// For example `or' doesn't need to be quoted. +bool isReservedKeyword(const std::string_view str) +{ + static const std::unordered_set reservedKeywords = { + "if", "then", "else", "assert", "with", "let", "in", "rec", "inherit" + }; + return reservedKeywords.contains(str); +} + std::ostream & printIdentifier(std::ostream & str, std::string_view s) { if (s.empty()) str << "\"\""; - else if (s == "if") // FIXME: handle other keywords + else if (isReservedKeyword(s)) str << '"' << s << '"'; else { char c = s[0]; @@ -50,10 +66,10 @@ printIdentifier(std::ostream & str, std::string_view s) { return str; } -// FIXME: keywords static bool isVarName(std::string_view s) { if (s.size() == 0) return false; + if (isReservedKeyword(s)) return false; char c = s[0]; if ((c >= '0' && c <= '9') || c == '-' || c == '\'') return false; for (auto & i : s) diff --git a/src/libexpr/print.hh b/src/libexpr/print.hh index f9cfc3964..3b72ae201 100644 --- a/src/libexpr/print.hh +++ b/src/libexpr/print.hh @@ -35,6 +35,12 @@ namespace nix { */ std::ostream & printAttributeName(std::ostream & o, std::string_view s); + /** + * Returns `true' is a string is a reserved keyword which requires quotation + * when printing attribute set field names. + */ + bool isReservedKeyword(const std::string_view str); + /** * Print a string as an identifier in the Nix expression language syntax. * diff --git a/tests/eval.sh b/tests/eval.sh index ffae08a6a..8e694c327 100644 --- a/tests/eval.sh +++ b/tests/eval.sh @@ -19,6 +19,7 @@ nix eval --expr 'assert 1 + 2 == 3; true' [[ $(nix eval attr -f "./eval.nix") == '{ foo = "bar"; }' ]] [[ $(nix eval attr --json -f "./eval.nix") == '{"foo":"bar"}' ]] [[ $(nix eval int -f - < "./eval.nix") == 123 ]] +[[ $(nix eval --expr '{"assert"=1;bar=2;}') == '{ "assert" = 1; bar = 2; }' ]] # Check if toFile can be utilized during restricted eval [[ $(nix eval --restrict-eval --expr 'import (builtins.toFile "source" "42")') == 42 ]] @@ -29,6 +30,7 @@ nix-instantiate --eval -E 'assert 1 + 2 == 3; true' [[ $(nix-instantiate -A attr --eval "./eval.nix") == '{ foo = "bar"; }' ]] [[ $(nix-instantiate -A attr --eval --json "./eval.nix") == '{"foo":"bar"}' ]] [[ $(nix-instantiate -A int --eval - < "./eval.nix") == 123 ]] +[[ $(nix-instantiate --eval -E '{"assert"=1;bar=2;}') == '{ "assert" = 1; bar = 2; }' ]] # Check that symlink cycles don't cause a hang. ln -sfn cycle.nix $TEST_ROOT/cycle.nix From 82296f81130aacb774fba4683e2e655e51c0a210 Mon Sep 17 00:00:00 2001 From: Alex Ameen Date: Tue, 9 May 2023 09:59:18 -0500 Subject: [PATCH 2/3] prevent double quotation --- src/libexpr/eval.cc | 8 +------- tests/eval.sh | 4 ++-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index af9d037ec..0b4243670 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -121,13 +121,7 @@ void Value::print(const SymbolTable & symbols, std::ostream & str, else { str << "{ "; for (auto & i : attrs->lexicographicOrder(symbols)) { - // Quote reserved keywords so that the output can be - // re-evaluated later without upsetting the lexer. - if (isReservedKeyword(symbols[i->name])) { - str << "\"" << symbols[i->name] << "\" = "; - } else { - str << symbols[i->name] << " = "; - } + str << symbols[i->name] << " = "; i->value->print(symbols, str, seen); str << "; "; } diff --git a/tests/eval.sh b/tests/eval.sh index 8e694c327..0c1b2cf3a 100644 --- a/tests/eval.sh +++ b/tests/eval.sh @@ -19,7 +19,7 @@ nix eval --expr 'assert 1 + 2 == 3; true' [[ $(nix eval attr -f "./eval.nix") == '{ foo = "bar"; }' ]] [[ $(nix eval attr --json -f "./eval.nix") == '{"foo":"bar"}' ]] [[ $(nix eval int -f - < "./eval.nix") == 123 ]] -[[ $(nix eval --expr '{"assert"=1;bar=2;}') == '{ "assert" = 1; bar = 2; }' ]] +[[ "$(nix eval --expr '{"assert"=1;bar=2;}')" == '{ "assert" = 1; bar = 2; }' ]] # Check if toFile can be utilized during restricted eval [[ $(nix eval --restrict-eval --expr 'import (builtins.toFile "source" "42")') == 42 ]] @@ -30,7 +30,7 @@ nix-instantiate --eval -E 'assert 1 + 2 == 3; true' [[ $(nix-instantiate -A attr --eval "./eval.nix") == '{ foo = "bar"; }' ]] [[ $(nix-instantiate -A attr --eval --json "./eval.nix") == '{"foo":"bar"}' ]] [[ $(nix-instantiate -A int --eval - < "./eval.nix") == 123 ]] -[[ $(nix-instantiate --eval -E '{"assert"=1;bar=2;}') == '{ "assert" = 1; bar = 2; }' ]] +[[ "$(nix-instantiate --eval -E '{"assert"=1;bar=2;}')" == '{ "assert" = 1; bar = 2; }' ]] # Check that symlink cycles don't cause a hang. ln -sfn cycle.nix $TEST_ROOT/cycle.nix From 82d1d74a85bc3d06ff3b17e735deedd98eb47b66 Mon Sep 17 00:00:00 2001 From: Alex Ameen Date: Tue, 9 May 2023 10:06:26 -0500 Subject: [PATCH 3/3] quote subshell expansion in tests/eval.sh --- tests/eval.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/eval.sh b/tests/eval.sh index 0c1b2cf3a..066d8fc36 100644 --- a/tests/eval.sh +++ b/tests/eval.sh @@ -16,7 +16,7 @@ nix eval --expr 'assert 1 + 2 == 3; true' [[ $(nix eval int -f "./eval.nix") == 123 ]] [[ $(nix eval str -f "./eval.nix") == '"foo"' ]] [[ $(nix eval str --raw -f "./eval.nix") == 'foo' ]] -[[ $(nix eval attr -f "./eval.nix") == '{ foo = "bar"; }' ]] +[[ "$(nix eval attr -f "./eval.nix")" == '{ foo = "bar"; }' ]] [[ $(nix eval attr --json -f "./eval.nix") == '{"foo":"bar"}' ]] [[ $(nix eval int -f - < "./eval.nix") == 123 ]] [[ "$(nix eval --expr '{"assert"=1;bar=2;}')" == '{ "assert" = 1; bar = 2; }' ]] @@ -27,7 +27,7 @@ nix eval --expr 'assert 1 + 2 == 3; true' nix-instantiate --eval -E 'assert 1 + 2 == 3; true' [[ $(nix-instantiate -A int --eval "./eval.nix") == 123 ]] [[ $(nix-instantiate -A str --eval "./eval.nix") == '"foo"' ]] -[[ $(nix-instantiate -A attr --eval "./eval.nix") == '{ foo = "bar"; }' ]] +[[ "$(nix-instantiate -A attr --eval "./eval.nix")" == '{ foo = "bar"; }' ]] [[ $(nix-instantiate -A attr --eval --json "./eval.nix") == '{"foo":"bar"}' ]] [[ $(nix-instantiate -A int --eval - < "./eval.nix") == 123 ]] [[ "$(nix-instantiate --eval -E '{"assert"=1;bar=2;}')" == '{ "assert" = 1; bar = 2; }' ]]