Skip to content

Commit 1ccf744

Browse files
hoxyqAndyPengc12
hoxyq
authored and
AndyPengc12
committed
refactor[devtools/extension]: refactored messaging logic across different parts of the extension (facebook#27417)
1. bvaughn@9fc04ea#diff-2c5e1f5e80e74154e65b2813cf1c3638f85034530e99dae24809ab4ad70d0143 introduced a vulnerability: we listen to `'fetch-file-with-cache'` event from `window` to fetch sources of the file, in which we want to parse hook names. We send this event via `window`, which means any page can also use this and manipulate the extension to perform some `fetch()` calls. With these changes, instead of transporting message via `window`, we have a distinct content script, which is responsible for fetching sources. It is notified via `chrome.runtime.sendMessage` api, so it can't be manipulated. 2. Consistent structure of messages `{source: string, payload: object}` in different parts of the extension 3. Added some wrappers around `chrome.scripting.executeScript` API in `packages/react-devtools-extensions/src/background/executeScript.js`, which support custom flow for Firefox, to simulate support of `ExecutionWorld.MAIN`.
1 parent 8053d6b commit 1ccf744

File tree

14 files changed

+406
-291
lines changed

14 files changed

+406
-291
lines changed

‎packages/react-devtools-extensions/src/background/dynamicallyInjectContentScripts.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ const contentScriptsToInject = IS_FIREFOX
1313
persistAcrossSessions: true,
1414
runAt: 'document_end',
1515
},
16+
{
17+
id: '@react-devtools/file-fetcher',
18+
js: ['build/fileFetcher.js'],
19+
matches: ['<all_urls>'],
20+
persistAcrossSessions: true,
21+
runAt: 'document_end',
22+
},
1623
]
1724
: [
1825
{
@@ -23,6 +30,14 @@ const contentScriptsToInject = IS_FIREFOX
2330
runAt: 'document_end',
2431
world: chrome.scripting.ExecutionWorld.ISOLATED,
2532
},
33+
{
34+
id: '@react-devtools/file-fetcher',
35+
js: ['build/fileFetcher.js'],
36+
matches: ['<all_urls>'],
37+
persistAcrossSessions: true,
38+
runAt: 'document_end',
39+
world: chrome.scripting.ExecutionWorld.ISOLATED,
40+
},
2641
{
2742
id: '@react-devtools/hook',
2843
js: ['build/installHook.js'],

‎packages/react-devtools-extensions/src/background/executeScript.js‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/* global chrome */
2+
3+
import {IS_FIREFOX} from '../utils';
4+
5+
// Firefox doesn't support ExecutionWorld.MAIN yet
6+
// https://bugzilla.mozilla.org/show_bug.cgi?id=1736575
7+
function executeScriptForFirefoxInMainWorld({target, files}) {
8+
return chrome.scripting.executeScript({
9+
target,
10+
func: fileNames => {
11+
function injectScriptSync(src) {
12+
let code = '';
13+
const request = new XMLHttpRequest();
14+
request.addEventListener('load', function () {
15+
code = this.responseText;
16+
});
17+
request.open('GET', src, false);
18+
request.send();
19+
20+
const script = document.createElement('script');
21+
script.textContent = code;
22+
23+
// This script runs before the <head> element is created,
24+
// so we add the script to <html> instead.
25+
if (document.documentElement) {
26+
document.documentElement.appendChild(script);
27+
}
28+
29+
if (script.parentNode) {
30+
script.parentNode.removeChild(script);
31+
}
32+
}
33+
34+
fileNames.forEach(file => injectScriptSync(chrome.runtime.getURL(file)));
35+
},
36+
args: [files],
37+
});
38+
}
39+
40+
export function executeScriptInIsolatedWorld({target, files}) {
41+
return chrome.scripting.executeScript({
42+
target,
43+
files,
44+
world: chrome.scripting.ExecutionWorld.ISOLATED,
45+
});
46+
}
47+
48+
export function executeScriptInMainWorld({target, files}) {
49+
if (IS_FIREFOX) {
50+
return executeScriptForFirefoxInMainWorld({target, files});
51+
}
52+
53+
return chrome.scripting.executeScript({
54+
target,
55+
files,
56+
world: chrome.scripting.ExecutionWorld.MAIN,
57+
});
58+
}

‎packages/react-devtools-extensions/src/background/index.js‎

Lines changed: 21 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@
22

33
'use strict';
44

5-
import {IS_FIREFOX, EXTENSION_CONTAINED_VERSIONS} from '../utils';
6-
75
import './dynamicallyInjectContentScripts';
86
import './tabsManager';
9-
import setExtensionIconAndPopup from './setExtensionIconAndPopup';
7+
8+
import {
9+
handleDevToolsPageMessage,
10+
handleBackendManagerMessage,
11+
handleReactDevToolsHookMessage,
12+
handleFetchResourceContentScriptMessage,
13+
} from './messageHandlers';
1014

1115
/*
1216
{
@@ -173,67 +177,21 @@ function connectExtensionAndProxyPorts(extensionPort, proxyPort, tabId) {
173177
}
174178

175179
chrome.runtime.onMessage.addListener((message, sender) => {
176-
const tab = sender.tab;
177-
// sender.tab.id from content script points to the tab that injected the content script
178-
if (tab) {
179-
const id = tab.id;
180-
// This is sent from the hook content script.
181-
// It tells us a renderer has attached.
182-
if (message.hasDetectedReact) {
183-
setExtensionIconAndPopup(message.reactBuildType, id);
184-
} else {
185-
const extensionPort = ports[id]?.extension;
186-
187-
switch (message.payload?.type) {
188-
case 'fetch-file-with-cache-complete':
189-
case 'fetch-file-with-cache-error':
190-
// Forward the result of fetch-in-page requests back to the extension.
191-
extensionPort?.postMessage(message);
192-
break;
193-
// This is sent from the backend manager running on a page
194-
case 'react-devtools-required-backends':
195-
const backendsToDownload = [];
196-
message.payload.versions.forEach(version => {
197-
if (EXTENSION_CONTAINED_VERSIONS.includes(version)) {
198-
if (!IS_FIREFOX) {
199-
// equivalent logic for Firefox is in prepareInjection.js
200-
chrome.scripting.executeScript({
201-
target: {tabId: id},
202-
files: [`/build/react_devtools_backend_${version}.js`],
203-
world: chrome.scripting.ExecutionWorld.MAIN,
204-
});
205-
}
206-
} else {
207-
backendsToDownload.push(version);
208-
}
209-
});
210-
211-
// Request the necessary backends in the extension DevTools UI
212-
// TODO: handle this message in index.js to build the UI
213-
extensionPort?.postMessage({
214-
payload: {
215-
type: 'react-devtools-additional-backends',
216-
versions: backendsToDownload,
217-
},
218-
});
219-
break;
220-
}
180+
switch (message?.source) {
181+
case 'devtools-page': {
182+
handleDevToolsPageMessage(message);
183+
break;
221184
}
222-
}
223-
224-
// This is sent from the devtools page when it is ready for injecting the backend
225-
if (message?.payload?.type === 'react-devtools-inject-backend-manager') {
226-
// sender.tab.id from devtools page may not exist, or point to the undocked devtools window
227-
// so we use the payload to get the tab id
228-
const tabId = message.payload.tabId;
229-
230-
if (tabId && !IS_FIREFOX) {
231-
// equivalent logic for Firefox is in prepareInjection.js
232-
chrome.scripting.executeScript({
233-
target: {tabId},
234-
files: ['/build/backendManager.js'],
235-
world: chrome.scripting.ExecutionWorld.MAIN,
236-
});
185+
case 'react-devtools-fetch-resource-content-script': {
186+
handleFetchResourceContentScriptMessage(message);
187+
break;
188+
}
189+
case 'react-devtools-backend-manager': {
190+
handleBackendManagerMessage(message, sender);
191+
break;
192+
}
193+
case 'react-devtools-hook': {
194+
handleReactDevToolsHookMessage(message, sender);
237195
}
238196
}
239197
});

‎packages/react-devtools-extensions/src/background/injectProxy.js‎

Lines changed: 0 additions & 12 deletions
This file was deleted.

‎packages/react-devtools-extensions/src/background/messageHandlers.js‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/* global chrome */
2+
3+
import setExtensionIconAndPopup from './setExtensionIconAndPopup';
4+
import {executeScriptInMainWorld} from './executeScript';
5+
6+
import {EXTENSION_CONTAINED_VERSIONS} from '../utils';
7+
8+
export function handleReactDevToolsHookMessage(message, sender) {
9+
const {payload} = message;
10+
11+
switch (payload?.type) {
12+
case 'react-renderer-attached': {
13+
setExtensionIconAndPopup(payload.reactBuildType, sender.tab.id);
14+
15+
break;
16+
}
17+
}
18+
}
19+
20+
export function handleBackendManagerMessage(message, sender) {
21+
const {payload} = message;
22+
23+
switch (payload?.type) {
24+
case 'require-backends': {
25+
payload.versions.forEach(version => {
26+
if (EXTENSION_CONTAINED_VERSIONS.includes(version)) {
27+
executeScriptInMainWorld({
28+
target: {tabId: sender.tab.id},
29+
files: [`/build/react_devtools_backend_${version}.js`],
30+
});
31+
}
32+
});
33+
34+
break;
35+
}
36+
}
37+
}
38+
39+
export function handleDevToolsPageMessage(message) {
40+
const {payload} = message;
41+
42+
switch (payload?.type) {
43+
// Proxy this message from DevTools page to content script via chrome.tabs.sendMessage
44+
case 'fetch-file-with-cache': {
45+
const {
46+
payload: {tabId, url},
47+
} = message;
48+
49+
if (!tabId) {
50+
throw new Error("Couldn't fetch file sources: tabId not specified");
51+
}
52+
53+
if (!url) {
54+
throw new Error("Couldn't fetch file sources: url not specified");
55+
}
56+
57+
chrome.tabs.sendMessage(tabId, {
58+
source: 'devtools-page',
59+
payload: {
60+
type: 'fetch-file-with-cache',
61+
url,
62+
},
63+
});
64+
65+
break;
66+
}
67+
68+
case 'inject-backend-manager': {
69+
const {
70+
payload: {tabId},
71+
} = message;
72+
73+
if (!tabId) {
74+
throw new Error("Couldn't inject backend manager: tabId not specified");
75+
}
76+
77+
executeScriptInMainWorld({
78+
target: {tabId},
79+
files: ['/build/backendManager.js'],
80+
});
81+
82+
break;
83+
}
84+
}
85+
}
86+
87+
export function handleFetchResourceContentScriptMessage(message) {
88+
const {payload} = message;
89+
90+
switch (payload?.type) {
91+
case 'fetch-file-with-cache-complete':
92+
case 'fetch-file-with-cache-error':
93+
// Forward the result of fetch-in-page requests back to the DevTools page.
94+
// We switch the source here because of inconsistency between Firefox and Chrome
95+
// In Chromium this message will be propagated from content script to DevTools page
96+
// For Firefox, only background script will get this message, so we need to forward it to DevTools page
97+
chrome.runtime.sendMessage({
98+
source: 'react-devtools-background',
99+
payload,
100+
});
101+
break;
102+
}
103+
}

‎packages/react-devtools-extensions/src/contentScripts/backendManager.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ function updateRequiredBackends() {
170170
{
171171
source: 'react-devtools-backend-manager',
172172
payload: {
173-
type: 'react-devtools-required-backends',
173+
type: 'require-backends',
174174
versions: Array.from(requiredBackends),
175175
},
176176
},

‎packages/react-devtools-extensions/src/contentScripts/fileFetcher.js‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/* global chrome */
2+
3+
function fetchResource(url) {
4+
const reject = value => {
5+
chrome.runtime.sendMessage({
6+
source: 'react-devtools-fetch-resource-content-script',
7+
payload: {
8+
type: 'fetch-file-with-cache-error',
9+
url,
10+
value,
11+
},
12+
});
13+
};
14+
15+
const resolve = value => {
16+
chrome.runtime.sendMessage({
17+
source: 'react-devtools-fetch-resource-content-script',
18+
payload: {
19+
type: 'fetch-file-with-cache-complete',
20+
url,
21+
value,
22+
},
23+
});
24+
};
25+
26+
fetch(url, {cache: 'force-cache'}).then(
27+
response => {
28+
if (response.ok) {
29+
response
30+
.text()
31+
.then(text => resolve(text))
32+
.catch(error => reject(null));
33+
} else {
34+
reject(null);
35+
}
36+
},
37+
error => reject(null),
38+
);
39+
}
40+
41+
chrome.runtime.onMessage.addListener(message => {
42+
if (
43+
message?.source === 'devtools-page' &&
44+
message?.payload?.type === 'fetch-file-with-cache'
45+
) {
46+
fetchResource(message.payload.url);
47+
}
48+
});

‎packages/react-devtools-extensions/src/contentScripts/installHook.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,11 @@ if (!window.hasOwnProperty('__REACT_DEVTOOLS_GLOBAL_HOOK__')) {
1010
function ({reactBuildType}) {
1111
window.postMessage(
1212
{
13-
source: 'react-devtools-detector',
14-
reactBuildType,
13+
source: 'react-devtools-hook',
14+
payload: {
15+
type: 'react-renderer-attached',
16+
reactBuildType,
17+
},
1518
},
1619
'*',
1720
);

0 commit comments

Comments
 (0)