fix: cap request bodies, link login error, add book-form retry, guard double add
Limit request bodies to 1 MiB (413 JSON), wire aria-describedby on the login form, add Retry and cancellation to the book form load, disable Add entry while a note POST is in flight, and poll pg_isready in README. Claude-Session: https://claude.ai/code/session_01M9MLit5Ko3X4s7rzC5Kv7X
This commit is contained in:
@@ -14,7 +14,7 @@ Optionally set `PASSWORD_PEPPER` (≥ 32 chars). Without it, a development peppe
|
|||||||
```bash
|
```bash
|
||||||
mise trust && mise install # Python 3.14, Node 24
|
mise trust && mise install # Python 3.14, Node 24
|
||||||
docker compose up -d db # postgres:16 on 127.0.0.1:5432
|
docker compose up -d db # postgres:16 on 127.0.0.1:5432
|
||||||
docker compose exec -T db pg_isready -U postgres # repeat until it reports "accepting connections"
|
until docker compose exec -T db pg_isready -U postgres; do sleep 1; done
|
||||||
mise exec -- python -m venv .venv && .venv/bin/pip install -r backend/requirements.txt
|
mise exec -- python -m venv .venv && .venv/bin/pip install -r backend/requirements.txt
|
||||||
mise exec -- npm install
|
mise exec -- npm install
|
||||||
.venv/bin/python backend/app.py # API on :5000
|
.venv/bin/python backend/app.py # API on :5000
|
||||||
|
|||||||
@@ -18,6 +18,9 @@ MUTATING_METHODS = {"POST", "PUT", "PATCH", "DELETE"}
|
|||||||
def create_app() -> Flask:
|
def create_app() -> Flask:
|
||||||
logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(name)s: %(message)s")
|
logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(name)s: %(message)s")
|
||||||
app = Flask(__name__)
|
app = Flask(__name__)
|
||||||
|
# Werkzeug buffers and parses the whole body before our validation runs, so unauthenticated
|
||||||
|
# callers could exhaust memory; 1 MiB is far above the largest legitimate payload (10,000-char note).
|
||||||
|
app.config["MAX_CONTENT_LENGTH"] = 1024 * 1024
|
||||||
db.init_app(app)
|
db.init_app(app)
|
||||||
auth.init_app(app)
|
auth.init_app(app)
|
||||||
app.register_blueprint(books.bp)
|
app.register_blueprint(books.bp)
|
||||||
|
|||||||
@@ -30,3 +30,8 @@ class AppTests(ApiTestCase):
|
|||||||
response.get_json(),
|
response.get_json(),
|
||||||
{"error": "Request body must be JSON with Content-Type: application/json"},
|
{"error": "Request body must be JSON with Content-Type: application/json"},
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_oversized_body_is_rejected_before_parsing(self):
|
||||||
|
response = self.call("POST", "/api/auth/login", {"username": "a", "password": "x" * (2 * 1024 * 1024)})
|
||||||
|
self.assertEqual(response.status_code, 413)
|
||||||
|
self.assertIn("error", response.get_json())
|
||||||
|
|||||||
@@ -45,6 +45,8 @@ export function LoginForm() {
|
|||||||
value={username}
|
value={username}
|
||||||
onChange={(e) => setUsername(e.target.value)}
|
onChange={(e) => setUsername(e.target.value)}
|
||||||
autoComplete="username"
|
autoComplete="username"
|
||||||
|
aria-invalid={error ? true : undefined}
|
||||||
|
aria-describedby={error ? 'login-error' : undefined}
|
||||||
required
|
required
|
||||||
/>
|
/>
|
||||||
</label>
|
</label>
|
||||||
@@ -57,11 +59,13 @@ export function LoginForm() {
|
|||||||
onChange={(e) => setPassword(e.target.value)}
|
onChange={(e) => setPassword(e.target.value)}
|
||||||
autoComplete={isLogin ? 'current-password' : 'new-password'}
|
autoComplete={isLogin ? 'current-password' : 'new-password'}
|
||||||
minLength={isLogin ? undefined : 12}
|
minLength={isLogin ? undefined : 12}
|
||||||
|
aria-invalid={error ? true : undefined}
|
||||||
|
aria-describedby={error ? 'login-error' : undefined}
|
||||||
required
|
required
|
||||||
/>
|
/>
|
||||||
</label>
|
</label>
|
||||||
{error && (
|
{error && (
|
||||||
<p role="alert" className="text-sm text-red-700">
|
<p id="login-error" role="alert" className="text-sm text-red-700">
|
||||||
{error}
|
{error}
|
||||||
</p>
|
</p>
|
||||||
)}
|
)}
|
||||||
|
|||||||
@@ -35,6 +35,7 @@ describe('auth flow', () => {
|
|||||||
await user.type(screen.getByLabelText('Password'), 'wrong password!')
|
await user.type(screen.getByLabelText('Password'), 'wrong password!')
|
||||||
await user.click(screen.getByRole('button', { name: 'Sign in' }))
|
await user.click(screen.getByRole('button', { name: 'Sign in' }))
|
||||||
expect(await screen.findByRole('alert')).toHaveTextContent('Invalid username or password')
|
expect(await screen.findByRole('alert')).toHaveTextContent('Invalid username or password')
|
||||||
|
expect(screen.getByLabelText('Password')).toHaveAttribute('aria-describedby', 'login-error')
|
||||||
})
|
})
|
||||||
|
|
||||||
it('registers a new account', async () => {
|
it('registers a new account', async () => {
|
||||||
|
|||||||
@@ -62,4 +62,15 @@ describe('BookForm', () => {
|
|||||||
body: { title: 'Dune Messiah', author: 'Frank Herbert', genre_id: 5, total_pages: 380, current_page: 142 },
|
body: { title: 'Dune Messiah', author: 'Frank Herbert', genre_id: 5, total_pages: 380, current_page: 142 },
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('retries a failed load and shows the prefilled form', async () => {
|
||||||
|
let attempts = 0
|
||||||
|
signedIn({
|
||||||
|
'GET /api/books/7': () => (++attempts === 1 ? [500, { error: 'Database unavailable' }] : [200, dune]),
|
||||||
|
})
|
||||||
|
const user = renderApp('/books/7/edit')
|
||||||
|
expect(await screen.findByRole('alert')).toHaveTextContent('Database unavailable')
|
||||||
|
await user.click(screen.getByRole('button', { name: 'Retry' }))
|
||||||
|
expect(await screen.findByLabelText('Title')).toHaveValue('Dune')
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
+19
-7
@@ -19,11 +19,16 @@ export function BookForm() {
|
|||||||
const [loadError, setLoadError] = useState<string | null>(null)
|
const [loadError, setLoadError] = useState<string | null>(null)
|
||||||
const [error, setError] = useState<FormError | null>(null)
|
const [error, setError] = useState<FormError | null>(null)
|
||||||
const [busy, setBusy] = useState(false)
|
const [busy, setBusy] = useState(false)
|
||||||
|
const [attempt, setAttempt] = useState(0)
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!editing) return
|
if (!editing) return
|
||||||
|
let cancelled = false
|
||||||
|
setLoadError(null)
|
||||||
|
setLoading(true)
|
||||||
api<Book>('GET', `/books/${id}`).then(
|
api<Book>('GET', `/books/${id}`).then(
|
||||||
(book) => {
|
(book) => {
|
||||||
|
if (cancelled) return
|
||||||
setFields({
|
setFields({
|
||||||
title: book.title,
|
title: book.title,
|
||||||
author: book.author,
|
author: book.author,
|
||||||
@@ -34,11 +39,15 @@ export function BookForm() {
|
|||||||
setLoading(false)
|
setLoading(false)
|
||||||
},
|
},
|
||||||
(err) => {
|
(err) => {
|
||||||
|
if (cancelled) return
|
||||||
setLoadError(errorMessage(err))
|
setLoadError(errorMessage(err))
|
||||||
setLoading(false)
|
setLoading(false)
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
}, [editing, id])
|
return () => {
|
||||||
|
cancelled = true
|
||||||
|
}
|
||||||
|
}, [editing, id, attempt])
|
||||||
|
|
||||||
function set(name: keyof Fields) {
|
function set(name: keyof Fields) {
|
||||||
return (event: { target: { value: string } }) => setFields((prev) => ({ ...prev, [name]: event.target.value }))
|
return (event: { target: { value: string } }) => setFields((prev) => ({ ...prev, [name]: event.target.value }))
|
||||||
@@ -72,14 +81,17 @@ export function BookForm() {
|
|||||||
|
|
||||||
if (loadError) {
|
if (loadError) {
|
||||||
return (
|
return (
|
||||||
<div className={card}>
|
<div role="alert" className={card}>
|
||||||
<p role="alert" className="text-red-700">
|
<p className="text-red-700">{loadError}</p>
|
||||||
{loadError}
|
<div className="mt-2 flex items-center gap-3">
|
||||||
</p>
|
<button type="button" className={secondaryButton} onClick={() => setAttempt((n) => n + 1)}>
|
||||||
<Link to="/books" className="mt-2 inline-block text-indigo-700 underline">
|
Retry
|
||||||
|
</button>
|
||||||
|
<Link to="/books" className="text-indigo-700 underline">
|
||||||
← Back to library
|
← Back to library
|
||||||
</Link>
|
</Link>
|
||||||
</div>
|
</div>
|
||||||
|
</div>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
if (loading) return <p className="text-slate-500">Loading…</p>
|
if (loading) return <p className="text-slate-500">Loading…</p>
|
||||||
@@ -106,7 +118,7 @@ export function BookForm() {
|
|||||||
))}
|
))}
|
||||||
</select>
|
</select>
|
||||||
</label>
|
</label>
|
||||||
{genresError && <p className="text-sm text-red-700">{`Could not load genres: ${genresError}`}</p>}
|
{genresError && <p role="alert" className="text-sm text-red-700">{`Could not load genres: ${genresError}`}</p>}
|
||||||
<div className="flex gap-4">
|
<div className="flex gap-4">
|
||||||
<label className="block">
|
<label className="block">
|
||||||
<span className={labelText}>Total pages</span>
|
<span className={labelText}>Total pages</span>
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { screen, within } from '@testing-library/react'
|
import { fireEvent, screen, within } from '@testing-library/react'
|
||||||
import { describe, expect, it } from 'vitest'
|
import { describe, expect, it } from 'vitest'
|
||||||
import type { Note } from '../api'
|
import type { Note } from '../api'
|
||||||
import { dune, renderApp, signedIn } from '../test/helpers'
|
import { dune, renderApp, signedIn } from '../test/helpers'
|
||||||
@@ -77,4 +77,22 @@ describe('NotesJournal', () => {
|
|||||||
await user.click(screen.getByRole('button', { name: 'Add entry' }))
|
await user.click(screen.getByRole('button', { name: 'Add entry' }))
|
||||||
expect(await screen.findByRole('alert')).toHaveTextContent('body must be at most 10000 characters')
|
expect(await screen.findByRole('alert')).toHaveTextContent('body must be at most 10000 characters')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('posts a double-clicked entry only once', async () => {
|
||||||
|
const calls = signedIn({
|
||||||
|
'GET /api/books/7': () => [200, dune],
|
||||||
|
'GET /api/books/7/notes': () => [200, []],
|
||||||
|
'POST /api/books/7/notes': ({ body }) => [201, { ...older, id: 12, body: (body as { body: string }).body }],
|
||||||
|
})
|
||||||
|
const user = renderApp('/books/7')
|
||||||
|
await screen.findByText('No entries yet.')
|
||||||
|
await user.type(screen.getByLabelText('New journal entry'), 'Once only')
|
||||||
|
const add = screen.getByRole('button', { name: 'Add entry' })
|
||||||
|
// Two clicks before the pending POST settles; user.dblClick would wait for it and mask the bug.
|
||||||
|
fireEvent.click(add)
|
||||||
|
fireEvent.click(add)
|
||||||
|
|
||||||
|
await screen.findByText('Once only')
|
||||||
|
expect(calls.filter((c) => c.method === 'POST')).toHaveLength(1)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ export function NotesJournal({ bookId }: { bookId: number }) {
|
|||||||
const [attempt, setAttempt] = useState(0)
|
const [attempt, setAttempt] = useState(0)
|
||||||
const [draft, setDraft] = useState('')
|
const [draft, setDraft] = useState('')
|
||||||
const [addError, setAddError] = useState<string | null>(null)
|
const [addError, setAddError] = useState<string | null>(null)
|
||||||
|
const [adding, setAdding] = useState(false)
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
let cancelled = false
|
let cancelled = false
|
||||||
@@ -29,6 +30,8 @@ export function NotesJournal({ bookId }: { bookId: number }) {
|
|||||||
|
|
||||||
async function addNote(event: FormEvent) {
|
async function addNote(event: FormEvent) {
|
||||||
event.preventDefault()
|
event.preventDefault()
|
||||||
|
if (adding) return
|
||||||
|
setAdding(true)
|
||||||
setAddError(null)
|
setAddError(null)
|
||||||
try {
|
try {
|
||||||
const note = await api<Note>('POST', `/books/${bookId}/notes`, { body: draft })
|
const note = await api<Note>('POST', `/books/${bookId}/notes`, { body: draft })
|
||||||
@@ -36,6 +39,8 @@ export function NotesJournal({ bookId }: { bookId: number }) {
|
|||||||
setDraft('')
|
setDraft('')
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
setAddError(errorMessage(err))
|
setAddError(errorMessage(err))
|
||||||
|
} finally {
|
||||||
|
setAdding(false)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -57,7 +62,7 @@ export function NotesJournal({ bookId }: { bookId: number }) {
|
|||||||
aria-describedby={addError ? 'add-note-error' : undefined}
|
aria-describedby={addError ? 'add-note-error' : undefined}
|
||||||
/>
|
/>
|
||||||
</label>
|
</label>
|
||||||
<button type="submit" className={primaryButton}>
|
<button type="submit" disabled={adding} className={primaryButton}>
|
||||||
Add entry
|
Add entry
|
||||||
</button>
|
</button>
|
||||||
{addError && (
|
{addError && (
|
||||||
|
|||||||
Reference in New Issue
Block a user