Skip to content

Commit 7b0749d

Browse files
committed
fix(native-bridge): address PR review feedback
- Extract BridgeState/BridgeStatus to shared/types/common/nativeBridge.ts - Dynamic permission check via chrome.permissions.contains() in getStatus() - Port race safety: capture currentPort in connect(), guard all listeners - Hello handshake error handling with try/catch and .catch() - Validate boolean input in nativeBridgeSetEnabled - Remove getExtStorage/getLogger from bridge allowlist (security) - getSiteList: include isDead from site metadata, fallback name to id - getDownloaderList: remove address field (may contain credentials) - Browser detection via UA for setup command (chrome/chromium/edge/firefox)
1 parent 2161d96 commit 7b0749d

File tree

7 files changed

+110
-85
lines changed

7 files changed

+110
-85
lines changed

src/entries/background/utils/nativeMessaging.ts

Lines changed: 70 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { onMessage, sendMessage } from "@/messages.ts";
22
import { setupOffscreenDocument } from "./offscreen.ts";
3+
import type { BridgeState, BridgeStatus } from "@/shared/types.ts";
34

45
const NATIVE_HOST_NAME = "com.ptd.native";
56
const INSTANCE_ID_KEY = "ptd_native_instance_id";
@@ -16,9 +17,6 @@ const FATAL_ERRORS = [
1617

1718
/** Methods the bridge will proxy to sendMessage(). Everything else is rejected. */
1819
const ALLOWED_METHODS = new Set([
19-
// Storage and logging (read-only)
20-
"getExtStorage",
21-
"getLogger",
2220
// Site config
2321
"getSiteList",
2422
"getSiteUserConfig",
@@ -54,12 +52,10 @@ const ALLOWED_METHODS = new Set([
5452
]);
5553

5654
// ── Module-scoped state ──────────────────────────────────────────────
57-
type BridgeState = "no-permission" | "disabled" | "connecting" | "connected" | "retrying" | "error";
5855

5956
let port: chrome.runtime.Port | null = null;
6057
let reconnectTimer: ReturnType<typeof setTimeout> | null = null;
6158
let reconnectAttempt = 0;
62-
let permissionGranted = false;
6359
let enabled = true;
6460
let state: BridgeState = "no-permission";
6561
let lastError: string | undefined;
@@ -79,12 +75,21 @@ async function getOrCreateInstanceId(): Promise<string> {
7975
return id;
8076
}
8177

82-
function getStatus() {
78+
async function checkPermission(): Promise<boolean> {
79+
try {
80+
return await chrome.permissions.contains({ permissions: ["nativeMessaging"] });
81+
} catch {
82+
return false;
83+
}
84+
}
85+
86+
async function getStatus(): Promise<BridgeStatus> {
87+
const permissionGranted = await checkPermission();
8388
return {
8489
permissionGranted,
8590
enabled,
8691
state,
87-
connected: state === "connected",
92+
connected: port !== null && state === "connected",
8893
lastError,
8994
};
9095
}
@@ -133,7 +138,7 @@ function scheduleReconnect() {
133138
}
134139

135140
function connect() {
136-
if (!permissionGranted || !enabled) {
141+
if (!enabled) {
137142
return;
138143
}
139144

@@ -142,8 +147,10 @@ function connect() {
142147
lastError = undefined;
143148
intentionalDisconnect = false;
144149

150+
let currentPort: chrome.runtime.Port;
145151
try {
146-
port = chrome.runtime.connectNative(NATIVE_HOST_NAME);
152+
currentPort = chrome.runtime.connectNative(NATIVE_HOST_NAME);
153+
port = currentPort;
147154
} catch (e: any) {
148155
state = "error";
149156
lastError = e?.message ?? String(e);
@@ -154,29 +161,50 @@ function connect() {
154161
// Send hello handshake — mark connected after successful send.
155162
// The native host does not send an ack, so a successful postMessage
156163
// is our best signal. If the host is absent, onDisconnect fires.
157-
getOrCreateInstanceId().then((instanceId) => {
158-
if (!port) return;
159-
port.postMessage({
160-
type: "hello",
161-
instanceId,
162-
browser: __BROWSER__,
163-
extensionId: chrome.runtime.id,
164-
version: __EXT_VERSION__,
165-
capabilities: ["bridge-v1"],
164+
getOrCreateInstanceId()
165+
.then((instanceId) => {
166+
if (port !== currentPort) return;
167+
168+
try {
169+
currentPort.postMessage({
170+
type: "hello",
171+
instanceId,
172+
browser: __BROWSER__,
173+
extensionId: chrome.runtime.id,
174+
version: __EXT_VERSION__,
175+
capabilities: ["bridge-v1"],
176+
});
177+
} catch (e: any) {
178+
state = "error";
179+
lastError = e?.message ?? String(e);
180+
console.debug("[PTD] Failed to send native hello message:", lastError);
181+
return;
182+
}
183+
184+
state = "connected";
185+
reconnectAttempt = 0;
186+
})
187+
.catch((e: any) => {
188+
if (port !== currentPort) return;
189+
state = "error";
190+
lastError = e?.message ?? String(e);
191+
console.debug("[PTD] Failed to get or create native instance id:", lastError);
192+
try {
193+
currentPort.disconnect();
194+
} catch {
195+
// ignore
196+
}
166197
});
167-
state = "connected";
168-
reconnectAttempt = 0;
169-
});
170198

171-
port.onMessage.addListener(async (msg: any) => {
199+
currentPort.onMessage.addListener(async (msg: any) => {
172200
if (msg?.type !== "request" || !msg.id || !msg.method) {
173201
return;
174202
}
175203

176204
const { id, method, params } = msg;
177205

178206
if (!ALLOWED_METHODS.has(method)) {
179-
port!.postMessage({
207+
currentPort.postMessage({
180208
type: "response",
181209
id,
182210
error: { code: "METHOD_NOT_ALLOWED", message: `Method '${method}' is not allowed` },
@@ -187,58 +215,56 @@ function connect() {
187215
try {
188216
await setupOffscreenDocument();
189217
const result = await sendMessage(method as any, params);
190-
port?.postMessage({ type: "response", id, result });
218+
if (port === currentPort) {
219+
currentPort.postMessage({ type: "response", id, result });
220+
}
191221
} catch (e: any) {
192-
port?.postMessage({
193-
type: "response",
194-
id,
195-
error: { code: "EXTENSION_ERROR", message: e?.message ?? String(e) },
196-
});
222+
if (port === currentPort) {
223+
currentPort.postMessage({
224+
type: "response",
225+
id,
226+
error: { code: "EXTENSION_ERROR", message: e?.message ?? String(e) },
227+
});
228+
}
197229
}
198230
});
199231

200-
port.onDisconnect.addListener(() => {
232+
currentPort.onDisconnect.addListener(() => {
201233
const err = chrome.runtime.lastError;
202234
const errMsg = err?.message ?? "";
203235

236+
// Stale port — a new connection has already replaced this one
237+
if (port !== currentPort) return;
238+
204239
port = null;
205240

206241
if (intentionalDisconnect) {
207-
// Intentional disconnect — don't retry
208242
return;
209243
}
210244

211245
if (err) {
212246
console.debug("[PTD] Native messaging disconnected:", errMsg);
213247
}
214248

215-
// Fatal errors — no retry
216249
if (FATAL_ERRORS.some((e) => errMsg.includes(e))) {
217250
state = "error";
218251
lastError = errMsg;
219252
console.debug("[PTD] Native host not available, CLI bridge disabled.");
220253
return;
221254
}
222255

223-
// Recoverable (e.g. host exited) — retry with backoff
224256
lastError = errMsg || "Connection lost";
225257
scheduleReconnect();
226258
});
227259
}
228260

229261
async function init() {
230-
// Refresh permission state
231-
try {
232-
permissionGranted = await chrome.permissions.contains({ permissions: ["nativeMessaging"] });
233-
} catch {
234-
permissionGranted = false;
235-
}
262+
const permissionGranted = await checkPermission();
236263

237264
// Refresh enabled flag
238265
const stored = await chrome.storage.local.get(ENABLED_KEY);
239266
enabled = stored[ENABLED_KEY] !== false; // default true
240267

241-
// Reconcile desired state
242268
if (!permissionGranted) {
243269
disconnect(true);
244270
state = "no-permission";
@@ -253,7 +279,6 @@ async function init() {
253279
return;
254280
}
255281

256-
// Permission granted and enabled — connect
257282
connect();
258283
}
259284

@@ -278,16 +303,19 @@ onMessage("nativeBridgeGetStatus", async () => {
278303
});
279304

280305
onMessage("nativeBridgeSetEnabled", async ({ data }) => {
306+
if (typeof data !== "boolean") {
307+
return getStatus();
308+
}
281309
await chrome.storage.local.set({ [ENABLED_KEY]: data });
282310
await init();
283-
// Brief wait for the async hello handshake to complete
284311
if (state === "connecting") {
285312
await new Promise((r) => setTimeout(r, 200));
286313
}
287314
return getStatus();
288315
});
289316

290317
onMessage("nativeBridgeReconnect", async () => {
318+
const permissionGranted = await checkPermission();
291319
if (!permissionGranted) {
292320
lastError = "Permission not granted — cannot reconnect";
293321
return getStatus();

src/entries/messages.ts

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import {
4040
AugmentedRequired,
4141
IKeepUploadTask,
4242
TKeepUploadTaskKey,
43+
BridgeStatus,
4344
} from "@/shared/types.ts";
4445

4546
import { isDebug } from "~/helper.ts";
@@ -148,30 +149,12 @@ interface ProtocolMap extends TMessageMap {
148149

149150
// 2.8 Lightweight list queries (for CLI discovery)
150151
getSiteList(): Array<{ id: string; name: string; url: string; offline: boolean }>;
151-
getDownloaderList(): Array<{ id: string; name: string; type: string; enabled: boolean; address: string }>;
152+
getDownloaderList(): Array<{ id: string; name: string; type: string; enabled: boolean }>;
152153

153154
// 2.9 Native messaging bridge control
154-
nativeBridgeGetStatus(): {
155-
permissionGranted: boolean;
156-
enabled: boolean;
157-
state: "no-permission" | "disabled" | "connecting" | "connected" | "retrying" | "error";
158-
connected: boolean;
159-
lastError?: string;
160-
};
161-
nativeBridgeSetEnabled(data: boolean): {
162-
permissionGranted: boolean;
163-
enabled: boolean;
164-
state: "no-permission" | "disabled" | "connecting" | "connected" | "retrying" | "error";
165-
connected: boolean;
166-
lastError?: string;
167-
};
168-
nativeBridgeReconnect(): {
169-
permissionGranted: boolean;
170-
enabled: boolean;
171-
state: "no-permission" | "disabled" | "connecting" | "connected" | "retrying" | "error";
172-
connected: boolean;
173-
lastError?: string;
174-
};
155+
nativeBridgeGetStatus(): BridgeStatus;
156+
nativeBridgeSetEnabled(data: boolean): BridgeStatus;
157+
nativeBridgeReconnect(): BridgeStatus;
175158
}
176159

177160
// 全局消息处理函数映射

src/entries/offscreen/utils/download.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ onMessage("getDownloaderList", async () => {
4949
name: config.name ?? "",
5050
type: config.type ?? "",
5151
enabled: config.enabled ?? false,
52-
address: config.address ?? "",
5352
}));
5453
});
5554

src/entries/offscreen/utils/site.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,18 @@ onMessage("getSiteList", async () => {
5757
const metadata = (await sendMessage("getExtStorage", "metadata")) as IMetadataPiniaStorageSchema;
5858
const sites = metadata?.sites ?? {};
5959
const nameMap = metadata?.siteNameMap ?? {};
60-
return Object.entries(sites).map(([id, config]) => ({
61-
id,
62-
name: nameMap[id] ?? "",
63-
url: config.url ?? "",
64-
offline: config.isOffline ?? false,
65-
}));
60+
return Promise.all(
61+
Object.entries(sites).map(async ([id, config]) => {
62+
const siteMetaData = await getDefinedSiteMetadata(id as TSiteID);
63+
const isDead = siteMetaData.isDead ?? false;
64+
return {
65+
id,
66+
name: nameMap[id] ?? config.merge?.name ?? id,
67+
url: config.url ?? "",
68+
offline: (isDead || config.isOffline) ?? false,
69+
};
70+
}),
71+
);
6672
});
6773

6874
export async function getSiteInstance<TYPE extends "private" | "public">(

src/entries/options/views/Settings/SetBase/NativeBridgeWindow.vue

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,28 @@ import { computed, onMounted, ref, shallowRef } from "vue";
33
import { useI18n } from "vue-i18n";
44
55
import { sendMessage } from "@/messages.ts";
6+
import type { BridgeState, BridgeStatus } from "@/shared/types.ts";
67
78
const { t } = useI18n();
89
910
const extensionId = chrome.runtime.id;
11+
12+
function detectBrowserFamily(): string {
13+
if (__BROWSER__ === "firefox") return "firefox";
14+
const ua = navigator.userAgent;
15+
if (ua.includes("Edg/")) return "edge";
16+
if (ua.includes("Chromium/")) return "chromium";
17+
return "chrome";
18+
}
19+
20+
const browserFamily = detectBrowserFamily();
1021
const setupCommand = computed(() => {
11-
if (__BROWSER__ === "firefox") {
22+
if (browserFamily === "firefox") {
1223
return "ptd install --browser firefox";
1324
}
14-
return `ptd install --browser chrome --extension-id ${extensionId}`;
25+
return `ptd install --browser ${browserFamily} --extension-id ${extensionId}`;
1526
});
1627
17-
type BridgeState = "no-permission" | "disabled" | "connecting" | "connected" | "retrying" | "error";
18-
19-
interface BridgeStatus {
20-
permissionGranted: boolean;
21-
enabled: boolean;
22-
state: BridgeState;
23-
connected: boolean;
24-
lastError?: string;
25-
}
26-
2728
const status = shallowRef<BridgeStatus>({
2829
permissionGranted: false,
2930
enabled: true,
@@ -48,7 +49,6 @@ async function grantPermission() {
4849
try {
4950
const granted = await chrome.permissions.request({ permissions: ["nativeMessaging"] });
5051
if (granted) {
51-
// Permission granted — refresh status (onAdded listener in background will call init())
5252
await refreshStatus();
5353
} else {
5454
console.debug("[PTD]", t("SetNativeBridge.permission.grantFailed"));
@@ -99,7 +99,6 @@ async function testConnection() {
9999
testLoading.value = true;
100100
try {
101101
status.value = await sendMessage("nativeBridgeReconnect", undefined);
102-
// Poll until state settles (connected/error) or timeout
103102
if (status.value.state === "connecting") {
104103
await waitForSettledState();
105104
}

src/entries/shared/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { TBackupFields } from "./types/storages/metadata.ts";
88
export * from "./types/extends.ts";
99
export * from "./types/common/download.ts";
1010
export * from "./types/common/ptpp.ts";
11+
export * from "./types/common/nativeBridge.ts";
1112
export * from "./types/storages/config.ts";
1213
export * from "./types/storages/indexdb.ts";
1314
export * from "./types/storages/metadata.ts";
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
export type BridgeState = "no-permission" | "disabled" | "connecting" | "connected" | "retrying" | "error";
2+
3+
export interface BridgeStatus {
4+
permissionGranted: boolean;
5+
enabled: boolean;
6+
state: BridgeState;
7+
connected: boolean;
8+
lastError?: string;
9+
}

0 commit comments

Comments
 (0)