Skip to content

Commit 61d4269

Browse files
jpenillajpenilla
committed
Improve regex input validation
1 parent 5c7ea0f commit 61d4269

File tree

4 files changed

+65
-44
lines changed

4 files changed

+65
-44
lines changed

‎web/src/lib/components/diff-filtering/DiffFilterDialog.svelte‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,30 @@
11
<script lang="ts">
22
import { getFileStatusProps, MultiFileDiffViewerState } from "$lib/diff-viewer.svelte";
33
import { Button, Dialog, ToggleGroup } from "bits-ui";
4-
import { type FilePathFilterMode } from "./index.svelte";
4+
import { tryCompileRegex, type TryCompileRegexFailure } from "$lib/util";
55
import { FILE_STATUSES } from "$lib/github.svelte";
66
import { slide } from "svelte/transition";
7+
import { type FilePathFilterMode } from "$lib/components/diff-filtering/index.svelte";
78
89
const viewer = MultiFileDiffViewerState.get();
910
const instance = viewer.filter;
1011
12+
let newFilePathFilterElement: HTMLInputElement | undefined = $state();
1113
let newFilePathFilterInput = $state("");
14+
let newFilePathFilterInputResult = $derived.by(() => {
15+
if (newFilePathFilterInput === "") {
16+
return { success: false, error: "No input provided", input: "" } satisfies TryCompileRegexFailure;
17+
}
18+
return tryCompileRegex(newFilePathFilterInput);
19+
});
20+
$effect(() => {
21+
if (newFilePathFilterElement && newFilePathFilterInputResult.success) {
22+
newFilePathFilterElement.setCustomValidity("");
23+
} else if (newFilePathFilterElement && !newFilePathFilterInputResult.success) {
24+
newFilePathFilterElement.setCustomValidity(newFilePathFilterInputResult.error);
25+
}
26+
});
27+
1228
let newFilePathFilterMode: FilePathFilterMode = $state("exclude");
1329
</script>
1430

@@ -54,9 +70,8 @@
5470
class="mb-1 flex w-full items-center gap-1"
5571
onsubmit={(e) => {
5672
e.preventDefault();
57-
if (newFilePathFilterInput === "") return;
58-
// TODO error handling
59-
instance.addFilePathFilter(newFilePathFilterInput, newFilePathFilterMode);
73+
if (!newFilePathFilterInputResult.success) return;
74+
instance.addFilePathFilter(newFilePathFilterInputResult, newFilePathFilterMode);
6075
newFilePathFilterInput = "";
6176
}}
6277
>
@@ -65,6 +80,7 @@
6580
placeholder="Enter regular expression here..."
6681
class="grow rounded-md border px-2 py-1 inset-shadow-xs ring-focus focus:outline-none focus-visible:ring-2"
6782
bind:value={newFilePathFilterInput}
83+
bind:this={newFilePathFilterElement}
6884
/>
6985
<Button.Root
7086
type="button"
@@ -110,7 +126,7 @@
110126
</li>
111127
{/each}
112128
{#if instance.reverseFilePathFilters.length === 0}
113-
<li class="flex size-full items-center justify-center text-em-med">No file path filters. Add one using the above form.</li>
129+
<li class="flex size-full items-center justify-center px-4 text-em-med">No file path filters. Add one using the above form.</li>
114130
{/if}
115131
</ul>
116132
</div>

‎web/src/lib/components/diff-filtering/index.svelte.ts‎

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { FileDetails } from "$lib/diff-viewer.svelte";
22
import { FILE_STATUSES } from "$lib/github.svelte";
3+
import type { TryCompileRegexSuccess } from "$lib/util";
34
import { SvelteSet } from "svelte/reactivity";
45

56
export type FilePathFilterMode = "include" | "exclude";
@@ -15,30 +16,16 @@ export class FilePathFilter {
1516
}
1617
}
1718

