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.

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

Jump to bottom

fix: EventEmitter is missing properties in sandbox preload script. #35522

Merged
merged 2 commits into from Sep 15, 2022
Merged

fix: EventEmitter is missing properties in sandbox preload script. #35522

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cce3e54
fix: EventEmitter is missing properties in sandbox preload script.
marekharanczyk Aug 31, 2022
d05d4f7
fixup! fix: EventEmitter is missing properties in sandbox preload scr…
marekharanczyk Sep 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Expand Up @@ -45,6 +45,7 @@
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-standard": "^4.0.1",
"eslint-plugin-typescript": "^0.14.0",
"events": "^3.2.0",
Copy link
Member

Choose a reason for hiding this comment

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

does webpack automatically detect this module somehow? I'm surprised that just adding it to package.json is enough to have this be added to the preload bundle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for late answer, I was on vacation. I have not looked into webpack internals that much to be honest. Most migration guides state that you should include what you use, so I just did that, similarly to original PR adding process and buffer.

"express": "^4.16.4",
"folder-hash": "^2.1.1",
"fs-extra": "^9.0.1",
Expand Down
20 changes: 20 additions & 0 deletions spec/api-browser-window-spec.ts
View file
Open in desktop
Expand Up @@ -2970,6 +2970,26 @@ describe('BrowserWindow module', () => {
expect(url).to.equal(expectedUrl);
});

it('exposes full EventEmmiter object to preload script', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
it('exposes full EventEmmiter object to preload script', async () => {
it('exposes full EventEmitter object to preload script', async () => {

const w = new BrowserWindow({
show: false,
webPreferences: {
sandbox: true,
preload: path.join(fixtures, 'module', 'preload-eventemitter.js')
}
});
w.loadURL('about:blank');
const [, rendererEventEmitterProperties] = await emittedOnce(ipcMain, 'answer');
const { EventEmitter } = require('events');
const emitter = new EventEmitter();
let browserEventEmitterProperties = '';
let currentObj = emitter;
do {
Object.getOwnPropertyNames(currentObj).map(property => { browserEventEmitterProperties += property + ';'; });
} while ((currentObj = Object.getPrototypeOf(currentObj)));
expect(rendererEventEmitterProperties).to.equal(browserEventEmitterProperties);
});

it('should open windows in same domain with cross-scripting enabled', async () => {
const w = new BrowserWindow({
show: true,
Expand Down
11 changes: 11 additions & 0 deletions spec/fixtures/module/preload-eventemitter.js
View file
Open in desktop
@@ -0,0 +1,11 @@
(function () {
const { EventEmitter } = require('events');
const emitter = new EventEmitter();
let rendererEventEmitterProperties = '';
let currentObj = emitter;
do {
Object.getOwnPropertyNames(currentObj).map(property => { rendererEventEmitterProperties += property + ';'; });
} while ((currentObj = Object.getPrototypeOf(currentObj)));
const { ipcRenderer } = require('electron');
ipcRenderer.send('answer', rendererEventEmitterProperties);
Copy link
Member

Choose a reason for hiding this comment

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

This can be an array of strings, no need to flatten it out to a string.

Suggested change
let rendererEventEmitterProperties = '';
let currentObj = emitter;
do {
Object.getOwnPropertyNames(currentObj).map(property => { rendererEventEmitterProperties += property + ';'; });
} while ((currentObj = Object.getPrototypeOf(currentObj)));
const { ipcRenderer } = require('electron');
ipcRenderer.send('answer', rendererEventEmitterProperties);
let rendererEventEmitterProperties = [];
let currentObj = emitter;
do {
rendererEventEmitterProperties.push(...Object.getOwnPropertyNames(currentObj));
} while ((currentObj = Object.getPrototypeOf(currentObj)));
const { ipcRenderer } = require('electron');
ipcRenderer.send('answer', rendererEventEmitterProperties);

})();