fix: capture the promise global to avoid userland mutation #20925
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This was caught as we adopted the promisified methods and noticed them not resolving in the renderer |
codebytere
approved these changes
Nov 4, 2019
nornagon
requested changes
Nov 4, 2019
nornagon
approved these changes
Nov 4, 2019
MarshallOfSound
force-pushed
the
capture-the-promise-global
branch
from
ae1cef9
to
1d08b8a
Compare
|
Release Notes Persisted
|
|
I was unable to backport this PR to "7-0-x" cleanly; |
|
I was unable to backport this PR to "8-x-y" cleanly; |
|
@MarshallOfSound has manually backported this PR to "8-x-y", please check out #20946 |
|
@MarshallOfSound has manually backported this PR to "7-0-x", please check out #20947 |
|
@MarshallOfSound has manually backported this PR to "7-1-x", please check out #20947 |
MarshallOfSound
added a commit
that referenced
this pull request
Nov 4, 2019
MarshallOfSound
added a commit
that referenced
this pull request
Nov 4, 2019
andersk
added a commit
to andersk/electron-remote
that referenced
this pull request
Mar 1, 2022
This an equivalent of electron/electron#20925. It’s needed for us to pass the test suite when we’re actually testing this module instead of Electron’s builtin remote. Signed-off-by: Anders Kaseorg <andersk@mit.edu>
nornagon
pushed a commit
to electron/remote
that referenced
this pull request
Mar 15, 2022
… 14, 15, 16, and 17 (#113) * fix: Capture Promise global to avoid userland mutation This an equivalent of electron/electron#20925. It’s needed for us to pass the test suite when we’re actually testing this module instead of Electron’s builtin remote. Signed-off-by: Anders Kaseorg <andersk@mit.edu> * test: Add missing semicolon Semicolons are optional in JavaScript, except when they aren’t. Signed-off-by: Anders Kaseorg <andersk@mit.edu> * test: Test this module, not the builtin electron.remote Signed-off-by: Anders Kaseorg <andersk@mit.edu> * test: Use enable() instead of enableRemoteModule: true Signed-off-by: Anders Kaseorg <andersk@mit.edu> * test: Cast remote-get-global message for Electron ≥ 14 Signed-off-by: Anders Kaseorg <andersk@mit.edu> * test: Skip error cause test for Electron ≥ 14 Signed-off-by: Anders Kaseorg <andersk@mit.edu> * ci: Test with Electron 14, 15, 16, and 17 Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A common module
Bluebirdoverrides thePromiseglobal with a non A+ implementation that doesn't support thennables (Promise.resolve({ then: () => Promise.resolve(123) })).This modifies Electrons code to make it impossible for us to rely on the global, instead we cache the global at runtime in the
capturedGlobalsobject that is auto-injected by webpack where it is used.There is also an eslint rule to prevent accidental re-introduction of this issue.
This is kinda similar to node's primordials but doesn't strip prototypes and deep clone, it just retains a reference to the original constructor.
Notes: Fixed issue where proxied remote promises might not resolve if
Bluebirdwas installed in the renderer