From 2c85fcce875404f07ce29f7a2bb2ed970d2d5840 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 4 Mar 2024 08:06:36 +0100 Subject: [PATCH] Merge pull request #9747 from awakesecurity/mz/fix-quadratic-splitString Fix performance of builtins.substring for empty substrings (cherry picked from commit b2deff1947c2fe57fdbf1a472eb9003eb407f8d3) Change-Id: I4ddfc8d26a4781c9520fff9807849a073ee7bed8 --- src/libexpr/primops.cc | 19 ++++++++++++++++--- .../lang/eval-okay-substring-context.exp | 1 + .../lang/eval-okay-substring-context.nix | 11 +++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 tests/functional/lang/eval-okay-substring-context.exp create mode 100644 tests/functional/lang/eval-okay-substring-context.nix diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index e107db228..d64f2d51e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -3700,9 +3700,6 @@ static RegisterPrimOp primop_toString({ static void prim_substring(EvalState & state, const PosIdx pos, Value * * args, Value & v) { int start = state.forceInt(*args[0], pos, "while evaluating the first argument (the start offset) passed to builtins.substring"); - int len = state.forceInt(*args[1], pos, "while evaluating the second argument (the substring length) passed to builtins.substring"); - NixStringContext context; - auto s = state.coerceToString(pos, *args[2], context, "while evaluating the third argument (the string) passed to builtins.substring"); if (start < 0) state.debugThrowLastTrace(EvalError({ @@ -3710,6 +3707,22 @@ static void prim_substring(EvalState & state, const PosIdx pos, Value * * args, .errPos = state.positions[pos] })); + + int len = state.forceInt(*args[1], pos, "while evaluating the second argument (the substring length) passed to builtins.substring"); + + // Special-case on empty substring to avoid O(n) strlen + // This allows for the use of empty substrings to efficently capture string context + if (len == 0) { + state.forceValue(*args[2], pos); + if (args[2]->type() == nString) { + v.mkString("", args[2]->string.context); + return; + } + } + + NixStringContext context; + auto s = state.coerceToString(pos, *args[2], context, "while evaluating the third argument (the string) passed to builtins.substring"); + v.mkString((unsigned int) start >= s->size() ? "" : s->substr(start, len), context); } diff --git a/tests/functional/lang/eval-okay-substring-context.exp b/tests/functional/lang/eval-okay-substring-context.exp new file mode 100644 index 000000000..2fe7f71fa --- /dev/null +++ b/tests/functional/lang/eval-okay-substring-context.exp @@ -0,0 +1 @@ +"okay" diff --git a/tests/functional/lang/eval-okay-substring-context.nix b/tests/functional/lang/eval-okay-substring-context.nix new file mode 100644 index 000000000..d0ef70d4e --- /dev/null +++ b/tests/functional/lang/eval-okay-substring-context.nix @@ -0,0 +1,11 @@ +with builtins; + +let + + s = "${builtins.derivation { name = "test"; builder = "/bin/sh"; system = "x86_64-linux"; }}"; + +in + +if getContext s == getContext "${substring 0 0 s + unsafeDiscardStringContext s}" +then "okay" +else throw "empty substring should preserve context"