IpcRenderer does not execute normally in preload.js #21437
Comments
|
I'd like to add some detail, it appears ipcRenderer does not include all methods when executing in a preload.js script. This is problematic, because there is no other way to pass an ipcRenderer to a renderer process. What does this mean? Renderer process cannot use ipcRenderer.on("", {}). preload.js What gets displayed in the console: |
|
It appears the ipcRenderer is only an EventEmitter instance in the preload.js There must be some lifecycle code/event/gen that populates the various properties for an ipcRenderer, and this process isn't happening in the preload script. |
|
I would gladly help if someone could point me in the right direction, my familiarity with the source code is very low. |
|
Another note, if you preload the ipcRenderer, using the contextBridge, even in a renderer process you get a subset of the methods (only the EventEmitter methods as described above), not the entire method set as mentioned in the docs. So the issue is not with the ipcRenderer but with the context bridge. |
|
There is no issue with contextBridge, simply the preload script itself, see MVP where you can test the bug (Electron v.7.1.8): https://github.com/reZach/broken-preload-example |
|
It looks like the problem is here: Lines 42 to 52 in ee58d60 |
|
Update on the sample repo. @ChinaLiuxiaosong It turns out ipcRenderer.on works fine in the preload, at least in my sample repo. Please view the following gist for more details. My test repo has also been updated to show you can call ipcRenderer.on in the preload. However - you still cannot call ipcRenderer.on(channel, args) from the renderer process. This is desired because it is not feasible to put all logic regarding the ipcRenderer in the preload. ie. I have a i18n library that needs to send ipc messages to the main process in order to save translation files to disk. This library exists in the renderer process, and in order for the translation files to be saved it needs to call out to the main process (which has access to Again, the MVP test repo has been updated and can replicate this problem for someone to replicate the issue. |
|
I've posted a $230 bounty on this issue for prioritization, thanks! |
|
Turns out, the problem is contextBridge. A possible solution is to not use contextBridge and simply load the |
|
@reZach That is not a solution and is a terrible idea. The problem is not the context bridge. Copying your desired state from your other issue, this is how you do it correctly: renderermainIsolated Preloadconst { contextBridge, ipcRenderer } = require('electron');
contextBridge.exposeInMainWorld('myapi', {
request: (data) => ipcRenderer.send("request", {
data,
}),
onResponse: (fn) => {
// Deliberately strip event as it includes `sender`
ipcRenderer.on('response', (event, ...args) => fn(...args));
}
}Rendererclass LibraryClass {
constructor(myValue = 1) {
this.classValue = myValue;
window.myapi.onResponse((args) => {
if (args.success) this.classValue++;
});
}
send() {
window.myapi.request(this.classValue);
}
}Main Processlet win = new BrowserWindow({...});
ipcMain.on("request", (IpcMainEvent, args) => {
// Use node module (ie. "fs");
// perhaps save value of args.data to file with a timestamp
win.webContents.send("response", {
success: true
});
});Things to note
Final Note@reZach I'd ask that you stop spamming these issues as your issue is not this potential bug rather user issues with how the APIs work. |
|
@MarshallOfSound Now, I have two questions:
|
|
@ChinaLiuxiaosong sorry my reply above wasn't to you, the reply was addressing a user who is not experiencing whatever you are and I haven't looked into your particular hug report yet. |
|
@MarshallOfSound Yes, this is my fault. I continued the conversation after I had realized the issue was not related to @ChinaLiuxiaosong's. I did so to help anyone who might find value in the process to work around my specific issue, not the one created. Thank you for steering me on the right course for this repo with you are a member of. I will remember my faults and not repeat them again. |
|
@ChinaLiuxiaosong, can you explain what you are trying to achieve, I think it would help us solve for you your question. What I think you are trying to do is communicate between two BrowserWindows? I think your current code can work, but I feel its not set up in the best way it can be. I took a stab at your code and I was able to see the "LOADED" message as I think you were aiming to see that. Something about the .once() calls were getting caught, so I removed it and removed the "LOADING" message as well (we could just send a LOADED message when done, it can feel redundant to send a LOADING message because we know it's still loading :)) const {
remote,
ipcRenderer
} = require("electron");
const winName = process.argv.pop();
const webContentsId = remote.getCurrentWebContents().id;
const otherWebContents = remote.webContents.getAllWebContents().find(wc => wc.id != webContentsId);
const otherWebContentsId = otherWebContents && otherWebContents.id;
function now() {
let date = new Date();
return `${date.getHours()}:${date.getMinutes()}:${date.getSeconds()}.${date.getMilliseconds()}`;
}
function sendToOtherWin(msg) {
// console.log(`[${now()}] listen once 'test.reply'`);
// ipcRenderer.once("test.reply", (event, message) => {
// console.log(`[${now()}] on 'test.reply' [message: ${message}] (expected message: reply.${msg})`);
// });
console.log(`[${now()}] send 'test' [message: ${msg}] to worker`);
ipcRenderer.sendTo(otherWebContentsId, "test", msg);
}
if (winName == "worker") {
console.log(`[${now()}] load preload in worker (${webContentsId})`);
console.log(`[${now()}] listen 'test'`);
ipcRenderer.on("test", (event, message) => {
console.log(`[${now()}] on 'test' [message: ${message}], reply to ui`);
ipcRenderer.sendTo(event.senderId, "test.reply", `reply.${message}`);
// event.sender.sendTo(event.senderId, "test.reply", `reply.${message}`);
});
} else if (winName == "ui") {
console.log(`[${now()}] load preload in ui (${webContentsId})`);
ipcRenderer.on("test.reply", (event, message) => {
console.log(`[${now()}] on 'test.reply' [message: ${message}]`);
});
//sendToOtherWin("LOADING");
window.addEventListener("load", () => {
let button = document.createElement("button");
button.innerHTML = "send";
button.onclick = () => {
sendToOtherWin("LOADED");
};
document.body.appendChild(button);
});
}
console.log(`[${now()}] loaded`);I keep on thinking, and of course Marshall is the better judge here because he's actually developing in the codebase, but my thought is that you need to communicate between the browser windows through your main file. My thought was something like this: Each browser window maintains it's own state, (ie. when it is "ready"), and when it is, each browser window can send an event back to the main process. The main process can keep track when both browser windows are ready and perform any logic you were thinking that should happen. This code is very rough but I hope it gives you an idea. preload.js (probably can be the same preload for both BrowserWindows) const {
remote,
ipcRenderer
} = require("electron");
const winName = process.argv.pop();
ipcRenderer.on("receive", (event, args) => {
console.log(`Received message from ${args.name}: '${args.message}'.`);
if (typeof args.callback === "function") args.callback();
});
window.api = {
"sendInformation": function(data){
ipcRenderer.send("send", {
name: winName,
message: data
});
}
};index.html <!DOCTYPE html>
<html lang="en-US">
<head>
<title>Title</title>
<meta http-equiv="Content-Security-Policy" content="default-src 'none'">
</head>
<body>
<script>
// Send data to main process
window.api.sendInformation("data");
</script>
</body>
</html>main const {
app,
BrowserWindow,
ipcMain
} = require("electron");
const path = require("path");
let workerWin;
let uiWin;
let windows = ["worker", "ui"];
app.on("ready", async () => {
workerWin = new BrowserWindow({
height: 600,
width: 800,
title: "Worker",
webPreferences: {
preload: path.join(__dirname, "./preload.js"),
additionalArguments: [windows[0]]
},
});
await workerWin.loadFile(path.join(__dirname, "./index.html"));
workerWin.webContents.openDevTools();
uiWin = new BrowserWindow({
height: 600,
width: 800,
title: "UI",
webPreferences: {
preload: path.join(__dirname, "./preload.js"),
additionalArguments: [windows[1]]
},
});
await uiWin.loadFile(path.join(__dirname, "./index.html"));
uiWin.webContents.openDevTools();
});
ipcMain.on("send", (event, data) => {
// find other window to send the event to
let other = windows[0] !== data.name ? windows[0] : windows[1];
if (other === "worker"){
workerWin.webContents.send("receive", {
name: "ui",
message: data
});
} else {
uiWin.webContents.send("receive", {
name: "worker",
message: data
});
}
}); |
|
Closing due to lack of activity |
this worked! thank you. |
Thanks for sharing this @MarshallOfSound. I just found it while looking into a similar issue. This seems to solve my problem but, now I'm wondering, wouldn't sending a callback to the preload script potentially give access to Electron and node APIs to a hijacker? Also, if I'm not mistaken there's a memory leak here right?: onResponse: (fn) => {
// Deliberately strip event as it includes `sender`
ipcRenderer.on('response', (event, ...args) => fn(...args));
}The event handler is being created over and over and never cleaned up. |
|
In an electron-react-typescript-webpack app I'm trying to make two windows communicate. preload.ts : index.d.ts : main.ts : renderer.ts: app/components/App.tsx : } export default hot(module)(App); I get this error: electron_WEBPACK_IMPORTED_MODULE_0__.BrowserWindow is not a constructor
These are the specs of the API Objects: https://www.electronjs.org/docs/all#api-objects The lines that cause this error seems to be this line in preload.ts : together with this line in renderer.ts: How to solve the error? |
|
Thanks @MarshallOfSound, this worked! |
|
@MarshallOfSound do you have any advice for how to remove an individual listener from a channel with this pattern, since it's an anonymous function that's being registered? Or is there any good workaround to avoid increasing the attack surface? I'm having trouble formulating a reasonable alternative. contextBridge.exposeInMainWorld('myapi', {
request: (data) => ipcRenderer.send("request", {
data,
}),
onResponse: (fn) => {
// Deliberately strip event as it includes `sender`
ipcRenderer.on('response', (event, ...args) => fn(...args)); // How could this listener be removed without a function name?
}
} |
|
I guess we could do something like this but it seems likely to quickly get out of hand. contextBridge.exposeInMainWorld('myapi', {
request: (data) => ipcRenderer.send("request", {
data,
}),
onResponse: (fn) => {
const saferFn = (event, ...args) => fn(...args)
// Deliberately strip event as it includes `sender`
ipcRenderer.on('response', saferFn);
return saferFn; // Return the newly created function so the user can store it somewhere and later remove it.
}
}Unfortunately storing these extra references would quickly make the code quite convoluted. |
|
@slapbox Unfortunately that wouldn't work as every time a function is passed over the bridge a new proxy function is made. i.e. function A (world 1) --> function B (world 2) It's a known inefficiency of the contextBridge. How I'd do this is by using some kind of token system. const listeners = {};
contextBridge.exposeInMainWorld('myapi', {
request: (data) => ipcRenderer.send("request", {
data,
}),
onResponse: (fn) => {
const saferFn = (event, ...args) => fn(...args)
// Deliberately strip event as it includes `sender`
ipcRenderer.on('response', saferFn);
const key = symbol();
listeners[key] = saferFn;
return key;
},
removeResponseHandler: (key) => {
const fn = listeners[key];
delete listeners[key];
ipcRenderer.removeListener('response', fn);
}
}You can always make a pretty helper for this to make the usage for multiple IPC events much cleaner |
|
@MarshallOfSound thanks so much for letting me know that what I'd outlined wouldn't work! The extra logic required is unfortunate, but it makes sense; security doesn't come free. |
|
Here is another simple way to avoid the problem of not being able to remove the listener because of that function references issues: |
Thanks for this @shumeiko, really helped me. Would you say this is a good way to generalise your approach, also stripping away the |
This comment was marked as abuse.
This comment was marked as abuse.
|
2022 simple and functional //main.js
ipcMain.on('messageFromRenderer', (event, arg) => {
console.log(arg); // Affiche "Hello world !" dans la console principale
event.reply('responseToRenderer', 'Message reçu !'); // Répond à l'ipcRenderer
});//index.html
...
<WebView id='frame' preload="./js/preload.js" src='URL' nodeintegration spellcheck=1></WebView>
...// preload.js
const { ipcRenderer } = require('electron');
ipcRenderer.send('messageFromRenderer', 'Hello world !'); // Envoie un message à l'ipcMain
ipcRenderer.on('responseToRenderer', (event, arg) => {
console.log(arg); // Affiche "Message reçu !" dans la console de la WebView
}); |
|
version controle button in index.html //index.html
...
<WebView id='frame' preload="./js/preload.js" src='URL' nodeintegration spellcheck=1></WebView>
<button onclick="ipcaction()">Button action</button>
...
<script>
const ipc = require("electron").ipcRenderer;
function ipcaction() {
console.log("ipcaction");
ipc.send('messageFromMain', 'Hello world 555 !');
}
</script>//main.js
const { app, BrowserWindow, ipcMain } = require('electron');
const ipc = require("electron").ipcMain;
let mainWindowWebContents = null;
app.on('ready', () => {
const mainWindow = new BrowserWindow({ width: 800, height: 600, webPreferences: { nodeIntegration: true } })
mainWindow.loadFile('index.html')
ipc.on("messageFromMain", event => {
console.log('messageFromMain');
sendToMainWindow('Hello from main.js W!');
});
})
function sendToMainWindow(message) {
if (mainWindowWebContents) {
mainWindowWebContents.send('messageFromMain', message)
}
}
app.on("web-contents-created", (event, webContents) => {
webContents.on('did-finish-load', () => {
if (webContents.getType() === 'webview') {
mainWindowWebContents = webContents;
sendToMainWindow('Hello from main.js X!');
}
});
});//preload.js :::: WebView id='frame' preload="./js/preload.js"
window.addEventListener('load', () => {
const { ipcRenderer } = require('electron')
ipcRenderer.on('messageFromMain', (event, arg) => {
console.log(arg) // affiche "Hello world 555 !"
})
}); |
We need to handle the `off` function differently due to issues with contextBridge. It turns out that `cb` will be a *copy* of the original function, hence a naive use of removeListener won't actually unlisten. See electron/electron#21437 (comment)
Preflight Checklist
Issue Details
Expected Behavior
In
preload.js,ipcRenderer.oncan work when script loading.Actual Behavior
In
preload.js,ipcRenderer.ondoes not work while script loaded.To Reproduce
https://gist.github.com/8beadb34f2123ee942968494edf439d0
Screenshots
The text was updated successfully, but these errors were encountered: