Skip to content

Commit facaca5

Browse files
Copilotcohm
andauthored
Fix security and performance issues found in code review
- fonts.ts: O(n²) string concatenation → O(n) Array.from+join for base64 - fonts.ts: isSystemFont() iterates Set directly instead of Array.from conversion - TimelineVisualization.tsx: replace CSS [data-group="name"] interpolation with filter-function approach so group names with special CSS chars are safe; selectByGroup helper defined once outside the forEach loop Agent-Logs-Url: https://github.com/cohm/ProgramVisualization/sessions/f61843d3-07a7-4e3c-ad7a-40060c98e600 Co-authored-by: cohm <7143813+cohm@users.noreply.github.com>
1 parent e556f5a commit facaca5

2 files changed

Lines changed: 19 additions & 22 deletions

File tree

src/components/TimelineVisualization.tsx

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2639,31 +2639,26 @@ const TimelineVisualization = forwardRef(function TimelineVisualization({ course
26392639
if (!containerRef.current || !cosmetics) return;
26402640
const container = d3.select(containerRef.current);
26412641

2642+
// Helpers for safe group-name filtering — defined once here and reused
2643+
// for all groups so we don't reallocate closures inside the forEach loop.
2644+
// Using filter functions instead of CSS [data-group="…"] attribute selectors
2645+
// prevents group names with special CSS characters from corrupting the selector.
2646+
const selectByGroup = (cls: string, name: string) =>
2647+
container.selectAll(cls).filter(function() {
2648+
return (this as Element).getAttribute('data-group') === name;
2649+
});
2650+
26422651
cosmetics.groups.forEach(group => {
26432652
const isGroupVisible = layers.groups[group.name] !== false;
26442653
const courseDisplay = (isGroupVisible && layers.courseBars) ? '' : 'none';
26452654
const examDisplay = (isGroupVisible && layers.exams && layers.courseBars) ? '' : 'none';
26462655
const reexamDisplay = (isGroupVisible && layers.reexams && layers.courseBars) ? '' : 'none';
26472656

2648-
container.selectAll(`.course-group[data-group="${group.name}"]`)
2649-
.interrupt()
2650-
.style('display', courseDisplay);
2651-
2652-
container.selectAll(`.course-connector-fill[data-group="${group.name}"]`)
2653-
.interrupt()
2654-
.style('display', courseDisplay);
2655-
2656-
container.selectAll(`.course-connector-border[data-group="${group.name}"]`)
2657-
.interrupt()
2658-
.style('display', courseDisplay);
2659-
2660-
container.selectAll(`.exam-dot[data-group="${group.name}"]`)
2661-
.interrupt()
2662-
.style('display', examDisplay);
2663-
2664-
container.selectAll(`.reexam-dot[data-group="${group.name}"]`)
2665-
.interrupt()
2666-
.style('display', reexamDisplay);
2657+
selectByGroup('.course-group', group.name).interrupt().style('display', courseDisplay);
2658+
selectByGroup('.course-connector-fill', group.name).interrupt().style('display', courseDisplay);
2659+
selectByGroup('.course-connector-border', group.name).interrupt().style('display', courseDisplay);
2660+
selectByGroup('.exam-dot', group.name).interrupt().style('display', examDisplay);
2661+
selectByGroup('.reexam-dot', group.name).interrupt().style('display', reexamDisplay);
26672662
});
26682663

26692664
// Prereq arrows: composite of (relevant prereq* layer) AND (both endpoint

src/lib/fonts.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ const WEIGHTS = '300;400;500;600;700;800;900';
1919
const cache = new Map<string, Promise<string>>();
2020

2121
function isSystemFont(name: string): boolean {
22-
return Array.from(SYSTEM_FONTS).some(s => name.includes(s));
22+
for (const s of SYSTEM_FONTS) if (name.includes(s)) return true;
23+
return false;
2324
}
2425

2526
async function fetchFontFaceCss(family: string): Promise<string> {
@@ -43,8 +44,9 @@ async function fetchFontFaceCss(family: string): Promise<string> {
4344
if (!fontResp.ok) continue;
4445
const buf = await fontResp.arrayBuffer();
4546
const u8 = new Uint8Array(buf);
46-
let binary = '';
47-
for (let i = 0; i < u8.length; i++) binary += String.fromCharCode(u8[i]);
47+
// Array.from + join is O(n) — avoids the O(n²) intermediate-string
48+
// churn of the naive `for (…) binary += String.fromCharCode(…)` loop.
49+
const binary = Array.from(u8, b => String.fromCharCode(b)).join('');
4850
const base64 = btoa(binary);
4951
const format = url.includes('.woff2') ? 'woff2' : url.includes('.woff') ? 'woff' : 'truetype';
5052
faces.push(`@font-face { font-family: '${family}'; src: url(data:font/${format};base64,${base64}) format('${format}'); font-weight: 100 900; font-style: normal; }`);

0 commit comments

Comments
 (0)