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.

Sign up for GitHub

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

fix: validate integer values in Deno.exitCode setter #24068

Merged
merged 6 commits into from
Jun 3, 2024
Merged

fix: validate integer values in Deno.exitCode setter #24068

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift click to select a range
9aec742
merge into tests/unit/os_test.ts
petamoriken Jun 1, 2024
d235a31
fix: validate integer values in `Deno.exitCode` setter
petamoriken Jun 1, 2024
7d45888
fix: no implicit type conversions in `Deno.exitCode` setter
petamoriken Jun 1, 2024
4ed4b69
fix: more strict `process.exitCode`
petamoriken Jun 1, 2024
0f53cf3
remove unmatched tests
petamoriken Jun 1, 2024
b28b045
Merge branch 'main' into fix/exit-code-tests
petamoriken Jun 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: validate integer values in Deno.exitCode setter
  • Loading branch information
@petamoriken
petamoriken committed Jun 1, 2024
commit d235a31fcdcd15ea11482e98464373fba68a0822
8 changes: 4 additions & 4 deletions runtime/js/30_os.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 22,7 @@ import {
const {
Error,
FunctionPrototypeBind,
NumberParseInt,
NumberIsInteger,
SymbolFor,
TypeError,
} = primordials;
Expand Down Expand Up @@ -102,10 102,10 @@ function getExitCode() {
}

function setExitCode(value) {
const code = NumberParseInt(value, 10);
if (typeof code !== "number") {
Comment on lines -105 to -106
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the code variable is always a number, this validation was dead code.

const code = value;
if (!NumberIsInteger(code)) {
throw new TypeError(
`Exit code must be a number, got: ${code} (${typeof code}).`,
`Exit code must be a integer, got: ${value} (${typeof value})`,
);
}
op_set_exit_code(code);
Expand Down
107 changes: 47 additions & 60 deletions tests/unit/os_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,22 307,28 @@ Deno.test("Deno.exitCode getter and setter", () => {
// Initial value is 0
assertEquals(Deno.exitCode, 0);

// Set a new value
Deno.exitCode = 5;
assertEquals(Deno.exitCode, 5);
try {
// Set a new value
Deno.exitCode = 5;
assertEquals(Deno.exitCode, 5);
} finally {
// Reset to initial value
Deno.exitCode = 0;
}

// Reset to initial value
Deno.exitCode = 0;
assertEquals(Deno.exitCode, 0);
});

Deno.test("Setting Deno.exitCode to NaN throws TypeError", () => {
// @ts-expect-error;
Deno.exitCode = "123";
assertEquals(Deno.exitCode, 123);
Deno.test("Setting Deno.exitCode to non-integer throws TypeError", () => {
try {
// @ts-expect-error;
Deno.exitCode = "123";
assertEquals(Deno.exitCode, 123);
} finally {
// Reset
Deno.exitCode = 0;
}

// Reset
Deno.exitCode = 0;
assertEquals(Deno.exitCode, 0);

// Throws on non-number values
Expand All @@ -332,64 338,45 @@ Deno.test("Setting Deno.exitCode to NaN throws TypeError", () => {
Deno.exitCode = "not a number";
},
TypeError,
"Exit code must be a number.",
"Exit code must be a integer, got: not a number (string)",
);
});

Deno.test("Setting Deno.exitCode does not cause an immediate exit", () => {
let exited = false;
const originalExit = Deno.exit;

// @ts-expect-error; read-only
Deno.exit = () => {
exited = true;
};

Deno.exitCode = 1;
assertEquals(exited, false);

// @ts-expect-error; read-only
Deno.exit = originalExit;
});

Deno.test("Running Deno.exit(value) overrides Deno.exitCode", () => {
let args: unknown[] | undefined;

const originalExit = Deno.exit;
// @ts-expect-error; read-only
Deno.exit = (...x) => {
args = x;
};
// Throws on non-integer values
assertThrows(
() => {
Deno.exitCode = 3.14;
},
TypeError,
"Exit code must be a integer, got: 3.14 (number)",
);

Deno.exitCode = 42;
Deno.exit(0);
// Throws on bigint values
assertThrows(
() => {
// @ts-expect-error Testing for runtime error
Deno.exitCode = 1n;
},
TypeError,
"Cannot convert a BigInt value to a number",
);

assertEquals(args, [0]);
// @ts-expect-error; read-only
Deno.exit = originalExit;
assertEquals(Deno.exitCode, 0);
});

Deno.test("Running Deno.exit() uses Deno.exitCode as fallback", () => {
let args: unknown[] | undefined;
Deno.test("Setting Deno.exitCode does not cause an immediate exit", () => {
let exited = false;

const originalExit = Deno.exit;
// @ts-expect-error; read-only
Deno.exit = (...x) => {
args = x;
Deno.exit = () => {
exited = true;
};

Deno.exitCode = 42;
Deno.exit();

assertEquals(args, [42]);
// @ts-expect-error; read-only
Deno.exit = originalExit;
});

Deno.test("Retrieving the set exit code before process termination", () => {
Deno.exitCode = 42;
assertEquals(Deno.exitCode, 42);

// Reset to initial value
Deno.exitCode = 0;
try {
Deno.exitCode = 1;
assertEquals(exited, false);
} finally {
Deno.exit = originalExit;
Deno.exitCode = 0;
}
});
Loading