about summary refs log tree commit diff
diff options
context:
space:
mode:
authorVincent Ambo <mail@tazj.in>2022-08-25T11·23+0300
committertazjin <tazjin@tvl.su>2022-09-03T00·47+0000
commit39b01c302937fe3e91c15758b520c3ecc5379c7b (patch)
treecff95e288671533056fcea8a62e3fe620b77ef6d
parent265393301e2b59659a0a5db15e42bc668d0c7c68 (diff)
fix(tvix/eval): correctly escape `${` in strings r/4607
Without this escape, it is possible for Nix to produce escaped
representations which are not literal Nix values again.

This was fixed in upstream Nix in
https://github.com/NixOS/nix/pull/4012 (though only for eval, not in
the REPL) and the updated test is picked from upstream after that commit.

Because we run the C++ Nix tests against our test suite as well, this
also bumps our custom Nix 2.3 to a commit that includes the
cherry-picked fix from the PR above.

Change-Id: I478547ade65f655c606ec46f7143932064192283
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6271
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
-rw-r--r--third_party/overlays/tvl.nix4
-rw-r--r--tvix/eval/src/tests/nix_tests/eval-okay-ind-string.exp2
-rw-r--r--tvix/eval/src/tests/tvix_tests/identity-dollar-escape.nix1
-rw-r--r--tvix/eval/src/value/string.rs26
4 files changed, 19 insertions, 14 deletions
diff --git a/third_party/overlays/tvl.nix b/third_party/overlays/tvl.nix
index 9bb88dc2bed9..412a605aa9e9 100644
--- a/third_party/overlays/tvl.nix
+++ b/third_party/overlays/tvl.nix
@@ -9,14 +9,14 @@ let
   nixSrc =
     let
       # branch 2.3-backport-await-users
-      rev = "4510dbc8a6802902cbab6444134659548fffb9b0";
+      rev = "abdc60f49f1104696bac723331d3ed0296d5a784";
     in
     self.fetchFromGitHub
       {
         owner = "tvlfyi";
         repo = "nix";
         inherit rev;
-        hash = "sha256:0vg2xzwc8q1sw20b26qbyd4flnws8668yhi1cg2h6z3jb3wamhr5";
+        hash = "sha256:0c1pmg8y0yafdkliz970k52s92z3qin3xrz3g0n2ss7xcfbg8nzy";
       } // { revCount = 0; shortRev = builtins.substring 0 7 rev; };
 in
 {
diff --git a/tvix/eval/src/tests/nix_tests/eval-okay-ind-string.exp b/tvix/eval/src/tests/nix_tests/eval-okay-ind-string.exp
index 9cf4bd2ee78a..7862331fa551 100644
--- a/tvix/eval/src/tests/nix_tests/eval-okay-ind-string.exp
+++ b/tvix/eval/src/tests/nix_tests/eval-okay-ind-string.exp
@@ -1 +1 @@
-"This is an indented multi-line string\nliteral.  An amount of whitespace at\nthe start of each line matching the minimum\nindentation of all lines in the string\nliteral together will be removed.  Thus,\nin this case four spaces will be\nstripped from each line, even though\n  THIS LINE is indented six spaces.\n\nAlso, empty lines don't count in the\ndetermination of the indentation level (the\nprevious empty line has indentation 0, but\nit doesn't matter).\nIf the string starts with whitespace\n  followed by a newline, it's stripped, but\n  that's not the case here. Two spaces are\n  stripped because of the \"  \" at the start. \nThis line is indented\na bit further.\nAnti-quotations, like so, are\nalso allowed.\n  The \\ is not special here.\n' can be followed by any character except another ', e.g. 'x'.\nLikewise for $, e.g. $$ or $varName.\nBut ' followed by ' is special, as is $ followed by {.\nIf you want them, use anti-quotations: '', ${.\n   Tabs are not interpreted as whitespace (since we can't guess\n   what tab settings are intended), so don't use them.\n\tThis line starts with a space and a tab, so only one\n   space will be stripped from each line.\nAlso note that if the last line (just before the closing ' ')\nconsists only of whitespace, it's ignored.  But here there is\nsome non-whitespace stuff, so the line isn't removed. \nThis shows a hacky way to preserve an empty line after the start.\nBut there's no reason to do so: you could just repeat the empty\nline.\n  Similarly you can force an indentation level,\n  in this case to 2 spaces.  This works because the anti-quote\n  is significant (not whitespace).\nstart on network-interfaces\n\nstart script\n\n  rm -f /var/run/opengl-driver\n  ln -sf 123 /var/run/opengl-driver\n\n  rm -f /var/log/slim.log\n   \nend script\n\nenv SLIM_CFGFILE=abc\nenv SLIM_THEMESDIR=def\nenv FONTCONFIG_FILE=/etc/fonts/fonts.conf  \t\t\t\t# !!! cleanup\nenv XKB_BINDIR=foo/bin         \t\t\t\t# Needed for the Xkb extension.\nenv LD_LIBRARY_PATH=libX11/lib:libXext/lib:/usr/lib/          # related to xorg-sys-opengl - needed to load libglx for (AI)GLX support (for compiz)\n\nenv XORG_DRI_DRIVER_PATH=nvidiaDrivers/X11R6/lib/modules/drivers/ \n\nexec slim/bin/slim\nEscaping of ' followed by ': ''\nEscaping of $ followed by {: ${\nAnd finally to interpret \\n etc. as in a string: \n, \r, \t.\nfoo\n'bla'\nbar\ncut -d $'\\t' -f 1\nending dollar $$\n"
+"This is an indented multi-line string\nliteral.  An amount of whitespace at\nthe start of each line matching the minimum\nindentation of all lines in the string\nliteral together will be removed.  Thus,\nin this case four spaces will be\nstripped from each line, even though\n  THIS LINE is indented six spaces.\n\nAlso, empty lines don't count in the\ndetermination of the indentation level (the\nprevious empty line has indentation 0, but\nit doesn't matter).\nIf the string starts with whitespace\n  followed by a newline, it's stripped, but\n  that's not the case here. Two spaces are\n  stripped because of the \"  \" at the start. \nThis line is indented\na bit further.\nAnti-quotations, like so, are\nalso allowed.\n  The \\ is not special here.\n' can be followed by any character except another ', e.g. 'x'.\nLikewise for $, e.g. $$ or $varName.\nBut ' followed by ' is special, as is $ followed by {.\nIf you want them, use anti-quotations: '', \${.\n   Tabs are not interpreted as whitespace (since we can't guess\n   what tab settings are intended), so don't use them.\n\tThis line starts with a space and a tab, so only one\n   space will be stripped from each line.\nAlso note that if the last line (just before the closing ' ')\nconsists only of whitespace, it's ignored.  But here there is\nsome non-whitespace stuff, so the line isn't removed. \nThis shows a hacky way to preserve an empty line after the start.\nBut there's no reason to do so: you could just repeat the empty\nline.\n  Similarly you can force an indentation level,\n  in this case to 2 spaces.  This works because the anti-quote\n  is significant (not whitespace).\nstart on network-interfaces\n\nstart script\n\n  rm -f /var/run/opengl-driver\n  ln -sf 123 /var/run/opengl-driver\n\n  rm -f /var/log/slim.log\n   \nend script\n\nenv SLIM_CFGFILE=abc\nenv SLIM_THEMESDIR=def\nenv FONTCONFIG_FILE=/etc/fonts/fonts.conf  \t\t\t\t# !!! cleanup\nenv XKB_BINDIR=foo/bin         \t\t\t\t# Needed for the Xkb extension.\nenv LD_LIBRARY_PATH=libX11/lib:libXext/lib:/usr/lib/          # related to xorg-sys-opengl - needed to load libglx for (AI)GLX support (for compiz)\n\nenv XORG_DRI_DRIVER_PATH=nvidiaDrivers/X11R6/lib/modules/drivers/ \n\nexec slim/bin/slim\nEscaping of ' followed by ': ''\nEscaping of $ followed by {: \${\nAnd finally to interpret \\n etc. as in a string: \n, \r, \t.\nfoo\n'bla'\nbar\ncut -d $'\\t' -f 1\nending dollar $$\n"
diff --git a/tvix/eval/src/tests/tvix_tests/identity-dollar-escape.nix b/tvix/eval/src/tests/tvix_tests/identity-dollar-escape.nix
new file mode 100644
index 000000000000..08951d7637a6
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/identity-dollar-escape.nix
@@ -0,0 +1 @@
+"\${foobar}"
diff --git a/tvix/eval/src/value/string.rs b/tvix/eval/src/value/string.rs
index f352281be36a..51b0f0345457 100644
--- a/tvix/eval/src/value/string.rs
+++ b/tvix/eval/src/value/string.rs
@@ -90,13 +90,14 @@ impl NixString {
     }
 }
 
