Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/newznab-categories-actually-searched.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"comicarr": patch
---

The Newznab categories you set are now the categories Comicarr searches. Whatever you typed into **Settings → Search → Categories** was being folded into a legacy field that also carries the indexer's RSS user ID, and the searcher could not tell the two apart — so it fell back to its built-in comics category on every Usenet query, and the Settings page displayed the value you entered as though it were in use. Restricting or widening your categories had no effect, and no error said so.

The RSS user ID now has its own field beside Categories, so each means one thing. Existing indexers are re-read on upgrade: if the box shows fewer categories than you remember typing, that is the part that was actually reaching your indexer, and you can now correct it. Newly added indexers default to `7030` (Books/Comics) instead of `5030`, which is a TV category and was never right for this application.

Two related fixes to how providers are stored. An indexer whose *verify TLS* or *enabled* field was written as `True`/`False` rather than `1`/`0` — the shape produced when a legacy `torznab_*` block is absorbed, and present in some configs inherited from Mylar3 — was reported as enabled on the Acquisition tab while the searcher skipped it entirely, or took the search down with an error when it tried to read the TLS setting. Both fields are now normalised wherever configuration is read or written, so every part of Comicarr agrees on what a provider is set to. And an absorbed `torznab_*` entry now verifies TLS certificates by default rather than silently arriving with verification off.

Finally, the log messages about inert `torznab_*` fields no longer point you at a Settings UI that cannot edit Torznab providers. They now name `extra_torznabs` under `[Torznab]` in `config.ini` and show the entry format.
6 changes: 5 additions & 1 deletion comicarr/app/config/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,11 @@ def as_definition(self) -> tuple[type, str, Any]:
ConfigKey("TORZNAB_HOST", str, "Torznab", None),
ConfigKey("TORZNAB_APIKEY", str, "Torznab", None),
ConfigKey("TORZNAB_CATEGORY", str, "Torznab", None),
ConfigKey("TORZNAB_VERIFY", bool, "Torznab", False),
# Verify TLS by default. This value is only ever read to seed the verify
# field of a legacy torznab_* entry being absorbed into extra_torznabs, and
# an absorbed entry inherits every other field from the operator -- so a
# False here silently downgraded a provider they never chose to downgrade.
ConfigKey("TORZNAB_VERIFY", bool, "Torznab", True),
ConfigKey("EXPERIMENTAL", bool, "Experimental", False),
ConfigKey("ALTEXPERIMENTAL", bool, "Experimental", False),
ConfigKey("TAB_ENABLE", bool, "Tablet", False),
Expand Down
46 changes: 45 additions & 1 deletion comicarr/app/system/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,37 @@ def _http_origin(value):
return scheme, hostname, port


DEFAULT_NEWZNAB_RSS_UID = "1"


def split_newznab_category_field(value):
"""Split a Newznab category field into its RSS uid and its category list.

Field 5 of a Newznab record is ``uid#categories``: the uid is the ``i=``
parameter of the indexer's RSS URL, and everything after the first ``#`` is
the category list. A value with no ``#`` is a uid on its own, which is why
a bare ``7030`` typed into the Settings Categories box was stored as a uid
and searched nothing -- the categories the operator asked for were dropped
on the floor with no error. Returned as ``(uid, categories)`` with the
category separator normalised to a comma for display.

Torznab records have no uid; field 5 there is the category list alone.
"""
uid, separator, categories = str(value or "").partition("#")
if not separator:
return uid, ""
return uid, categories.replace("#", ",")


def join_newznab_category_field(uid, categories):
"""Rebuild the stored ``uid#categories`` field from its two halves."""
uid = str(uid or "").strip() or DEFAULT_NEWZNAB_RSS_UID
categories = str(categories or "").strip().replace(",", "#")
# A uid on its own, rather than a trailing '#', so the search path falls
# back to its built-in category instead of querying `cat=` empty.
return "%s#%s" % (uid, categories) if categories else uid


