From d59a9522ee754058a4ff03809592271059f8ae52 Mon Sep 17 00:00:00 2001 From: Shane McGovern Date: Fri, 7 Aug 2026 10:49:10 +0100 Subject: [PATCH] Fix mod update checks failing on non-JSON responses (#931) The mod update and Find Mods feeds feed the raw HTTP body straight to Json.decode. When the endpoint hands back something that is not JSON (an HTML error page, a proxy/captive prompt, or a plain-text outage message like "Exceeded secondary rate limit" -- usually still HTTP 200), the decoder's "unexpected character 'E'" assert escaped through the pcall and became the error message, blaming the parser instead of the response. Add Json.describeUnexpected() as a pre-decode content-type guard: it returns nil for body shapes the endpoints actually publish (JSON object or array) and otherwise a short message naming what the server sent (HTML page / plain text / empty, with a preview). Wire it into ModUpdate.parseReleases and ModIndex.parse, so both the sync and async update-check paths surface the real answer instead of the parse error. HTTP status was already checked upstream by HostShell.httpGet (non-2xx becomes "HTTP from (...)"); this closes the remaining "2xx but not JSON" gap everywhere, including bridge platforms that expose no status or headers. Add regression tests for plain-text, HTML, and empty bodies; strengthen the ModIndex HTML soft-fail test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/link/Json.lua | 22 ++++++++++++++++++++++ src/mods/ModIndex.lua | 4 +++- src/mods/ModUpdate.lua | 4 +++- tests/engine/mod_index_tests.lua | 6 +++++- tests/engine/mod_update_tests.lua | 25 +++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 3 deletions(-) diff --git a/src/link/Json.lua b/src/link/Json.lua index 9ddb2e30..b6bd58a4 100644 --- a/src/link/Json.lua +++ b/src/link/Json.lua @@ -171,4 +171,26 @@ function Json.decode(s) return nil, v end +-- For an HTTP response that was meant to carry JSON but did not. Returns nil +-- when `s` starts like a JSON object or array (the only shapes the update and +-- index endpoints publish), otherwise a short message naming what the server +-- actually sent -- so callers surface "the response was an HTML page/plain +-- text, not JSON (it starts with ...)" instead of leaking the decoder's +-- low-level "unexpected character 'E'" assert at the first byte of an error +-- page or plain-text outage message. +function Json.describeUnexpected(s) + if type(s) ~= "string" then + return "the response had no body to decode" + end + local first = s:match("^%s*(.)") + if first == "{" or first == "[" then return nil end + local preview = s:gsub("%s+", " "):gsub("^%s+", ""):gsub("%s+$", "") + if preview == "" then + return "the response was empty, not JSON" + end + if #preview > 60 then preview = preview:sub(1, 57) .. "..." end + local kind = (first == "<") and "an HTML page" or "plain text" + return ("the response was %s, not JSON (it starts with %q)"):format(kind, preview) +end + return Json diff --git a/src/mods/ModIndex.lua b/src/mods/ModIndex.lua index c1e145fa..3988b9a9 100644 --- a/src/mods/ModIndex.lua +++ b/src/mods/ModIndex.lua @@ -191,8 +191,10 @@ end -- Never throws: a truncated download, an HTML error page, or a feed from a -- future schema all come back as a message the panel can print. function ModIndex.parse(jsonText, Json) + Json = Json or require("src.link.Json") + local notJson = Json.describeUnexpected(jsonText) + if notJson then return nil, notJson end local ok, result, err = pcall(function() - Json = Json or require("src.link.Json") local doc, decodeErr = Json.decode(jsonText) if type(doc) ~= "table" then return nil, decodeErr or "index.json is not an object" diff --git a/src/mods/ModUpdate.lua b/src/mods/ModUpdate.lua index 8087b642..4768bf70 100644 --- a/src/mods/ModUpdate.lua +++ b/src/mods/ModUpdate.lua @@ -140,8 +140,10 @@ end -- Decode a releases array (GET /repos/.../releases) into a sorted list -- (newest first). Releases without a .zip asset are dropped. Never throws. function ModUpdate.parseReleases(jsonText, modId, Json) + Json = Json or require("src.link.Json") + local notJson = Json.describeUnexpected(jsonText) + if notJson then return nil, notJson end local ok, result, err = pcall(function() - Json = Json or require("src.link.Json") local doc, decodeErr = Json.decode(jsonText) if type(doc) ~= "table" then return nil, decodeErr or "releases json is not an array" diff --git a/tests/engine/mod_index_tests.lua b/tests/engine/mod_index_tests.lua index 82076453..87e96222 100644 --- a/tests/engine/mod_index_tests.lua +++ b/tests/engine/mod_index_tests.lua @@ -142,7 +142,11 @@ do index, err = ModIndex.parse(Json.encode({ mods = { NUZLOCKE } })) check(index == nil and err ~= nil, "a feed with no schema_version is refused") index, err = ModIndex.parse("404") - check(index == nil and err ~= nil, "an HTML error page soft-fails") + check(index == nil and tostring(err):find("HTML", 1, true) ~= nil, + "an HTML error page is named, not blamed on the parser") + index, err = ModIndex.parse("Error: upstream unavailable") + check(index == nil and tostring(err):find("not JSON", 1, true) ~= nil, + "a plain-text error names the response") index, err = ModIndex.parse('{"schema_version":1}') check(index == nil and err ~= nil, "a feed with no mods array soft-fails") end diff --git a/tests/engine/mod_update_tests.lua b/tests/engine/mod_update_tests.lua index d6ff3239..a0b3306e 100644 --- a/tests/engine/mod_update_tests.lua +++ b/tests/engine/mod_update_tests.lua @@ -76,6 +76,31 @@ do check(path == nil and dlErr ~= nil, "empty url soft-fails") end +-- the reported bug: a non-JSON answer (plain-text error, proxy/captive +-- prompt, outage message) used to leak the decoder's "unexpected character" +-- assert at the first byte of the body. The guard must name what the server +-- actually sent and never let that assert surface. +do + local list, err = ModUpdate.parseReleases("Error: API rate limit exceeded", "demo") + check(list == nil and err ~= nil, "plain-text error soft-fails") + check(tostring(err):find("not JSON", 1, true) ~= nil + and tostring(err):find("Error: API", 1, true) ~= nil, + "plain-text error names the response and previews what it said") + list, err = ModUpdate.parseReleases("502 Bad Gateway", "demo") + check(list == nil and tostring(err):find("HTML", 1, true) ~= nil, + "an HTML error page is named as such") + list, err = ModUpdate.parseReleases("", "demo") + check(list == nil and tostring(err):find("empty", 1, true) ~= nil, + "an empty response is named") + check(tostring(err):find("unexpected character", 1, true) == nil, + "the decoder's assert never leaks into the message") + list = ModUpdate.parseReleases(Json.encode({ + { tag_name = "v1.0.0", assets = { + { name = "demo-1.0.0.zip", browser_download_url = "https://x/d.zip" } } }, + }), "demo") + eq(#list, 1, "the guard lets real JSON through") +end + do local body = Json.encode({ tag_name = "v2.0.0",