Skip to content

Work around global Object.prototype pollution#725

Merged
timostamm merged 2 commits intotimostamm:mainfrom
jcready:patch-5
Jun 18, 2025
Merged

Work around global Object.prototype pollution#725
timostamm merged 2 commits intotimostamm:mainfrom
jcready:patch-5

Conversation

@jcready
Copy link
Contributor

@jcready jcready commented Jun 18, 2025

Fixes #724

There shouldn’t be any issue in mutating the baseDescriptors object directly since:

  1. Object.getOwnPropertyDescriptors() always returns a referentially unique object every time it’s called.
  2. Object.create() doesn’t retain any references to the second argument (the descriptors) in the resulting object.
var a = Object.getPrototypeOf({});
var b = Object.getPrototypeOf({});
a === b; // true

var descriptor  = Object.getOwnPropertyDescriptors(a);
var descriptor2 = Object.getOwnPropertyDescriptors(a);
descriptor === descriptor2; // false

descriptor.field = {};

descriptor.field.value = 1;
var one = Object.create(null, descriptor);
one.field === 1; // true

descriptor.field.value = 2;
var two = Object.create(null, descriptor);
two.field === 2; // true
one.field === 1; // still true

Fixes timostamm#724

There shouldn’t be any issue in mutating the `baseDescriptors` object directly since:

1. `Object.getOwnPropertyDescriptors()` always returns a referentially unique object every time it’s called.
2. `Object.create()` doesn’t retain any references to the second argument (the descriptors) in the resulting object.

```js
var a = Object.getPrototypeOf({});
var b = Object.getPrototypeOf({});
a === b; // true

var descriptor  = Object.getOwnPropertyDescriptors(a);
var descriptor2 = Object.getOwnPropertyDescriptors(a);
descriptor === descriptor2; // false

descriptor.field = {};

descriptor.field.value = 1;
var one = Object.create(null, descriptor);
one.field === 1; // true

descriptor.field.value = 2;
var two = Object.create(null, descriptor);
two.field === 2; // true
one.field === 1; // still true
```
@jcready
Copy link
Contributor Author

jcready commented Jun 18, 2025

Assuming this lands I predict the next bug related to this will be along the lines of "It looks like [name of widely used package] modifies Object.getOwnPropertyDescriptors() to always return a frozen object so assigning a property to the resulting object like this throws an error."

This is the bare minimum to satisfy type checks without relying on a types from lib.d.ts.
@timostamm
Copy link
Owner

Thanks. Pushed up 2db9c80 to satisfy type checks.

@timostamm timostamm merged commit 814b539 into timostamm:main Jun 18, 2025
3 checks passed
@timostamm timostamm mentioned this pull request Jun 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[@protobuf-ts/runtime] Creation of a new MessageType fails if Object prototype already has readonly props

2 participants

Comments