def _safe_provider_projection(config, provider_type):
"""Build the credential-free provider projection returned by the API."""
attr_name = "EXTRA_NEWZNABS" if provider_type == "newznab" else "EXTRA_TORZNABS"
Expand All @@ -368,6 +399,10 @@ def _safe_provider_projection(config, provider_type):
"enabled": str(entry[5]).lower() in {"1", "true", "yes", "on"},
"api_key_set": _secret_is_configured(entry[3]),
}
if provider_type == "newznab":
rss_uid, categories = split_newznab_category_field(entry[4])
row["rss_uid"] = rss_uid
row["categories"] = categories
if len(entry) >= 7:
try:
row["id"] = int(entry[6])
Expand Down Expand Up @@ -590,12 +625,21 @@ def update_providers(ctx, provider_data):
credential = old[3]
if old is not None and host == _safe_provider_host(old[1]):
host = old[1]
categories = str(row.get("categories") or "").replace(",", "#")
if provider_type == "newznab":
# Keep the uid the operator is already using when the client
# does not send one back, so editing categories cannot silently
# repoint the indexer's RSS feed at a different user.
rss_uid = row.get("rss_uid")
if rss_uid in (None, ""):
rss_uid = split_newznab_category_field(old[4])[0] if old is not None and len(old) >= 5 else None
categories = join_newznab_category_field(rss_uid, row.get("categories"))
normalized_row = [
row.get("name", ""),
host,
"1" if row.get("verify") else "0",
credential or "",
str(row.get("categories") or "").replace(",", "#"),
categories,
"1" if row.get("enabled") else "0",
]
provider_id = (
Expand Down
59 changes: 46 additions & 13 deletions comicarr/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@
_PROVIDER_EXTRA_WIDTHS = (6, 7)
_PROVIDER_CREDENTIAL_INDEX = 3
_PROVIDER_BOOLEAN_VALUES = {"0", "1", "false", "true", "no", "yes", "off", "on"}
_PROVIDER_BOOLEAN_TRUE = {"1", "true", "yes", "on"}
# Verify-TLS and enabled. Both are read as `bool(int(field))` by the search
# path and compared against the literal "1" by the enabled filters, so a field
# spelled any other legal way is a crash or a silent skip -- see
# _canonical_provider_boolean.
_PROVIDER_BOOLEAN_INDEXES = (2, 5)


def config_transaction_lock():
Expand All @@ -61,6 +67,22 @@ def config_transaction_lock():
return _CONFIG_TRANSACTION_LOCK


def _canonical_provider_boolean(value):
"""Return a provider boolean field as the only spelling every reader agrees on.

`_PROVIDER_BOOLEAN_VALUES` accepts eight spellings, but the consumers do
not. `search.py` and `rsscheck.py` read verify as `bool(int(field))`, which
raises `ValueError` on `"True"`; the enabled filters in `search.py` compare
against the literal `"1"` while `health.py` and the providers API accept
`true`/`yes`/`on`. So an entry stored as `True` was reported enabled by the
Acquisition tab and skipped by the searcher -- and one stored with a
non-numeric verify took the search down. Both fields are normalised here,
at the single boundary every reader and writer passes through, so tolerance
at the edge cannot become disagreement in the middle.
"""
return "1" if str(value).strip().lower() in _PROVIDER_BOOLEAN_TRUE else "0"


def _provider_entry_is_structurally_valid(entry):
"""Distinguish historical six- and seven-field provider records safely."""
if len(entry) not in _PROVIDER_EXTRA_WIDTHS:
Expand Down Expand Up @@ -103,7 +125,10 @@ def parse_provider_extras(value, config_version=15):
for entry in entries:
if not isinstance(entry, (list, tuple)) or not _provider_entry_is_structurally_valid(entry):
raise ValueError("Provider entries must contain six or seven fields")
parsed.append(tuple(entry))
values = list(entry)
for index in _PROVIDER_BOOLEAN_INDEXES:
values[index] = _canonical_provider_boolean(values[index])
parsed.append(tuple(values))
return parsed


Expand Down Expand Up @@ -578,8 +603,9 @@ def _scrub():
if missing:
logger.warn(
"[CONFIG] Legacy torznab_* fields under [Torznab] are set but incomplete "
"(missing: %s) and are NOT used for searching. Configure the provider via "
"the Settings UI (extra_torznabs) instead. Ignoring the legacy entry."
"(missing: %s) and are NOT used for searching. Complete them, or add the "
"provider to extra_torznabs under [Torznab] in config.ini as "
"'Name, https://host/api, 1, apikey, 7030, 1'. Ignoring the legacy entry."
% ", ".join("torznab_%s" % m for m in missing)
)
return
Expand All @@ -604,7 +630,7 @@ def _scrub():
logger.warn(
"[CONFIG] Legacy torznab_* fields under [Torznab] reuse the provider name "
"'%s' and are NOT used for searching. Rename or remove the legacy fields, or "
"configure the provider via the Settings UI (extra_torznabs) instead." % legacy["name"]
"edit the existing entry in extra_torznabs under [Torznab] in config.ini." % legacy["name"]
)
return

Expand All @@ -619,17 +645,20 @@ def _scrub():
(
legacy["name"],
legacy["host"],
self.TORZNAB_VERIFY,
# Canonical "1"/"0", not the raw bool: serialised it would
# reach the next startup as "True", and the search path reads
# this field as bool(int(field)).
_canonical_provider_boolean(self.TORZNAB_VERIFY),
legacy["apikey"],
legacy["category"],
str(int(bool(self.ENABLE_TORZNAB))),
_canonical_provider_boolean(self.ENABLE_TORZNAB),
candidate_id,
)
)
logger.info(
"[CONFIG] Migrated legacy torznab_* fields under [Torznab] into extra_torznabs "
"as provider '%s' (%s). The legacy single-provider fields are no longer read; "
"manage this provider via the Settings UI from now on." % (legacy["name"], legacy["host"])
"manage this provider through extra_torznabs from now on." % (legacy["name"], legacy["host"])
Comment thread
coderabbitai[bot] marked this conversation as resolved.
)
_scrub()