-fn nix_escape_char(ch: char) -> Option<&'static str> {
-    match ch {
-        '\\' => Some("\\\\"),
-        '"' => Some("\\\""),
-        '\n' => Some("\\n"),
-        '\t' => Some("\\t"),
-        '\r' => Some("\\r"),
+fn nix_escape_char(ch: char, next: Option<&char>) -> Option<&'static str> {
+    match (ch, next) {
+        ('\\', _) => Some("\\\\"),
+        ('"', _) => Some("\\\""),
+        ('\n', _) => Some("\\n"),
+        ('\t', _) => Some("\\t"),
+        ('\r', _) => Some("\\r"),
+        ('$', Some('{')) => Some("\\$"),
         _ => None,
     }
 }
@@ -106,14 +107,17 @@ fn nix_escape_char(ch: char) -> Option<&'static str> {
 //
 // Note that this does not add the outer pair of surrounding quotes.
 fn nix_escape_string(input: &str) -> Cow<str> {
-    for (i, c) in input.chars().enumerate() {
-        if let Some(esc) = nix_escape_char(c) {
+    let mut iter = input.chars().enumerate().peekable();
+
+    while let Some((i, c)) = iter.next() {
+        if let Some(esc) = nix_escape_char(c, iter.peek().map(|(_, c)| c)) {
             let mut escaped = String::with_capacity(input.len());
             escaped.push_str(&input[..i]);
             escaped.push_str(esc);
 
-            for c in input[i + 1..].chars() {
-                match nix_escape_char(c) {
+            let mut inner_iter = input[i + 1..].chars().peekable();
+            while let Some(c) = inner_iter.next() {
+                match nix_escape_char(c, inner_iter.peek()) {
                     Some(esc) => escaped.push_str(esc),
                     None => escaped.push(c),
                 }