18-
function tryCompileRegex(pattern: string): RegExp | undefined {
19-
try {
20-
return new RegExp(pattern);
21-
} catch {
22-
return undefined;
23-
}
24-
}
25-
2619
export class DiffFilterDialogState {
2720
filePathFilters = new SvelteSet<FilePathFilter>();
2821
reverseFilePathFilters = $derived([...this.filePathFilters].toReversed());
29-
filePathInclusions = $derived(this.reverseFilePathFilters.filter((f) => f.mode === "include"));
30-
filePathExclusions = $derived(this.reverseFilePathFilters.filter((f) => f.mode === "exclude"));
22+
filterFunction = $derived(this.createFilterFunction());
3123

3224
selectedFileStatuses: string[] = $state([...FILE_STATUSES]);
3325

34-
addFilePathFilter(filterString: string, mode: FilePathFilterMode): { invalidRegex: boolean } {
35-
const compiled = tryCompileRegex(filterString);
36-
if (!compiled) {
37-
return { invalidRegex: true };
38-
}
39-
const newFilter = new FilePathFilter(filterString, compiled, mode);
26+
addFilePathFilter(regex: TryCompileRegexSuccess, mode: FilePathFilterMode) {
27+
const newFilter = new FilePathFilter(regex.input, regex.regex, mode);
4028
this.filePathFilters.add(newFilter);
41-
return { invalidRegex: false };
4229
}
4330

4431
setDefaults() {
@@ -47,23 +34,33 @@ export class DiffFilterDialogState {
4734
}
4835

4936
filterFile(file: FileDetails): boolean {
50-
const statusAllowed = this.selectedFileStatuses.includes(file.status);
51-
if (!statusAllowed) {
52-
return false;
53-
}
54-
for (const exclude of this.filePathExclusions) {
55-
if (exclude.regex.test(file.toFile) || exclude.regex.test(file.fromFile)) {
37+
return this.filterFunction(file);
38+
}
39+
40+
private createFilterFunction() {
41+
const filePathInclusions = this.reverseFilePathFilters.filter((f) => f.mode === "include");
42+
const filePathExclusions = this.reverseFilePathFilters.filter((f) => f.mode === "exclude");
43+
const selectedFileStatuses = [...this.selectedFileStatuses];
44+
45+
return (file: FileDetails) => {
46+
const statusAllowed = selectedFileStatuses.includes(file.status);
47+
if (!statusAllowed) {
5648
return false;
5749
}
58-
}
59-
if (this.filePathInclusions.length > 0) {
60-
for (const include of this.filePathInclusions) {
61-
if (include.regex.test(file.toFile) || include.regex.test(file.fromFile)) {
62-
return true;
50+
for (const exclude of filePathExclusions) {
51+
if (exclude.regex.test(file.toFile) || exclude.regex.test(file.fromFile)) {
52+
return false;
53+
}
54+
}
55+
if (filePathInclusions.length > 0) {
56+
for (const include of filePathInclusions) {
57+
if (include.regex.test(file.toFile) || include.regex.test(file.fromFile)) {
58+
return true;
59+
}
6360
}
61+
return false;
6462
}
65-
return false;
66-
}
67-
return true;
63+
return true;
64+
};
6865
}
6966
}

‎web/src/lib/open-diff-dialog.svelte.ts‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { DirectoryEntry, FileEntry, MultimodalFileInputState, type MultimodalFil
33
import { SvelteSet } from "svelte/reactivity";
44
import { type FileStatus } from "$lib/github.svelte";
55
import { makeImageDetails, makeTextDetails, MultiFileDiffViewerState, type LoadPatchesOptions } from "$lib/diff-viewer.svelte";
6-
import { binaryFileDummyDetails, bytesEqual, isBinaryFile, isImageFile, parseMultiFilePatch } from "$lib/util";
6+
import { binaryFileDummyDetails, bytesEqual, isBinaryFile, isImageFile, parseMultiFilePatch, tryCompileRegex } from "$lib/util";
77
import { createTwoFilesPatch } from "diff";
88

99
export interface OpenDiffDialogProps {
@@ -47,13 +47,9 @@ export class OpenDiffDialogState {
4747
}
4848

4949
addBlacklistEntry() {
50-
if (this.dirBlacklistInput === "") {
51-
return;
52-
}
53-
try {
54-
new RegExp(this.dirBlacklistInput); // Validate regex
55-
} catch (e) {
56-
alert("'" + this.dirBlacklistInput + "' is not a valid regex pattern. Error: " + e);
50+
const result = tryCompileRegex(this.dirBlacklistInput);
51+
if (!result.success) {
52+
alert(result.error);
5753
return;
5854
}
5955
this.dirBlacklist.add(this.dirBlacklistInput);

‎web/src/lib/util.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,3 +420,15 @@ export function animationFramePromise() {
420420
export async function yieldToBrowser() {
421421
await new Promise((resolve) => setTimeout(resolve, 0));
422422
}
423+
424+
export type TryCompileRegexSuccess = { success: true; regex: RegExp; input: string };
425+
export type TryCompileRegexFailure = { success: false; error: string; input: string };
426+
export type TryCompileRegexResult = TryCompileRegexSuccess | TryCompileRegexFailure;
427+
428+
export function tryCompileRegex(pattern: string): TryCompileRegexResult {
429+
try {
430+
return { success: true, regex: new RegExp(pattern), input: pattern };
431+
} catch (e) {
432+
return { success: false, error: e instanceof Error ? e.message : String(e), input: pattern };
433+
}
434+
}

0 commit comments

Comments
 (0)