Skip to content

fix(assertString): throw typeError on objects with constructor and name property #2692

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

Open
newtondividedbysqm wants to merge 2 commits into
base: master
Choose a base branch
from
+14 −2
Open

fix(assertString): throw typeError on objects with constructor and name property #2692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba3c7ae
fix assertString to throw TypeErrors on objects with custom construct…
newtondividedbysqm Mar 30, 2026
5dc7d99
Add tests for boxed strings and Objects pretending to be a string
newtondividedbysqm Mar 30, 2026
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
2 changes: 1 addition & 1 deletion src/lib/util/assertString.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export default function assertString(input) {
if (input === undefined || input === null) throw new TypeError(`Expected a string but received a ${input}`);
if (input.constructor.name !== 'String') throw new TypeError(`Expected a string but received a ${input.constructor.name}`);
if (typeof input !== 'string' && !(input instanceof String)) throw new TypeError(`Expected a string but received a ${input?.constructor?.name ?? typeof input}`)
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

instanceof String will return false for boxed strings created in a different JS realm (e.g., iframe/window), which can cause valid String objects to be rejected. To avoid that cross-realm pitfall while still blocking { constructor: { name: 'String' } } spoofing, consider checking Object.prototype.toString.call(input) === '[object String]' (or reusing the existing typeOf util) alongside the primitive typeof input === 'string' check.

Suggested change
if (typeof input !== 'string' && !(input instanceof String)) throw new TypeError(`Expected a string but received a ${input?.constructor?.name ?? typeof input}`)
if (typeof input !== 'string' && Object.prototype.toString.call(input) !== '[object String]') throw new TypeError(`Expected a string but received a ${input?.constructor?.name ?? typeof input}`)

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

Missing semicolon at the end of this throw statement will fail the repo's ESLint (airbnb-base defaults to semi: always). Add a trailing semicolon (and keep statement formatting consistent with the previous line).

Suggested change
if (typeof input !== 'string' && !(input instanceof String)) throw new TypeError(`Expected a string but received a ${input?.constructor?.name ?? typeof input}`)
if (typeof input !== 'string' && !(input instanceof String)) throw new TypeError(`Expected a string but received a ${input?.constructor?.name ?? typeof input}`);

Copilot uses AI. Check for mistakes.
}
14 changes: 13 additions & 1 deletion test/util.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,23 @@ describe('assertString', () => {
assert.throws(() => { assertString([]); }, TypeError);
});

it('Should throw an error if argument provided is an Object pretending to be a \'string\'', () => {
assert.throws(() => { assertString({constructor: {name: "string"}}); }, TypeError);
});

it('Should throw an error if argument provided is an Object pretending to be a \'String\'', () => {
assert.throws(() => { assertString({constructor: {name: "String"}}); }, TypeError);
Comment on lines +58 to +62
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

These new test inputs use double quotes ("string", "String") while the surrounding tests consistently use single quotes, and the repo ESLint config extends airbnb-base (default quotes: single). Switching these to single quotes will prevent lint failures.

Suggested change
assert.throws(() => { assertString({constructor: {name: "string"}}); }, TypeError);
});
it('Should throw an error if argument provided is an Object pretending to be a \'String\'', () => {
assert.throws(() => { assertString({constructor: {name: "String"}}); }, TypeError);
assert.throws(() => { assertString({constructor: {name: 'string'}}); }, TypeError);
});
it('Should throw an error if argument provided is an Object pretending to be a \'String\'', () => {
assert.throws(() => { assertString({constructor: {name: 'String'}}); }, TypeError);

Copilot uses AI. Check for mistakes.
});

it('Should not throw an error if the argument is an empty string', () => {
assert.doesNotThrow(() => { assertString(''); });
});

Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

This blank line contains trailing whitespace, which will fail no-trailing-spaces under airbnb-base ESLint. Remove the spaces so the line is truly empty.

Suggested change

Copilot uses AI. Check for mistakes.
it('Should not throw an error if the argument is a String', () => {
assert.doesNotThrow(() => { assertString('antidisestablishmentarianism'); });
});

it('Should not throw an error if the argument is a boxed string', () => {
assert.doesNotThrow(() => { assertString(new String('antidisestablishmentarianism')); });
});
});
Loading