Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support more valid shell names on Windows #1137

Merged
merged 7 commits into from
Aug 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/fuzz-cmd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,5 @@ jobs:
with:
duration: 600 # seconds == 10 minutes
os: windows-2022
shell: cmd.exe
shell: cmd
targets: '["exec", "exec-file", "spawn"]'
2 changes: 1 addition & 1 deletion .github/workflows/fuzz-powershell.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,5 @@ jobs:
with:
duration: 600 # seconds == 10 minutes
os: windows-2022
shell: powershell.exe
shell: powershell
targets: '["exec", "exec-file", "spawn"]'
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Versioning].

## [Unreleased]

- _No changes yet_
- Support `shell` values without `.exe` for Windows. ([#1137])

## [1.7.3] - 2023-08-07

Expand Down Expand Up @@ -291,6 +291,7 @@ Versioning].
[#1082]: https://github.com/ericcornelissen/shescape/pull/1082
[#1083]: https://github.com/ericcornelissen/shescape/pull/1083
[#1094]: https://github.com/ericcornelissen/shescape/pull/1094
[#1137]: https://github.com/ericcornelissen/shescape/pull/1137
[552e8ea]: https://github.com/ericcornelissen/shescape/commit/552e8eab56861720b1d4e5474fb65741643358f9
[keep a changelog]: https://keepachangelog.com/en/1.0.0/
[semantic versioning]: https://semver.org/spec/v2.0.0.html
6 changes: 3 additions & 3 deletions src/win.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export function getDefaultShell({ env: { ComSpec } }) {
* @returns {Function | undefined} A function to escape arguments.
*/
export function getEscapeFunction(shellName, options) {
switch (shellName) {
switch (shellName.toLowerCase()) {
case binCmd:
return cmd.getEscapeFunction(options);
case binPowerShell:
Expand All @@ -71,7 +71,7 @@ export function getEscapeFunction(shellName, options) {
* @returns {Function[] | undefined} A function pair to escape & quote arguments.
*/
export function getQuoteFunction(shellName) {
switch (shellName) {
switch (shellName.toLowerCase()) {
case binCmd:
return cmd.getQuoteFunction();
case binPowerShell:
Expand All @@ -86,7 +86,7 @@ export function getQuoteFunction(shellName) {
* @returns {Function | undefined} A function to protect against flag injection.
*/
export function getFlagProtectionFunction(shellName) {
switch (shellName) {
switch (shellName.toLowerCase()) {
case binCmd:
return cmd.getFlagProtectionFunction();
case binPowerShell:
Expand Down
4 changes: 2 additions & 2 deletions test/_runners.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const shescape = require("shescape");
function isShellCmd(shell) {
return (
(constants.isWindows && [undefined, true, false].includes(shell)) ||
/cmd\.exe$/u.test(shell)
/cmd(?:\.(?:EXE|exe))?$/u.test(shell)
);
}

Expand All @@ -41,7 +41,7 @@ function isShellCsh(shell) {
* @returns {boolean} `true` if `shell` is PowerShell, `false` otherwise.
*/
function isShellPowerShell(shell) {
return /powershell\.exe$/u.test(shell);
return /powershell(?:\.(?:EXE|exe))?$/u.test(shell);
}

/**
Expand Down
78 changes: 37 additions & 41 deletions test/integration/_generators.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import common from "../_constants.cjs";
/**
* Returns the shells officially supported by Shescape for the current platform.
*
* @returns {string[]} Supported shells for the current platform.
* @yields {string} Supported shells for the current platform.
*/
function getPlatformShells() {
export function* platformShells() {
if (common.isWindows) {
return common.shellsWindows;
yield* common.shellsWindows;
} else {
return common.shellsUnix;
yield* common.shellsUnix;
}
}

Expand Down Expand Up @@ -51,59 +51,55 @@ function getShellFixtures(shell) {
/**
* Generates example fixtures for escaping for the current platform.
*
* @param {string} shell A shell name.
* @yields Examples of the form `{ expected, input, options }`.
*/
export function* escapeExamples() {
const shells = getPlatformShells();
for (const shell of shells) {
const shellFixtures = getShellFixtures(shell);
export function* escapeExamples(shell) {
const shellFixtures = getShellFixtures(shell);

for (const example of shellFixtures.escape) {
const input = example.input;
for (const example of shellFixtures.escape) {
const input = example.input;

{
const expected = example.expected.interpolation;
const options = { flagProtection: false, interpolation: true, shell };
yield { expected, input, options };
}

{
const expected = example.expected.noInterpolation;
const options = { flagProtection: false, interpolation: false, shell };
yield { expected, input, options };
}
{
const expected = example.expected.interpolation;
const options = { flagProtection: false, interpolation: true, shell };
yield { expected, input, options };
}

for (const example of shellFixtures.flag) {
const input = example.input;
const expected = example.expected.unquoted;
const options = { flagProtection: true, shell };
{
const expected = example.expected.noInterpolation;
const options = { flagProtection: false, interpolation: false, shell };
yield { expected, input, options };
}
}

for (const example of shellFixtures.flag) {
const input = example.input;
const expected = example.expected.unquoted;
const options = { flagProtection: true, shell };
yield { expected, input, options };
}
}
/**
* Generates example fixtures for quoting for the current platform.
*
* @param {string} shell A shell name.
* @yields Examples of the form `{ expected, input, options }`.
*/
export function* quoteExamples() {
const shells = getPlatformShells();
for (const shell of shells) {
const shellFixtures = getShellFixtures(shell);
export function* quoteExamples(shell) {
const shellFixtures = getShellFixtures(shell);

for (const example of shellFixtures.quote) {
const input = example.input;
const expected = example.expected;
const options = { flagProtection: false, shell };
yield { expected, input, options };
}
for (const example of shellFixtures.quote) {
const input = example.input;
const expected = example.expected;
const options = { flagProtection: false, shell };
yield { expected, input, options };
}

for (const example of shellFixtures.flag) {
const input = example.input;
const expected = example.expected.quoted;
const options = { flagProtection: true, shell };
yield { expected, input, options };
}
for (const example of shellFixtures.flag) {
const input = example.input;
const expected = example.expected.quoted;
const options = { flagProtection: true, shell };
yield { expected, input, options };
}
}
14 changes: 8 additions & 6 deletions test/integration/escape-all.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ import { arbitrary, constants, generate, macros } from "./_.js";
import { escape, escapeAll } from "shescape";
import { escapeAll as escapeAllCjs } from "../../index.cjs";

test("inputs are escaped", (t) => {
for (const { expected, input, options } of generate.escapeExamples()) {
const result = escapeAll([input], options);
t.deepEqual(result, [expected]);
}
});
for (const shell of generate.platformShells()) {
test(`inputs are escaped for ${shell}`, (t) => {
for (const { expected, input, options } of generate.escapeExamples(shell)) {
const result = escapeAll([input], options);
t.deepEqual(result, [expected]);
}
});
}

testProp(
"return values",
Expand Down
14 changes: 8 additions & 6 deletions test/integration/escape.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ import { arbitrary, constants, generate, macros } from "./_.js";
import { escape as escape } from "shescape";
import { escape as escapeCjs } from "../../index.cjs";

test("input is escaped", (t) => {
for (const { expected, input, options } of generate.escapeExamples()) {
const result = escape(input, options);
t.is(result, expected);
}
});
for (const shell of generate.platformShells()) {
test(`input is escaped for ${shell}`, (t) => {
for (const { expected, input, options } of generate.escapeExamples(shell)) {
const result = escape(input, options);
t.is(result, expected);
}
});
}

testProp(
"return values",
Expand Down
14 changes: 8 additions & 6 deletions test/integration/quote-all.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ import { arbitrary, constants, generate, macros } from "./_.js";
import { quote, quoteAll as quoteAll } from "shescape";
import { quoteAll as quoteAllCjs } from "../../index.cjs";

test("inputs are quoted", (t) => {
for (const { expected, input, options } of generate.quoteExamples()) {
const result = quoteAll([input], options);
t.deepEqual(result, [expected]);
}
});
for (const shell of generate.platformShells()) {
test(`inputs are quoted for ${shell}`, (t) => {
for (const { expected, input, options } of generate.quoteExamples(shell)) {
const result = quoteAll([input], options);
t.deepEqual(result, [expected]);
}
});
}

testProp(
"return values",
Expand Down
14 changes: 8 additions & 6 deletions test/integration/quote.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ import { arbitrary, constants, generate, macros } from "./_.js";
import { quote as quote } from "shescape";
import { quote as quoteCjs } from "../../index.cjs";

test("input is quoted", (t) => {
for (const { expected, input, options } of generate.quoteExamples()) {
const result = quote(input, options);
t.is(result, expected);
}
});
for (const shell of generate.platformShells()) {
test(`input is quoted for ${shell}`, (t) => {
for (const { expected, input, options } of generate.quoteExamples(shell)) {
const result = quote(input, options);
t.is(result, expected);
}
});
}

testProp(
"return value",
Expand Down
6 changes: 4 additions & 2 deletions test/unit/win/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ import * as win from "../../../src/win.js";
import * as powershell from "../../../src/win/powershell.js";

const shells = [
{ module: cmd, shellName: constants.binCmd },
{ module: powershell, shellName: constants.binPowerShell },
{ module: cmd, shellName: "cmd.exe" },
{ module: powershell, shellName: "powershell.exe" },
{ module: cmd, shellName: "cmd.EXE" },
{ module: powershell, shellName: "powershell.EXE" },
];

testProp(
Expand Down