Restores the fds after builtins finish executing (#3184)

* Restores the fds after builtins finish executing * Adds an extra test
yarnpkg · Jul 27, 2021 · 344818b · 344818b
1 parent 7197a44
commit 344818b
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 1 deletion.
diff --git a/.yarn/versions/8e84815d.yml b/.yarn/versions/8e84815d.yml
@@ -0,0 +1,23 @@
+releases:
+  "@yarnpkg/cli": patch
+  "@yarnpkg/shell": patch
+
+declined:
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-essentials"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-nm"
+  - "@yarnpkg/plugin-npm-cli"
+  - "@yarnpkg/plugin-pack"
+  - "@yarnpkg/plugin-patch"
+  - "@yarnpkg/plugin-pnp"
+  - "@yarnpkg/plugin-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - "@yarnpkg/builder"
+  - "@yarnpkg/core"
+  - "@yarnpkg/doctor"
diff --git a/packages/yarnpkg-shell/sources/index.ts b/packages/yarnpkg-shell/sources/index.ts
@@ -632,11 +632,23 @@ function makeCommandAction(args: Array<string>, opts: ShellOptions, state: Shell
     throw new Error(`Assertion failed: A builtin should exist for "${name}"`);
 
   return makeBuiltin(async ({stdin, stdout, stderr}) => {
+    const {
+      stdin: initialStdin,
+      stdout: initialStdout,
+      stderr: initialStderr,
+    } = state;
+
     state.stdin = stdin;
     state.stdout = stdout;
     state.stderr = stderr;
 
-    return await builtin(rest, opts, state);
+    try {
+      return await builtin(rest, opts, state);
+    } finally {
+      state.stdin = initialStdin;
+      state.stdout = initialStdout;
+      state.stderr = initialStderr;
+    }
   });
 }
 

diff --git a/packages/yarnpkg-shell/tests/shell.test.ts b/packages/yarnpkg-shell/tests/shell.test.ts
@@ -739,6 +739,60 @@ describe(`Shell`, () => {
         });
       });
 
+      it(`shouldn't affect unrelated commands`, async () => {
+        await xfs.mktempPromise(async tmpDir => {
+          const file = ppath.join(tmpDir, `file` as Filename);
+
+          await expect(bufferResult(
+            `echo "hello world" > "${file}"; echo foo`,
+          )).resolves.toMatchObject({
+            stdout: `foo\n`,
+          });
+
+          await expect(xfs.readFilePromise(file, `utf8`)).resolves.toEqual(`hello world\n`);
+        });
+
+        await xfs.mktempPromise(async tmpDir => {
+          const file = ppath.join(tmpDir, `file` as Filename);
+
+          await expect(bufferResult(
+            `echo "hello world" > "${file}" && echo foo`,
+          )).resolves.toMatchObject({
+            stdout: `foo\n`,
+          });
+
+          await expect(xfs.readFilePromise(file, `utf8`)).resolves.toEqual(`hello world\n`);
+        });
+      });
+
+      it(`shouldn't do weird stuff when piping a builtin redirection`, async () => {
+        await xfs.mktempPromise(async tmpDir => {
+          const file1 = ppath.join(tmpDir, `file1` as Filename);
+          const file2 = ppath.join(tmpDir, `file2` as Filename);
+
+          await expect(bufferResult(
+            `echo "hello world" > "${file1}" | echo "foo bar" > "${file2}"; echo test`,
+          )).resolves.toMatchObject({
+            stdout: `test\n`,
+          });
+
+          await expect(xfs.readFilePromise(file1, `utf8`)).resolves.toEqual(`hello world\n`);
+          await expect(xfs.readFilePromise(file2, `utf8`)).resolves.toEqual(`foo bar\n`);
+        });
+
+        await xfs.mktempPromise(async tmpDir => {
+          const file = ppath.join(tmpDir, `file` as Filename);
+
+          await expect(bufferResult(
+            `echo "hello world" > "${file}" && echo foo`,
+          )).resolves.toMatchObject({
+            stdout: `foo\n`,
+          });
+
+          await expect(xfs.readFilePromise(file, `utf8`)).resolves.toEqual(`hello world\n`);
+        });
+      });
+
       it(`should support output redirections from fd (stdout)`, async () => {
         await xfs.mktempPromise(async tmpDir => {
           const file = ppath.join(tmpDir, `file` as Filename);