mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-24 07:53:09 +00:00
fix: make the app keyboard-reachable, and close four DoS-able patterns
The bulk of what SonarQube still had open on the reliability side, read site by
site rather than swept. Two clusters are worth calling out because they are
defects, not lint.
**Four regexes could be made to hang the process.** The worst is
`/.+@.+\..+/` in the forgot-password path: three unbounded `.+` that all
overlap, unanchored, on an endpoint that needs no login and whose schema is a
bare z.string() under a 100kb body cap. A thousand '@' characters had not
finished after 100 seconds — one request pins the event loop, and Node has one.
EMAIL_REGEX in auth.helpers is the same shape on register and profile update
(310ms for 8kb). The two tag strippers over an uploaded KML description and over
Commons metadata took 0.52s on a 16k run of '<'. Each rewrite was proven
equivalent by brute force against the original — millions of strings for the
coordinate patterns, 671,876 for the tag stripper — because a subtly different
regex on a parsing path is worse than the finding.
`<[^>]+>` could not be fixed in the pattern: `[^>]` matches `<` too, so the only
unambiguous form changes what happens to malformed markup. It is now a scan in
common/stripHtmlTags.ts, quirks included — `<>` is left alone, `<a<b>` is one
tag — with the equivalence pinned by a test that compares against the old regex
over every string up to length six.
**And roughly 500 places could only be reached with a mouse.** Clickable divs
became real buttons where the markup allowed (52 of them), took
role + tabIndex + Enter/Space where they had to stay divs, and modal backdrops
took role="presentation" — the honest role, since a backdrop is a mouse shortcut
for a Cancel control that is already in the tab order, and making it a button
plants a nameless full-screen tab stop around its own contents.
Alongside those: `.sort()` calls that now say the order they always had, `{n &&
<X/>}` guards that rendered a literal "0" for a zero-byte file and for a place at
latitude 0, an optimistic budget write that stored a boolean where the contract
and both transports use 0/1, and the Number statics.
sonar-project.properties documents the six that were deliberately left: two wiki
patterns with no overlapping quantifier pair whose "fix" would stop parsing
`[see [1]](url)`, four tar header fields bounded at 12-155 bytes by the format,
two djb2 hashes where the 32-bit wrap is the algorithm, two `| 0` that are also
coercing undefined to 0 on a path where the API sends no user_id, and the MCP
session maps Sonar reads as empty because they are filled from another module.
This commit is contained in:
@@ -132,24 +132,20 @@ describe('TodoListPanel', () => {
|
||||
const user = userEvent.setup();
|
||||
let putCalled = false;
|
||||
server.use(
|
||||
http.put('/api/trips/1/todo/:id/toggle', () => {
|
||||
http.put('/api/trips/1/todo/:id', () => {
|
||||
putCalled = true;
|
||||
return HttpResponse.json({ success: true });
|
||||
})
|
||||
);
|
||||
const items = [buildTodoItem({ id: 5, name: 'Toggle Me', checked: 0 })];
|
||||
render(<TodoListPanel tripId={1} items={items} />);
|
||||
// Click the checkbox button (Square icon)
|
||||
const checkboxes = screen.getAllByRole('button');
|
||||
// Find the checkbox button near the item
|
||||
const checkboxBtn = checkboxes.find(btn => {
|
||||
const parent = btn.closest('[style*="cursor: pointer"]');
|
||||
return parent && parent.textContent?.includes('Toggle Me');
|
||||
});
|
||||
if (checkboxBtn) {
|
||||
await user.click(checkboxBtn);
|
||||
await waitFor(() => expect(putCalled).toBe(true));
|
||||
}
|
||||
// The checkbox is the row's own <button>; the row around it is a button role.
|
||||
const row = screen.getByText('Toggle Me').closest('[role="button"]') as HTMLElement;
|
||||
const checkboxBtn = row.querySelector('button') as HTMLElement;
|
||||
|
||||
await user.click(checkboxBtn);
|
||||
|
||||
await waitFor(() => expect(putCalled).toBe(true));
|
||||
});
|
||||
|
||||
it('FE-COMP-TODO-013: clicking a task row opens its detail pane', async () => {
|
||||
@@ -180,7 +176,7 @@ describe('TodoListPanel', () => {
|
||||
expect(screen.getByText('JobTask')).toBeInTheDocument();
|
||||
expect(screen.getByText('HomeTask')).toBeInTheDocument();
|
||||
// Category buttons exist in sidebar (by accessible name or text)
|
||||
const catBtn = screen.getByRole('button', { name: /JobCat/ });
|
||||
const catBtn = sidebarButton(/JobCat/);
|
||||
expect(catBtn).toBeInTheDocument();
|
||||
// Clicking the category button should work without throwing
|
||||
await user.click(catBtn);
|
||||
@@ -467,9 +463,19 @@ function pickOption(label: string | RegExp) {
|
||||
* The sidebar filters are an inline component, so every re-render (the members
|
||||
* fetch resolving, for one) swaps the button element. userEvent's async steps can
|
||||
* land on the detached node — fireEvent stays atomic.
|
||||
*
|
||||
* A task row is a button too (it is what selects the task), and its name is built
|
||||
* from everything it shows, category chip included. So the sidebar entry is picked
|
||||
* by being an actual <button>.
|
||||
*/
|
||||
function sidebarButton(name: RegExp | string): HTMLElement {
|
||||
const [button] = screen.getAllByRole('button', { name }).filter(el => el.tagName === 'BUTTON');
|
||||
expect(button).toBeDefined();
|
||||
return button;
|
||||
}
|
||||
|
||||
function clickFilter(name: RegExp | string) {
|
||||
fireEvent.click(screen.getByRole('button', { name }));
|
||||
fireEvent.click(sidebarButton(name));
|
||||
}
|
||||
|
||||
describe('TodoListPanel — sidebar', () => {
|
||||
|
||||
@@ -267,7 +267,7 @@ export default function TodoListPanel({ tripId, items, addItemSignal = 0 }: { tr
|
||||
/>
|
||||
)}
|
||||
{selectedItem && !isAddingNew && isMobile && (
|
||||
<div onClick={e => { if (e.target === e.currentTarget) setSelectedId(null) }}
|
||||
<div role="presentation" onClick={e => { if (e.target === e.currentTarget) setSelectedId(null) }}
|
||||
style={{ position: 'fixed', inset: 0, zIndex: 1000, background: 'rgba(0,0,0,0.4)', display: 'flex', justifyContent: 'center', alignItems: 'flex-end', paddingBottom: 'var(--bottom-nav-h)' }}>
|
||||
<div style={{ width: '100%', maxHeight: '85vh', borderRadius: '16px 16px 0 0', overflow: 'auto' }}
|
||||
ref={el => { if (el) { const child = el.firstElementChild as HTMLElement; if (child) { child.style.width = '100%'; child.style.borderLeft = 'none'; child.style.borderRadius = '16px 16px 0 0' } } }}>
|
||||
@@ -282,7 +282,7 @@ export default function TodoListPanel({ tripId, items, addItemSignal = 0 }: { tr
|
||||
</div>
|
||||
)}
|
||||
{isAddingNew && !selectedItem && !isMobile && createPortal(
|
||||
<div onClick={e => { if (e.target === e.currentTarget) setIsAddingNew(false) }}
|
||||
<div role="presentation" onClick={e => { if (e.target === e.currentTarget) setIsAddingNew(false) }}
|
||||
className="trek-modal-backdrop"
|
||||
style={{ position: 'fixed', inset: 0, zIndex: 1000, background: 'rgba(15,23,42,0.5)', display: 'flex', justifyContent: 'center', alignItems: 'flex-start', paddingTop: 'calc(var(--nav-h) + 60px)', paddingBottom: 40 }}>
|
||||
<div style={{ width: 'min(520px, 92vw)', maxHeight: 'calc(100vh - var(--nav-h) - 120px)', overflow: 'auto', borderRadius: 16, boxShadow: '0 20px 60px rgba(0,0,0,0.25)' }}
|
||||
@@ -300,7 +300,7 @@ export default function TodoListPanel({ tripId, items, addItemSignal = 0 }: { tr
|
||||
document.body
|
||||
)}
|
||||
{isAddingNew && !selectedItem && isMobile && createPortal(
|
||||
<div onClick={e => { if (e.target === e.currentTarget) setIsAddingNew(false) }}
|
||||
<div role="presentation" onClick={e => { if (e.target === e.currentTarget) setIsAddingNew(false) }}
|
||||
className="trek-modal-backdrop"
|
||||
style={{ position: 'fixed', inset: 0, zIndex: 1000, background: 'rgba(0,0,0,0.4)', display: 'flex', justifyContent: 'center', alignItems: 'flex-end', paddingBottom: 'var(--bottom-nav-h)' }}>
|
||||
<div style={{ width: '100%', maxHeight: '85vh', borderRadius: '16px 16px 0 0', overflow: 'auto' }}
|
||||
|
||||
@@ -73,26 +73,26 @@ describe('TodoRow', () => {
|
||||
})
|
||||
|
||||
it('FE-W4TDR-004: the checkbox toggles the task without selecting the row', () => {
|
||||
const { onToggle, onSelect } = setup(todo())
|
||||
const { onToggle, onSelect, container } = setup(todo())
|
||||
|
||||
fireEvent.click(screen.getAllByRole('button')[0])
|
||||
fireEvent.click(container.querySelector('button')!)
|
||||
|
||||
expect(onToggle).toHaveBeenCalledWith(10, true)
|
||||
expect(onSelect).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('FE-W4TDR-005: a done task strikes through and unchecks on click', () => {
|
||||
const { onToggle } = setup(todo({ checked: 1 } as Partial<TodoItem>))
|
||||
const { onToggle, container } = setup(todo({ checked: 1 } as Partial<TodoItem>))
|
||||
|
||||
expect(screen.getByText('Book ferry')).toHaveStyle({ textDecoration: 'line-through' })
|
||||
fireEvent.click(screen.getAllByRole('button')[0])
|
||||
fireEvent.click(container.querySelector('button')!)
|
||||
expect(onToggle).toHaveBeenCalledWith(10, false)
|
||||
})
|
||||
|
||||
it('FE-W4TDR-006: a read-only row swallows the toggle', () => {
|
||||
const { onToggle } = setup(todo(), { canEdit: false })
|
||||
const { onToggle, container } = setup(todo(), { canEdit: false })
|
||||
|
||||
fireEvent.click(screen.getAllByRole('button')[0])
|
||||
fireEvent.click(container.querySelector('button')!)
|
||||
|
||||
expect(onToggle).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
@@ -33,7 +33,18 @@ export default function TodoRow({ item, members, categories, today, isSelected,
|
||||
|
||||
return (
|
||||
<div key={item.id}
|
||||
// Selecting the task has no other trigger, so the row is the control. Its
|
||||
// checkbox and its drag handle answer for themselves, hence the key handler
|
||||
// only fires when the row itself has focus.
|
||||
role="button"
|
||||
tabIndex={0}
|
||||
onClick={() => onSelect(isSelected ? null : item.id)}
|
||||
onKeyDown={e => {
|
||||
if (e.target !== e.currentTarget) return
|
||||
if (e.key !== 'Enter' && e.key !== ' ') return
|
||||
e.preventDefault()
|
||||
onSelect(isSelected ? null : item.id)
|
||||
}}
|
||||
onDragOver={canDrag ? (e => { e.preventDefault(); e.dataTransfer.dropEffect = 'move'; drag!.onOver(item.id) }) : undefined}
|
||||
onDragLeave={canDrag ? (e => { if (!e.currentTarget.contains(e.relatedTarget as Node)) drag!.onOver(-1) }) : undefined}
|
||||
onDrop={canDrag ? (e => { e.preventDefault(); e.stopPropagation(); drag!.onDrop(item.id) }) : undefined}
|
||||
@@ -50,6 +61,9 @@ export default function TodoRow({ item, members, categories, today, isSelected,
|
||||
|
||||
{canDrag && (
|
||||
<div
|
||||
// Dragging is the handle's whole job; the click only keeps the row from
|
||||
// selecting, so the handle stays presentational.
|
||||
role="presentation"
|
||||
draggable
|
||||
onClick={e => e.stopPropagation()}
|
||||
onDragStart={e => { e.stopPropagation(); e.dataTransfer.effectAllowed = 'move'; drag!.onStart(item.id) }}
|
||||
@@ -83,7 +97,7 @@ export default function TodoRow({ item, members, categories, today, isSelected,
|
||||
</div>
|
||||
)}
|
||||
{/* Inline badges */}
|
||||
{(item.priority || item.due_date || catColor || assignedUser) && (
|
||||
{!!(item.priority || item.due_date || catColor || assignedUser) && (
|
||||
<div style={{ display: 'flex', gap: 5, marginTop: 5, flexWrap: 'wrap' }}>
|
||||
{item.priority > 0 && PRIO_CONFIG[item.priority] && (
|
||||
<span style={{
|
||||
|
||||
@@ -15,7 +15,7 @@ export function katColor(kat: string, allCategories: string[]) {
|
||||
const idx = allCategories.indexOf(kat)
|
||||
if (idx >= 0) return KAT_COLORS[idx % KAT_COLORS.length]
|
||||
let h = 0
|
||||
for (let i = 0; i < kat.length; i++) h = ((h << 5) - h + kat.charCodeAt(i)) | 0
|
||||
for (let i = 0; i < kat.length; i++) h = ((h << 5) - h + kat.codePointAt(i)) | 0
|
||||
return KAT_COLORS[Math.abs(h) % KAT_COLORS.length]
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user