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

Altering Object.prototype.name breaks in the REPL #11614

Closed
nem035 opened this issue Feb 28, 2017 · 3 comments
Closed

Altering Object.prototype.name breaks in the REPL #11614

nem035 opened this issue Feb 28, 2017 · 3 comments
Labels
util Issues and PRs related to the built-in util module.

Comments

@nem035
Copy link

nem035 commented Feb 28, 2017

  • Version: Tested on all from 5.11.1 to 7.6.0.
  • Platform: Darwin 15.6.0 Darwin Kernel Version 15.6.0: Thu Sep 1 15:01:16 PDT 2016; root:xnu-3248.60.11~2/RELEASE_X86_64 x86_64
  • Subsystem: Util

Problematic code:

Object.prototype.name = "bug"
Object.prototype

Sample run:

screen shot 2017-02-28 at 12 17 35 pm

Code breaks in stylizeWithColor the following line.

Why?

Because we are printing a property, the call to stylizeWithColor passes 'name' as the styleType where 'name' style type is actually ignored.

These style types are stored in the plain object inspect.styles so the execution expects inspect.styles['name'] on the first line of stylizeWithColor to return undefined (since it is ignored) and not enter the if statement.

function stylizeWithColor(str, styleType) {
  var style = inspect.styles[styleType]; // <-- this should return `undefined`

  if (style) { // <-- this shouldn't happen
    return `\u001b[${inspect.colors[style][0]}m${str}` +
           `\u001b[${inspect.colors[style][1]}m`;
  } else {
    return str;
  }
}

However, since we added a "name" property on Object.prototype, inspect.styles['name'] doesn't find 'name' directly but does find it up the [[Prototype]] chain and actually returns the string"bug".

function stylizeWithColor(str, styleType) {
  var style = inspect.styles[styleType]; // <-- this returns "bug"` because it found it up the [[Prototype]] chain

  if (style) { // <-- // <-- this happens because "bug" is truthy
    return `\u001b[${inspect.colors[style][0]}m${str}` +
           `\u001b[${inspect.colors[style][1]}m`;
  } else {
    return str;
  }
}

This means that the code tries to execute inspect.colors["bug"][0], which gives undefined[0], and throws the error in the issue.

A funny consequence is that, if we make name equal to a color value used in inspect.colors, then the error wouldn't happen and the property name will be printed in that color.

screen shot 2017-02-28 at 10 30 52 pm

Proposed solution

Use an object not linked to Object.prototype for inspect.style.

const styles = {
  'special': 'cyan',
  'number': 'yellow',
  'boolean': 'yellow',
  'undefined': 'grey',
  'null': 'bold',
  'string': 'green',
  'symbol': 'green',
  'date': 'magenta',
  // "name": intentionally not styling
  'regexp': 'red'
};
inspect.styles = Object.assign(Object.create(null), styles);

If confirmed, I'd be happy to implement it.


Inspired by this StackOverflow question

@nem035 nem035 changed the title Altering Object.prototype breaks in the REPL Altering Object.prototype.name breaks in the REPL Feb 28, 2017
@addaleax
Copy link
Member

If confirmed, I'd be happy to implement it.

I personally wouldn’t have a problem with that, a prototypeless object is certainly not wrong for styles.
But tbh, if you’re messing with Object.prototype, you’re on thin ice anyway. 😄

@addaleax addaleax added the util Issues and PRs related to the built-in util module. label Feb 28, 2017
@nem035
Copy link
Author

nem035 commented Feb 28, 2017

@addaleax cool. And you're definitely right, but I guess the REPL would be the best place to experiment with such ice skating 😄

@nem035
Copy link
Author

nem035 commented Feb 28, 2017

Although not directly related to this bug, it might also make sense to use a prototypeless object for inspect.colors. What do you think @addaleax?

nem035 pushed a commit to nem035/node that referenced this issue Mar 1, 2017
…ects

Use a prototype-less object for inspect.styles and inspect.colors to allow
modification of Object.prototype in the REPL.

Fixes: nodejs#11614
@jasnell jasnell closed this as completed in aab0d20 Mar 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
util Issues and PRs related to the built-in util module.
Projects
None yet
Development

No branches or pull requests

2 participants