Expand Down Expand Up @@ -2456,13 +2485,17 @@ def get_extras(self):
ex = extra_torznabs

for x in ex:
# Field 5 is passed through exactly as stored. It used to be
# rewritten here -- '#' to ',', then the leading ',' back to
# '#' -- which left the runtime value in a shape none of its
# readers expected: `search.py` tests for '#' to decide whether
# a category was configured at all, so a stored `1#7030`
# arrived as `1,7030`, failed that test, and every Newznab
# search silently fell back to the built-in 7030. Only a value
# that already began with '#' survived the round trip. One
# storage contract now, read the same way by search.py,
# rsscheck.py, and the providers API.
x_cat = x[4]
if x_cat:
if "#" in x_cat:
x_t = x[4].split("#")
x_cat = ",".join(x_t)
if x_cat[0] == ",":
x_cat = re.sub(",", "#", x_cat, 1)
try:
if cnt == 0:
x_newzcat.append((x[0], x[1], x[2], x[3], x_cat, x[5], int(x[6])))
Expand Down
33 changes: 31 additions & 2 deletions frontend/src/components/settings/SearchTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,12 @@ function NewznabProviderForm({
name: "",
host: "",
verify: true,
categories: "5030",
// 7030 is Books/Comics in the standard Newznab category numbering.
// The old 5030 default was a TV category — harmless while categories
// were being discarded before reaching the search, and wrong now that
// they are not.
categories: "7030",
rss_uid: "1",
enabled: true,
api_key_set: false,
api_key: "",
Expand Down Expand Up @@ -265,13 +270,37 @@ function NewznabProviderForm({
id={`indexer-categories-${suffix}`}
className="mt-1.5"
value={provider.categories}
placeholder="5030"
placeholder="7030"
onChange={(event) =>
updateProvider(index, {
categories: event.target.value,
})
}
/>
<p className="mt-1 text-[11px] text-muted-foreground">
Newznab category IDs, comma-separated. 7030 is
Books/Comics.
</p>
</div>
<div>
<Label htmlFor={`indexer-rss-uid-${suffix}`}>
RSS user ID
</Label>
<Input
id={`indexer-rss-uid-${suffix}`}
className="mt-1.5"
value={provider.rss_uid ?? ""}
placeholder="1"
onChange={(event) =>
updateProvider(index, {
rss_uid: event.target.value,
})
}
/>
<p className="mt-1 text-[11px] text-muted-foreground">
The <code>i=</code> parameter of this indexer&rsquo;s RSS
URL. Leave as 1 unless your indexer says otherwise.
</p>
</div>
</div>
<div className="mt-3 grid gap-2 sm:grid-cols-2">
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/types/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ export interface NewznabProvider {
enabled: boolean;
api_key_set: boolean;
api_key?: string;
/**
* The `i=` parameter of the indexer's RSS URL. Stored joined to the
* categories as `uid#categories`, split apart by the API so the categories
* field means categories. Newznab only — Torznab records have no uid.
*/
rss_uid?: string;
}

export interface ProviderConfigResponse {
Expand Down
55 changes: 55 additions & 0 deletions frontend/tests/pages/SettingsPage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,61 @@ describe("settings configuration", () => {
);
});

it("edits Newznab categories and the RSS user ID as separate fields", async () => {
let saved: unknown = null;
server.use(
http.get("/api/config/providers", () =>
HttpResponse.json({
newznab: {
enabled: true,
providers: [
{
id: 101,
name: "Indexer",
host: "https://indexer.test",
verify: true,
// Server-side split of the stored `42#7030` field. The uid
// used to be folded into the Categories box, where editing
// categories quietly rewrote it.
categories: "7030",
rss_uid: "42",
enabled: true,
api_key_set: true,
},
],
},
}),
),
http.put("/api/config/providers", async ({ request }) => {
saved = await request.json();
return HttpResponse.json({ success: true });
}),
);
const user = userEvent.setup();

render(createElement(SettingsPage));
await screen.findByText("Settings");
await user.click(screen.getAllByRole("button", { name: "Search" })[0]);

const categories = await screen.findByLabelText("Categories");
expect((categories as HTMLInputElement).value).toBe("7030");
expect(
(screen.getByLabelText("RSS user ID") as HTMLInputElement).value,
).toBe("42");

await user.clear(categories);
await user.type(categories, "7030,7020");
await user.click(screen.getByRole("button", { name: "Save indexers" }));

await waitFor(() => expect(saved).not.toBeNull());
expect(saved).toMatchObject({
type: "newznab",
providers: [
expect.objectContaining({ categories: "7030,7020", rss_uid: "42" }),
],
});
});

it("requires a replacement indexer key when its server changes", async () => {
let saved: unknown = null;
server.use(
Expand Down
Loading
Loading