Skip to content

Commit 2f1c6a3

Browse files
VendicatedVendicated
authored
fix many bugs found by Claude (#4436)
Fixes: - Webpack Patcher: incorrect patch error handling could lead to some functional patches not being applied - CommandsAPI: subcommands were impossible to unregister - useAwaiter: on error, the value was set to null even if a fallbackValue was specified, violating the type contract - Patcher: build check boolean logic was grouped incorrectly due to missing parantheses - CustomCommands: commands weren't unregistered on stop, leading to error when enabling, disabling & re-renabling the plugin - Notification Log: delete was by timestamp instead of ID and non-atomic - FixSpotifyEmbeds: settings listener was registered inside browser-window-created event handler, leading to duplicate listeners - CustomRPC: changing activity type or timestamp didn't update the RPC - CopyStickerLinks: message patch had non-sense code, just removed it - XSOverlay: UDP socket wasn't closed on stop - VoiceMessages: Web recorder never closed the mic media stream, which continues showing the OS mic indicator - PictureInPicture: Video clone wasn't properly cleaned up, which leaked memory - Themes (Extension version): theme object urls were never cleaned up, which leaked memory on theme change - Themes (Extension version): Upload Theme button hitbox was only the text, not the whole button. Also added Drag & Drop from file manager support - Dearrow: toggling for one embed flipped the global dearrow by default setting - Extension Installer: the zip extract function was vulnerable to path traversal, although we only ever used this for React DevTools obtained via https so it has low impact - FileWatchers: ensure always properly cleaned up - components/Link: mutated props which is a very bad practice All fixes are human written, only the audit was done by Claude Fable (thanks for ur tokens, Zere).
1 parent 73d8490 commit 2f1c6a3

29 files changed

Lines changed: 309 additions & 142 deletions

File tree

‎src/api/Badges.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export interface ProfileBadge {
4040
/** Action to perform when you click the badge */
4141
onClick?(event: React.MouseEvent, props: ProfileBadge & BadgeUserArgs): void;
4242
/** Action to perform when you right click the badge */
43-
onContextMenu?(event: React.MouseEvent, props: BadgeUserArgs & BadgeUserArgs): void;
43+
onContextMenu?(event: React.MouseEvent, props: ProfileBadge & BadgeUserArgs): void;
4444
/** Should the user display this badge? */
4545
shouldShow?(userInfo: BadgeUserArgs): boolean;
4646
/** Optional props (e.g. style) for the badge, ignored for component badges */

‎src/api/Commands/index.ts‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,14 @@ export let OptionalMessageOption: CommandOption = OptPlaceholder;
4646
*/
4747
export let RequiredMessageOption: CommandOption = ReqPlaceholder;
4848

49-
// Discord's command list has random gaps for some reason, which can cause issues while rendering the commands
50-
// Add this offset to every added command to keep them unique
51-
let commandIdOffset: number;
49+
let idCounter = 99;
5250

5351
export const _init = function (cmds: VencordCommand[]) {
5452
try {
5553
BUILT_IN = cmds;
5654
OptionalMessageOption = cmds.find(c => (c.untranslatedName || c.displayName) === "shrug")!.options![0];
5755
RequiredMessageOption = cmds.find(c => (c.untranslatedName || c.displayName) === "me")!.options![0];
58-
commandIdOffset = Math.abs(BUILT_IN.map(x => Number(x.id)).sort((x, y) => x - y)[0]) - BUILT_IN.length;
56+
idCounter = Math.abs(BUILT_IN.map(x => Number(x.id)).sort((x, y) => x - y)[0]) + 1;
5957
} catch (e) {
6058
new Logger("CommandsAPI").error("Failed to load CommandsApi", e, " - cmds is", cmds);
6159
}
@@ -107,32 +105,42 @@ export function prepareOption<O extends CommandOption | VencordCommand>(opt: O):
107105
return opt;
108106
}
109107

108+
const isSubCommandParent = (cmd: VencordCommand) => cmd.options?.[0]?.type === ApplicationCommandOptionType.SUB_COMMAND;
109+
const getSubCommandName = (cmd: VencordCommand, option: CommandOption) => `${cmd.name} ${option.name}`;
110+
110111
// Yes, Discord registers individual commands for each subcommand
111-
// TODO: This probably doesn't support nested subcommands. If that is ever needed,
112-
// investigate
113112
function registerSubCommands(cmd: VencordCommand, plugin: string) {
114113
cmd.options?.forEach(o => {
115114
if (o.type !== ApplicationCommandOptionType.SUB_COMMAND)
116115
throw new Error("When specifying sub-command options, all options must be sub-commands.");
116+
117117
const subCmd = {
118118
...cmd,
119119
...o,
120120
options: o.options !== undefined ? o.options : undefined,
121121
type: ApplicationCommandType.CHAT_INPUT,
122-
name: `${cmd.name} ${o.name}`,
123122
id: `${o.name}-${cmd.id}`,
124-
displayName: `${cmd.name} ${o.name}`,
123+
name: getSubCommandName(cmd, o),
124+
displayName: getSubCommandName(cmd, o),
125125
subCommandPath: [{
126126
name: o.name,
127127
type: o.type,
128128
displayName: o.name
129129
}],
130130
rootCommand: cmd
131131
};
132-
registerCommand(subCmd as any, plugin);
132+
registerCommand(subCmd, plugin);
133133
});
134134
}
135135

136+
function unregisterSubCommands(cmd: VencordCommand): boolean {
137+
const results = BUILT_IN
138+
.filter(c => c.rootCommand === cmd)
139+
.map(c => unregisterCommand(c.name));
140+
141+
return results.length > 0 && results.every(x => x);
142+
}
143+
136144
export function registerCommand<C extends VencordCommand>(command: C, plugin: string) {
137145
if (!BUILT_IN) {
138146
console.warn(
@@ -149,24 +157,30 @@ export function registerCommand<C extends VencordCommand>(command: C, plugin: st
149157
command.isVencordCommand = true;
150158
command.untranslatedName ??= command.name;
151159
command.untranslatedDescription ??= command.description;
152-
command.id ??= `-${BUILT_IN.length + commandIdOffset + 1}`;
160+
command.id ??= `-${idCounter++}`;
153161
command.applicationId ??= "-1"; // BUILT_IN;
154162
command.type ??= ApplicationCommandType.CHAT_INPUT;
155163
command.inputType ??= ApplicationCommandInputType.BUILT_IN_TEXT;
156164
command.plugin ||= plugin;
157165

158166
prepareOption(command);
167+
commands[command.name] = command;
159168

160-
if (command.options?.[0]?.type === ApplicationCommandOptionType.SUB_COMMAND) {
169+
if (isSubCommandParent(command)) {
161170
registerSubCommands(command, plugin);
162171
return;
163172
}
164173

165-
commands[command.name] = command;
166174
BUILT_IN.push(command);
167175
}
168176

169-
export function unregisterCommand(name: string) {
177+
export function unregisterCommand(name: string, isSubCommands = false) {
178+
const cmd = commands[name];
179+
if (cmd && isSubCommandParent(cmd)) {
180+
delete commands[name];
181+
return unregisterSubCommands(cmd);
182+
}
183+
170184
const idx = BUILT_IN.findIndex(c => c.name === name);
171185
if (idx === -1)
172186
return false;

‎src/api/Commands/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ export { ApplicationCommandInputType, ApplicationCommandOptionType, ApplicationC
99

1010
export interface VencordCommand extends Command {
1111
isVencordCommand?: boolean;
12+
rootCommand?: VencordCommand;
1213
}

‎src/api/Notifications/notificationLog.tsx‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import { openNotificationSettingsModal } from "@components/settings/tabs/vencord
2222
import { classNameFactory } from "@utils/css";
2323
import { useAwaiter } from "@utils/react";
2424
import { RenderModalProps } from "@vencord/discord-types";
25-
import { ConfirmModal, Forms, ListScrollerThin, Modal,openModal, React, Timestamp, useEffect, useReducer, useState } from "@webpack/common";
25+
import { ConfirmModal, Forms, ListScrollerThin, Modal, openModal, React, Timestamp, useEffect, useReducer, useState } from "@webpack/common";
2626
import { nanoid } from "nanoid";
2727
import type { DispatchWithoutAction } from "react";
2828

@@ -74,14 +74,21 @@ export async function persistNotification(notification: NotificationData) {
7474
signals.forEach(x => x());
7575
}
7676

77-
export async function deleteNotification(timestamp: number) {
78-
const log = await getLog();
79-
const index = log.findIndex(x => x.timestamp === timestamp);
80-
if (index === -1) return;
77+
export async function deleteNotification(id: string) {
78+
let found = false;
8179

82-
log.splice(index, 1);
83-
await DataStore.set(KEY, log);
84-
signals.forEach(x => x());
80+
await DataStore.update(KEY, (old: PersistentNotificationData[] | undefined) => {
81+
const log = old ?? [];
82+
const index = log.findIndex(x => x.id === id);
83+
if (index === -1) return log;
84+
85+
log.splice(index, 1);
86+
found = true;
87+
return log;
88+
});
89+
90+
if (found)
91+
signals.forEach(x => x());
8592
}
8693

8794
export function useLogs() {
@@ -113,7 +120,7 @@ function NotificationEntry({ data }: { data: PersistentNotificationData; }) {
113120
if (removing) return;
114121
setRemoving(true);
115122

116-
setTimeout(() => deleteNotification(data.timestamp), 200);
123+
setTimeout(() => deleteNotification(data.id), 200);
117124
}}
118125
richBody={
119126
<div className={cl("body-wrapper")}>

‎src/api/Themes.ts‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import { Settings, SettingsStore } from "@api/Settings";
2020
import { createAndAppendStyle } from "@utils/css";
21+
import { isNonNullish } from "@utils/guards";
2122
import { ThemeStore } from "@vencord/discord-types";
2223
import { PopoutWindowStore } from "@webpack/common";
2324

@@ -42,6 +43,9 @@ async function toggle(isEnabled: boolean) {
4243
style.disabled = !isEnabled;
4344
}
4445

46+
// for cleanup
47+
let previousThemeBlobObjectURLs = [] as string[];
48+
4549
async function initThemes() {
4650
themesStyle ??= createAndAppendStyle("vencord-themes", userStyleRootNode);
4751

@@ -66,12 +70,18 @@ async function initThemes() {
6670
.filter(link => link !== null);
6771

6872
if (IS_WEB) {
69-
for (const theme of enabledThemes) {
73+
previousThemeBlobObjectURLs.forEach(url => URL.revokeObjectURL(url));
74+
75+
const objectUrls = await Promise.all(enabledThemes.map(async theme => {
7076
const themeData = await VencordNative.themes.getThemeData(theme);
71-
if (!themeData) continue;
77+
if (!themeData) return null;
78+
7279
const blob = new Blob([themeData], { type: "text/css" });
73-
links.push(URL.createObjectURL(blob));
74-
}
80+
return URL.createObjectURL(blob);
81+
}));
82+
83+
previousThemeBlobObjectURLs = objectUrls.filter(isNonNullish);
84+
links.push(...previousThemeBlobObjectURLs);
7585
} else {
7686
const localThemes = enabledThemes.map(theme => `vencord:///themes/${theme}?v=${Date.now()}`);
7787
links.push(...localThemes);

‎src/components/Link.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ interface Props extends React.DetailedHTMLProps<React.AnchorHTMLAttributes<HTMLA
2323
}
2424

2525
export function Link(props: React.PropsWithChildren<Props>) {
26+
props = { ...props };
27+
2628
if (props.disabled) {
2729
props.style ??= {};
2830
props.style.pointerEvents = "none";

‎src/components/settings/QuickAction.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,14 @@ export interface QuickActionProps {
1717
text: ReactNode;
1818
action?: () => void;
1919
disabled?: boolean;
20+
style?: React.CSSProperties;
2021
}
2122

2223
export function QuickAction(props: QuickActionProps) {
23-
const { Icon, action, text, disabled } = props;
24+
const { Icon, action, text, disabled, style } = props;
2425

2526
return (
26-
<button className={cl("pill")} onClick={action} disabled={disabled}>
27+
<button className={cl("pill")} onClick={action} disabled={disabled} style={style}>
2728
<Icon className={cl("img")} />
2829
{text}
2930
</button>

‎src/components/settings/tabs/plugins/index.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import { Margins } from "@utils/margins";
3535
import { classes } from "@utils/misc";
3636
import { useAwaiter, useCleanupEffect } from "@utils/react";
3737
import { PluginTag, PluginTags } from "@utils/types";
38-
import { Button, ConfirmModal,lodash, openModal, Parser, React, SearchableSelect, Select, TextInput, Tooltip, useMemo, useRef, useState } from "@webpack/common";
38+
import { Button, ConfirmModal, lodash, openModal, Parser, React, SearchableSelect, Select, TextInput, Tooltip, useMemo, useRef, useState } from "@webpack/common";
3939
import { JSX } from "react";
4040

4141
import Plugins, { ExcludedPlugins, PluginMeta } from "~plugins";
@@ -133,10 +133,10 @@ function PluginSettings() {
133133
<>
134134
<p>The following plugins require a restart:</p>
135135
<div>{changes.map((s, i) => (
136-
<>
136+
<React.Fragment key={s}>
137137
{i > 0 && ", "}
138138
{Parser.parse("`" + s.split(".")[0] + "`")}
139-
</>
139+
</React.Fragment>
140140
))}</div>
141141
</>
142142
</ConfirmModal>

‎src/components/settings/tabs/themes/LocalThemesTab.tsx‎

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,7 @@ function onLocalThemeChange(fileName: string, value: boolean) {
4343
}
4444
}
4545

46-
async function onFileUpload(e: SyntheticEvent<HTMLInputElement>) {
47-
e.stopPropagation();
48-
e.preventDefault();
49-
50-
if (!e.currentTarget?.files?.length) return;
51-
const { files } = e.currentTarget;
52-
46+
async function doUploadThemes(files: ArrayLike<File>) {
5347
const uploads = Array.from(files, file => {
5448
const { name } = file;
5549
if (!name.endsWith(".css")) return;
@@ -68,6 +62,47 @@ async function onFileUpload(e: SyntheticEvent<HTMLInputElement>) {
6862
await Promise.all(uploads);
6963
}
7064

65+
async function onFileUpload(e: SyntheticEvent<HTMLInputElement>) {
66+
e.stopPropagation();
67+
e.preventDefault();
68+
69+
if (!e.currentTarget?.files?.length) return;
70+
await doUploadThemes(e.currentTarget.files);
71+
}
72+
73+
function useDropFile(refreshThemes: Function) {
74+
useEffect(() => {
75+
const onDragOver = (e: DragEvent) => {
76+
if (!e.dataTransfer?.items.length) return;
77+
if (!Array.from(e.dataTransfer.items).some(item => item.kind === "file" && item.getAsFile()?.name.endsWith(".css")))
78+
return;
79+
80+
e.preventDefault();
81+
e.dataTransfer.dropEffect = "copy";
82+
};
83+
84+
const onDrop = async (e: DragEvent) => {
85+
e.preventDefault();
86+
87+
if (!e.dataTransfer?.files.length) return;
88+
89+
await doUploadThemes(
90+
Array.from(e.dataTransfer.files).filter(file => file.name.endsWith(".css"))
91+
);
92+
93+
refreshThemes();
94+
};
95+
96+
window.addEventListener("dragover", onDragOver);
97+
window.addEventListener("drop", onDrop);
98+
99+
return () => {
100+
window.removeEventListener("dragover", onDragOver);
101+
window.removeEventListener("drop", onDrop);
102+
};
103+
}, []);
104+
}
105+
71106
export function LocalThemesTab() {
72107
const settings = useSettings(["enabledThemes"]);
73108

@@ -79,6 +114,9 @@ export function LocalThemesTab() {
79114
refreshLocalThemes();
80115
}, []);
81116

117+
// This condition is compile time so conditional hook is okay
118+
if (IS_WEB) useDropFile(refreshLocalThemes);
119+
82120
async function refreshLocalThemes() {
83121
const themes = await VencordNative.themes.getThemesList();
84122
setUserThemes(themes);
@@ -109,7 +147,7 @@ export function LocalThemesTab() {
109147
(
110148
<QuickAction
111149
text={
112-
<span style={{ position: "relative" }}>
150+
<span>
113151
Upload Theme
114152
<FileInput
115153
ref={fileInputRef}
@@ -123,6 +161,7 @@ export function LocalThemesTab() {
123161
</span>
124162
}
125163
Icon={PlusIcon}
164+
style={{ position: "relative" }}
126165
/>
127166
) : (
128167
<QuickAction

‎src/main/index.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,16 @@
1616
* along with this program. If not, see <https://www.gnu.org/licenses/>.
1717
*/
1818

19+
import "./ipcMain";
20+
1921
import { app, net, protocol } from "electron";
2022
import { join } from "path";
2123
import { pathToFileURL } from "url";
2224

2325
import { initCsp } from "./csp";
24-
import { ensureSafePath } from "./ipcMain";
2526
import { RendererSettings } from "./settings";
2627
import { IS_VANILLA, THEMES_DIR } from "./utils/constants";
28+
import { ensureSafePath } from "./utils/ensureSafePath";
2729
import { installExt } from "./utils/extensions";
2830

2931
if (IS_VESKTOP || !IS_VANILLA) {

0 commit comments

Comments
 (0)