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

util: fix .format() not always calling toString when it should be #30343

Closed
wants to merge 2 commits into from

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Nov 9, 2019

This makes sure that util.format('%s', object) will always call
a user defined toString function. It was formerly not the case
when the object had the function declared on the super class.

At the same time this also makes sure that getters won't be
triggered accessing the constructor property.

Fixes: #30333

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Nov 9, 2019
@nodejs-github-bot

This comment has been minimized.

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 12, 2019
Copy link
Member

@lundibundi lundibundi left a comment

Choose a reason for hiding this comment

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

LGTM, 👍

lib/internal/util/inspect.js Outdated Show resolved Hide resolved
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

This makes sure that `util.format('%s', object)` will always call
a user defined `toString` function. It was formerly not the case
when the object had the function declared on the super class.

At the same time this also makes sure that getters won't be
triggered accessing the `constructor` property.
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

addaleax pushed a commit that referenced this pull request Nov 30, 2019
This makes sure that `util.format('%s', object)` will always call
a user defined `toString` function. It was formerly not the case
when the object had the function declared on the super class.

At the same time this also makes sure that getters won't be
triggered accessing the `constructor` property.

PR-URL: #30343
Fixes: #30333
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@addaleax
Copy link
Member

Landed in be30911

@addaleax addaleax closed this Nov 30, 2019
targos pushed a commit that referenced this pull request Dec 1, 2019
This makes sure that `util.format('%s', object)` will always call
a user defined `toString` function. It was formerly not the case
when the object had the function declared on the super class.

At the same time this also makes sure that getters won't be
triggered accessing the `constructor` property.

PR-URL: #30343
Fixes: #30333
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@BridgeAR BridgeAR mentioned this pull request Dec 3, 2019
@targos
Copy link
Member

targos commented Jan 14, 2020

Should this be backported to v12.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
This makes sure that `util.format('%s', object)` will always call
a user defined `toString` function. It was formerly not the case
when the object had the function declared on the super class.

At the same time this also makes sure that getters won't be
triggered accessing the `constructor` property.

Backport-PR-URL: #31431
PR-URL: #30343
Fixes: #30333
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
This makes sure that `util.format('%s', object)` will always call
a user defined `toString` function. It was formerly not the case
when the object had the function declared on the super class.

At the same time this also makes sure that getters won't be
triggered accessing the `constructor` property.

Backport-PR-URL: #31431
PR-URL: #30343
Fixes: #30333
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request May 5, 2024
This makes sure that `util.format('%s', object)` will always call
a user defined `toString` function. It was formerly not the case
when the object had the function declared on the super class.

At the same time this also makes sure that getters won't be
triggered accessing the `constructor` property.

PR-URL: nodejs/node#30343
Fixes: nodejs/node#30333
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

util.format('%s', o) fails to call String(o) in certain cases
8 participants