A non-admin could bind an arbitrary OAuth identity to their own account
by patching google_id or openid_connect_id, bypassing the duplicate
check enforced in the OAuth callback. Remove both fields from the
update request so binding only happens through the OAuth flow.
Browsers normalize backslashes to forward slashes, so a redirect target
like "/\evil.com" parsed as a relative path by url.Parse and resolved to
//evil.com by the browser, resulting in an open redirect. Reject any link
containing a backslash before validating it as a relative path.
WithTags previously emitted one "LOWER($i) = ANY(LOWER(e.tags::text)::text[])"
condition and one parameter per filter tag. With K tags this means K
predicates, K parameters, and K evaluations of the LOWER(e.tags::text)::text[]
sub-expression per candidate row.
This commit replaces the loop with a single predicate using the array
containment operator:
LOWER(e.tags::text)::text[] @> LOWER($N::text)::text[]
Same case-insensitive "row must contain all listed tags" semantics, fewer
predicates for the planner, and the row-side expression is referenced once
instead of K times.
findSubscriptionsFromWebPage was running four separate goquery.Find passes, one
per supported MIME type. This commit replaces it with one
doc.Find("link[type]") traversal and a switch on the type attribute, which is
functionally equivalent, but it visits the DOM once instead of four times and
emits results in document order. Microbenchmark on a representative <head> (8
link tags + ~100 unrelated head children):
before: ~25 µs/op, 1536 B/op, 50 allocs/op
after: ~9 µs/op, 744 B/op, 21 allocs/op
- Don't call strings.TrimSpace twice on canonicalHref
- Use doc.FindMatcher+goquery.Single instead of doc.Find+First, as is done
everywhere else in the codebase.
Replace LEFT JOIN with INNER JOIN in queries where the WHERE clause or
foreign key constraints already guarantee matching rows exist:
- Icons(): filters on feeds.user_id
- UserByAPIKey(): filters on api_keys.token
- fetchFeedCounter(): filters on feeds columns when counterJoinFeeds is set
- fetchEntries(): entries have FK constraints to feeds, categories, and
users (feed_icons/icons kept as LEFT JOIN since icons are optional)
PostgreSQL already optimizes these identically, but INNER JOIN makes the
intent explicit.
The WHERE clause on feeds columns already eliminates NULL-extended rows,
making the LEFT JOINs logically equivalent to INNER JOINs. PostgreSQL's
planner is smart enough to recognize this and produces an identical
execution plan (verified with EXPLAIN ANALYZE), but using INNER JOIN
makes the intent explicit for humans like me reading the query.
findSubscriptionsFromWellKnownURLs iterates a fixed table of 9 well-known
feed paths against the discovered base URLs. The table was declared as a
map[string]string, which paid per-iteration hash overhead and gave
non-deterministic probe order across runs.
This commit replaces it with a fixed-size [...]struct{path, format string}.
Probes now run in declared order (more predictable behavior for users when
multiple well-known URLs respond, and easier to reason about in tests), and the
inner loop avoids the map iterator entirely.
Micro-benchmark replaying the double loop body against 2 base URLs ×
9 paths, sans HTTP I/O (medians of 5 runs):
name old ns/op new ns/op delta
KnownURLs 24,194 18,150 -25%
name old B/op new B/op delta
KnownURLs 9,688 9,072 -6.4%
name old allocs/op new allocs/op delta
KnownURLs 110 107 -3
- No need to use `make(…)` construct for empty slices, as is already the case
in the rest of internal/googlereader/handler.go
- Replace a useless condition in internal/reader/sanitizer/sanitizer.go with an
unconditional assignment.
- Remove a useless call to url.Parse in JoinBaseURLAndPath, as url.JoinPath
already performs the validation internally
parseLocalTimeDates is called once per parsed feed entry. Each call to
time.LoadLocation("America/Los_Angeles" | "America/New_York") reads and
parses the IANA tzdata file from disk or from the embedded zoneinfo.
This commit hoists the two LoadLocation calls to package-level vars so the
lookup happens once at init time and the hot path becomes a pointer load.
Benchmarked on the local-time fallback path:
before: ~24,000 ns/op, ~15 KB/op, 26 allocs/op
after: ~330 ns/op, 0 B/op, 0 allocs/op
The metrics Basic Auth check used != for username and password, which is
technically vulnerable to timing side-channel attacks. Since the metrics
credentials are typically short static config values, the timing difference is
very likely to be in the noise level, but oh well, it's a good practise to do
credential validation in constant time.
entries_feed_idx(feed_id) is covered by both the unique constraint
entries_feed_id_hash_key(feed_id, hash) and the explicit index
entries_feed_id_status_hash_idx(feed_id, status, hash), which handle
all feed_id-leading lookups including FK cascade deletes.
entries_user_status_idx(user_id, status) is a prefix of five existing
three-column indexes (entries_user_status_feed_idx,
entries_user_status_changed_idx, entries_user_status_published_idx,
entries_user_status_created_idx, entries_user_status_changed_published_idx),
all of which serve every query the two-column index could.
Saves ~14 MB per million entries.
Store WebAuthn backup eligibility and backup state with each
credential instead of overwriting BackupEligible from every login
assertion.
Use a nullable backup_eligible column to identify legacy credentials
and backfill those records on their next successful login. Also persist
the validated credential state after login, including sign count, clone
warning, and backup state.
Remove the username-based WebAuthn login flow because it allowed
username enumeration before password verification.
WebAuthn login now uses discoverable credentials only, and new
registrations require resident keys. Existing non-resident credentials
are no longer usable for first-factor login; they should only be used
in a post-password MFA flow, which Miniflux does not currently
implement.
BREAKING CHANGE: Users with existing non-resident WebAuthn credentials
must register a new passkey.
The icons table is deduplicated by hash and shared with feeds via the
feed_icons junction. When a feed is deleted, the ON DELETE CASCADE
removes its feed_icons row but leaves the icons row behind. The same
happens in StoreFeedIcon when a feed's icon is replaced. Over time
these orphaned bytea blobs accumulate and bloat the database.
Add Storage.CleanupOrphanIcons, which deletes icons rows that no
feed_icons row still references, and call it from runCleanupTasks.
The middleware (*authMiddleware).validateApiKey is registered for 14
routes in NewHandler. Its body was just `return http.HandlerFunc(func…)`,
which made the compiler consider it inlinable. As a result, the entire
losure body was duplicated at every call site, taking space in the .text
section.
Move the request-handling logic into a separate non-inlined method
serveValidated and keep validateApiKey as a thin wrapper that only
allocates the http.HandlerFunc. The 14 duplicated symbols are gone and
the stripped binary shrinks from 20,513,033 to 20,447,497 bytes, which isn't
that much, but it's still something, especially for such a simple commit.
Replace `num += string(char)` with index-based slicing of the input
string. The previous loop allocated a new string for every digit in
the duration (and again on each `+=`), giving O(n²) allocation
behavior. Slicing `after[start:i]` reuses the original string's
backing memory and only allocates once per numeric component when
passed to `strconv.Atoi`.
It also makes the code a bit more compact/simpler.
Called once per YouTube/podcast entry that exposes an ISO8601
duration during feed processing.
Drop the snake_case cred_uid, fix the missing-n typos in webAuthUser
and webAuthCredential, replace credCredential with validatedCredential,
unify uid/userID on userID, rename the shadowed url local in
newWebAuthn to baseURL, and use the full credential / credentials names
throughout instead of the abbreviated cred / creds. No behaviour change.
The web session middleware redirects unauthenticated requests to the
login page before any non-public handler runs, so request.UserID is
guaranteed non-zero in beginRegistration, finishRegistration, and
deleteCredential. Remove the dead checks to match the other WebAuthn
handlers.
While both deleteCredential either validate or pass down a uid,
saveCredential doesn't. This isn't exploitable as an authenticated attacker
would need to guess the 32 bytes handle of another one, but it doesn't hurt to
explicitly check if a user is only operating on their own user.
Account unlinking mutates state, so /oauth2/{provider}/unlink can no
longer be reached via GET. Pull the OAuth2/OIDC and WebAuthn sections
out of the settings form and render each as its own fieldset above the
form, with the unlink action as a self-contained POST form. Rename
the username/password fieldset legend to "Password Authentication" and
add matching legends for the federated and passkey sections so all
authentication methods read consistently.
The function strings.Fields will allocate every single word it's creating,
meaning that for a text of 10k words, 10k allocations will be made, only for
them to be counted an discarded. We can do much better by counting the words
ourself via a small countWords helper function, and write a test to prove that
it doesn't allocate anything.
Feed refresh endpoints mutate state and should not be reachable via
GET. Drop the GET registrations on /feeds/refresh, /feed/{id}/refresh,
/category/{id}/feeds/refresh and /category/{id}/entries/refresh, and
update the templates and the R keyboard shortcut to submit POST forms
with a CSRF token.
Logout is a state-changing action and should not be reachable via GET.
Switching to POST routes the request through the CSRF middleware so
prefetchers and cross-site GETs can no longer terminate the session.
gudvinr