MEDIA_PROXY_RESOURCE_TYPES and TRUSTED_REVERSE_PROXY_NETWORKS were
validated by splitting the raw value without trimming, while the parser
trims items and skips empty ones. Values such as "image, video" or
"192.168.0.0/16, 10.0.0.0/8", and lists with a trailing comma, failed
startup even though the parser accepts them.
Validate the parsed list instead, and reject values that contain no
items at all so that a comma-only value cannot silently clear the
default media proxy resource types.
getTranslationDict lazily populates the package-level defaultCatalog
map and runs on concurrent request goroutines via template functions,
flash messages, and error translation. Two concurrent requests for
languages not yet cached triggered Go's fatal "concurrent map writes"
and killed the daemon.
Guard the catalog with a sync.RWMutex using double-checked locking so
each language is loaded once. A failed load is no longer cached as an
empty dictionary, so unknown languages now return the error on every
call; both callers treat an error and an empty dictionary identically,
so rendered output is unchanged.
Add a regression test exercising concurrent lazy population; it fails
under the race detector on the previous implementation.
The showChooseSubscriptionPage handler never set hasProxyConfigured on
the view, so when the add_subscription template was re-rendered after a
validation or feed creation error, the fetch_via_proxy checkbox
silently disappeared from the form. Set the flag during view setup,
matching the other subscription handlers.
Pool.Shutdown() closed the job queue channel, but jobs can still be
pushed while workers are draining: the feed scheduler ticker goroutine
is never cancelled, and the UI and API refresh handlers push from
detached goroutines that outlive the HTTP server shutdown. Any of them
sending on the closed queue panicked the process mid-shutdown.
Keep the queue channel open and instead close a dedicated shutdown
channel, guarded by sync.Once. Push now selects between delivering a
job and the shutdown signal, discarding jobs once shutdown begins, and
workers select between the queue and the shutdown signal, so they
still finish their current job before Shutdown returns.
strings.Split("", ",") returns [""], so an empty tag configuration
attached a single empty-string tag to every bookmark saved through the
Raindrop integration. Split the configured tags with SplitSeq, trim
whitespace, drop empty items, and omit the tags field entirely from the
payload when no tags are configured.
SCHEDULER_ENTRY_FREQUENCY_FACTOR was the only scheduler option without a
validator, so 0 was accepted at startup. With the entry_frequency polling
scheduler, the factor is used as part of a divisor in ScheduleNextCheck,
and a feed with weekly entries then triggered a division-by-zero panic
inside a background worker, crashing the daemon.
Require the factor to be >= 1, matching the validation of the other
scheduler options, so the misconfiguration fails at startup instead.
WithLimit ignored a zero limit, so /v1/entries?limit=0 produced a query
without a SQL LIMIT and returned every matching entry, bypassing the
1000-entry cap. The official API client sends limit=0 for any filter
with an unset Limit field, making unbounded queries easy to trigger.
Clamp non-positive values to the maximum in both WithLimit and
WithLimitAndMaximum so every caller (REST API, Google Reader) stays
bounded, as intended by 0909323a.
CreateFeed copied every other option from the feed creation request but
never NoMediaPlayer, so the flag was silently dropped and always stored
as false when creating a feed.
An empty proxy_url in a feed modification request was rejected with
error.proxy_url_not_empty, so a proxy URL could never be unset once
configured. Accept the empty string to clear it, matching feed
creation, and only validate non-empty values.
ValidateUserModification reused ValidateEntryOrder, which accepts nine
sorting fields valid for the entry-listing order query parameter, while
the users.entry_order column is an entry_sorting_order enum allowing
only published_at and created_at. Requests such as
PUT /v1/users/{id} with entry_sorting_order=title passed validation and
failed in PostgreSQL, returning 500 instead of 400.
Validate the user preference against the enum values with a dedicated
validateEntrySortingOrder function and keep ValidateEntryOrder for the
entry-listing endpoint.
The total returned by GetEntriesWithCount comes from count(*) OVER(),
which is carried on the returned rows. When the requested offset lands
past the last matching row, the query returns no rows and the total was
reported as 0 even though matching entries exist, breaking clients that
paginate until offset >= total.
Fall back to a separate CountEntries() query when the page is empty and
the offset is greater than zero. With offset 0 an empty result genuinely
means zero matches, so the single-query fast path is unchanged for
normal requests.
Add an integration test requesting the page at offset == total, which
must return no entries while keeping the same total.
The guard in RemoveAndReplaceCategoriesByName counted categories with
"title != ANY($2)", which is true whenever the title differs from at
least one element of the array. With two or more titles in the list,
every category matched — including the ones being deleted — so the
"at least 1 category must remain" check could pass even when the
deletion would remove all of the user's categories, leaving feeds
with a NULL category. Using "title <> ALL($2)" counts only the
categories that would actually survive the deletion.
The listening Unix socket was created world-writable (0666), allowing
any local user to connect. Restrict it to the owner and group.
Deployments where the reverse proxy runs as a different user now need
that user to share a group with the Miniflux process.
The GET /v1/icons/{iconID} endpoint fetched icons by their internal
numeric identifier without any user scoping, allowing any authenticated
user to enumerate icon IDs and read favicon data associated with feeds
owned by other users on the same instance.
Rename IconByID to IconByUserAndIconID and gate the lookup on an EXISTS
check against feeds owned by the requesting user, so unauthorized icon
IDs return 404. Add integration tests covering cross-user access and
inexisting icon IDs.
Every integration talking to a JSON API hand-rolled the same dance:
json.Marshal the payload, build an *http.Request, set the Content-Type and
User-Agent headers, then run it through NewClientWithOptions. This is
near-identical copy-paste, so any change done to how requests are done had to
be repeated in each one.
This commit introduces a small request builder in internal/http/client and
route every integration through it:
```
client.NewRequestBuilder(endpoint).
WithMethod(http.MethodPost).
WithJSON(payload).
WithHeaders(extraHeaders).
Do()
```
Centralizing the logic also evens out disparities that had crept in between
integrations:
- private networks are now blocked everywhere, honoring
INTEGRATION_ALLOW_PRIVATE_NETWORKS; notion previously did not block them;
- request errors are wrapped consistently with %w instead of a mix of %v/%w;
- the per-integration defaultClientTimeout constant is replaced by a single
shared default in the client package;
- the Content-Type and Miniflux User-Agent headers are always set.
Reviewed-By: gudvinr
TestRequestBuilder_TimeoutConfiguration dominated the whole test suite
at ~2s. The handler slept a fixed 2s, and httptest.Server.Close() blocks
until in-flight handlers finish, so the package always paid the full
sleep even though the client timed out at 1s.
Block the handler on <-r.Context().Done() instead: when the client times
out and drops the connection, the request context is cancelled and the
handler returns immediately, so Close() no longer waits. Shrink the
timeout to 100ms as well.
Every integration tests in api_integration_test.go run sequentially against a
live server and are network-bound, taking around one minutes on my machine.
Each test is already isolated: it creates its own random-username user
and operates through that user's own client, with feeds/categories/
entries scoped per user. The shared admin client is only used read-only
or on error-path tests that create no durable state, and no test asserts
global counts, so the suite is safe to parallelize.
Add t.Parallel() to every test. No t.Setenv/Chdir or t.Run subtests are
present, so nothing conflicts with parallel execution.
It speeds things up from ~60s to ~7s on my machine. I ran the full testsuite a
handful of times just to be sure™, but didn't get any flakiness.
Databases restored from a dump without foreign key enforcement can
contain entries whose feed no longer exists. Migration v127 copied
those rows into entry_tombstones, violating its feed_id foreign key
and aborting the upgrade at schema version 126.
Only backfill tombstones for feeds that still exist: a tombstone for
a deleted feed is never consulted, and the orphaned rows are removed
by the DELETE that follows.
Fixes#4432
The remove_tables rule unwrapped each table element by appending its
inner HTML to the end of the parent node and then removing the element.
Any content located after the table (and the table content itself) was
therefore moved to the bottom of the entry instead of staying in place,
which reordered the article.
Replace the append-then-remove with an in-place ReplaceWithHtml so the
unwrapped content keeps its original document order. The existing
TestRewriteRemoveTables still passes because its content is fully nested
in a single root table; a new test with content surrounding the table
covers the regression.
Fixes#3110
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The "Mark all as read" button on the Unread page calls
MarkGloballyVisibleFeedsAsRead, which only filtered on feeds.hide_globally
and ignored the feed's category. Entries belonging to a category marked as
hidden from the global unread list were therefore marked as read even though
they are not shown on that page.
Join the categories table and exclude both feeds and categories that are
hidden globally, matching the visibility rules already used by the unread
query builder, the pagination builder and the navigation unread counter.
Fixes#4444
Miniflux' default user.EntriesPerPage value is 100, so people with a lot of
feeds/feed items might have 100 items displayed by default, which is a ton of
DOM elements: multiple inline SVG icons, two <button>, optional <time>,
optional reading-time chip, category tag, … the browser had to lay out and
paint all of it on initial render even though there are only a handful of rows
visible at once.
Setting content-visibility: auto on .entry-item and .feed-item lets
the browser skip layout, paint, and accessibility-tree work for rows
that fall outside the viewport, doing it lazily as the user scrolls.
contain-intrinsic-size: auto reserves a placeholder using each row's
last rendered size so the scrollbar and anchor scrolling stay
consistent after first measurement. The value 100px was picked as on my
machine/screen/…, a feed-item is a bit less than 100px, and an entry-item is
a bit more than 80px.
This doesn't introduced any regressions in keyboard shortcuts, both from
miniflux and from the browser. The two CSS properties are supported in current
modern browsers, and are simply ignored in those that don't.
Sources used:
- https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/content-visibility
- https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/contain-intrinsic-size
Tokenizer.Token() allocates a Token struct and parses the full
attribute slice for every start tag, even though stripIter only
consumes text tokens. Switch to checking the TokenType from Next()
and calling Text() only for text tokens.
Shared by StripTags and TruncateHTML, both called per entry:
StripTags 247KB -> 71KB/op (-71%), 12623 -> 4423 allocs (-65%)
TruncateHTML 19KB -> 7.9KB/op (-59%), 819 -> 297 allocs (-64%)
Both roughly 2x faster on tag-heavy input.
Article headings inherit .entry-content's fixed line-height: 1.4em (computed at
the body font-size), which is smaller than the heading font-size, so headings
that wrap onto two lines overlap. Add an explicit unitless line-height: 1.2
scoped to .entry-content headings so the leading scales with each heading's own
font-size, without affecting Miniflux's own UI headings.
Fixes#4399.
JSON Feed 1.1 defines content_html as HTML but content_text and summary
as plain text. The adapter stored whichever was present directly in
entry.Content, which is treated as HTML everywhere downstream, so any
markup-like characters in a content_text or summary value (for example
"<tag>") were dropped by the sanitizer.
Escape content_text and summary with html.EscapeString before storing
them, leaving content_html untouched. This mirrors how the Atom 0.3
reader already escapes plain-text constructs.
For every dynamic response above the 1 KiB compression threshold the
builder previously did one of:
brotli.NewWriterV2(b.w, brotli.DefaultCompression)
gzip.NewWriter(b.w)
flate.NewWriter(b.w, -1)
Each constructor allocates the encoder's working set from scratch:
brotli's sliding window + hash tables (~3.5 MiB at quality 6), gzip's
CRC32 + deflate state (~800 KiB), flate's hash chains (~800 KiB).
On a busy server that's per-request churn the GC then has to clean up.
All three writer types expose Reset(dst io.Writer), which rebinds the
destination without touching the internal buffers. So put each behind a
sync.Pool and Get/Reset/Put around the existing Write+Close pair. The
constructor signatures didn't change; only Pool.Get + Reset are new.
Note that brotli.NewWriterV2 (kept from before this change) returns
*matchfinder.Writer, not *brotli.Writer, as V2 is the pure-Go encoder
built on top of github.com/andybalholm/brotli/matchfinder, where the
actual Writer type lives. Hence the matchfinder import.
On a local artificial benchmarks of a 130 KiB HTML-like payload consisting of
250 entry-list items, on a single-core it improves performances by around 10%,
and for multicore under GC pressure, ns/op is reduces by ~80% and B/op by ~99%.
Render bound per-request, language-specific functions (t, plural, elapsed)
onto the shared template before executing it. Doing that while other
goroutines execute the same template races on its function map and can
render a response using another request's language.
Clone the template before binding those functions so each request executes
its own copy.
Fixes#4380
Entries without their own language now take the feed-level value,
matching the behaviour introduced for Atom. For JSON Feed this is
spec-mandated: an item declares a language only when it differs from
the primary language of the feed. For RSS and RDF, items are part of
the channel's content, and API consumers previously saw an empty
entries.language even when the channel declared one.
The RSS channel now also reads <dc:language>: hybrid feeds commonly
declare the channel language via Dublin Core instead of <language>.
Feed-declared language values were persisted and rendered with no
charset or length restriction. Normalize now rejects values longer
than 50 bytes or containing characters outside [a-z0-9-], keeping
control characters, NUL bytes (which Postgres rejects, failing the
feed refresh), and oversized values out of the database and the HTML
lang attribute. Invalid values are rejected rather than stripped,
since stripping could assemble a wrong tag ("fr, en" -> "fren").
Lower-casing is ASCII-only and done in the same pass as the charset
check: Unicode case folding maps some non-ASCII characters to ASCII
(e.g. the Kelvin sign U+212A to "k"), laundering input the filter
should reject into apparently valid tags.
NormalizeLanguage lived in the model package but is a parse-time
input-cleaning helper: all of its callers are the feed-format adapters,
nothing in model uses it, and as a free function it never enforced a
model invariant. Parse-time normalization helpers belong under
internal/reader alongside date, sanitizer, and urlcleaner.
Move it to a new internal/reader/language package and rename it to
Normalize so call sites read language.Normalize(...). No behaviour
change.
Per the XML specification, xml:lang applies to the whole subtree it is
declared on, so an Atom entry without its own xml:lang attribute takes
the language declared on the feed element. The parser previously left
such entries with an empty language, and only the web UI compensated
with a template-level fallback; API consumers reading entries.language
saw an empty value even when the feed declared one.
Apply the fallback in the Atom 1.0 and 0.3 adapters so the inherited
value is persisted and exposed everywhere.
Known limitation: an explicit xml:lang="" on an entry, which the spec
defines as undefining the language for that subtree, cannot be
distinguished from an absent attribute with a plain string field and
therefore inherits as well.
The Language fields added for Atom 1.0 and 0.3 feeds and entries used
the struct tag `xml:"lang,attr"`, which encoding/xml matches against a
lang attribute from any namespace, with the last one in document order
winning. A feed carrying e.g. foo:lang="zz" after xml:lang="fr" would
be stored with language "zz".
Qualify the tags with the XML namespace so only xml:lang matches. As a
side effect, a bare non-standard lang attribute is now ignored instead
of being treated as the feed language.
Also document each Language field and add regression tests covering
foreign-namespace and unqualified lang attributes, plus first coverage
of xml:lang parsing for Atom 0.3.
Extends language parsing to RDF/RSS 1.0 feeds by reading the
Dublin Core dc:language element at both the channel and item level.
Values are normalized the same way as for RSS 2.0 and Atom.
Renders lang="..." on the entry title (<h1> in detail view, <h2> in every
list view) and on the entry content <article>. The attribute prefers the
entry-level language and falls back to the feed-level language; if both
are empty, no lang= attribute is emitted (rather than lang="").
Reads the language declared by each feed and entry at parse time, persists
it on new `feeds.language` and `entries.language` columns, and exposes both
via the existing Feed/Entry JSON marshalling.
Sources:
- RSS feed: <language>
- RSS item: <dc:language>
- Atom 1.0 feed/entry: xml:lang
- Atom 0.3 feed/entry: xml:lang
- JSON Feed feed/item: "language"
Values are normalized at parse time (trim + lower-case + _ -> -) so they
are directly usable as an HTML lang attribute. No strict BCP-47 validation
is performed: many real feeds use loose values, and silently dropping them
yields worse downstream behaviour than passing them through.
The refresh path treats language as feed/entry-declared metadata and always
trusts the latest fetched value.
The entry list path applied the category_id query parameter twice: once
with validation in findEntries and again without validation in
configureFilters. Drop the unvalidated second application so an invalid
category_id consistently returns a bad request.
The categories handler parsed the counts query parameter by comparing
the raw string to "true". Use QueryBoolParam for consistency with the
other boolean query parameters in the package.
The admin user lookup handlers reported a database error from UserByID
and UserByUsername as a bad request with a generic message. Return a
server error instead, consistent with the other user handlers.
The update-feed handler returned a not found response for any error
from FeedByID, masking genuine database failures. Return a server error
on failure and keep the not found response for a nil feed.
The current-user and categories handlers used the value returned by
UserByID without checking whether it was nil. Since UserByID returns no
error when the user does not exist, the categories handler could
dereference a nil user. Return a not found response when the user is nil.
The mark-user-as-read and integrations-status handlers treated any
error from UserByID as a 404, which masked genuine database failures
and never detected a missing user since UserByID returns no error when
the user does not exist. Return a server error on failure and a not
found response only when the user is nil.
The single-entry endpoint proxified enclosure URLs while the list
endpoints did not, so the same enclosure was returned with different
URLs depending on the endpoint used.
Fred