fix(update): fetch release assets without metadata

Author: ReKylee

src/settings/SettingsModel.h

@@ -13,6 +13,7 @@
 namespace settings {
 
     inline constexpr std::string_view kMathFontTarget = "math";
+    inline constexpr std::string_view kDefaultRepositoryOwner = "ionutdecebal";
 
     enum class ReadingMode : uint8_t {
         rsvp,

src/ui/screens/NetworkSettingsScreen.cpp

@@ -43,8 +43,8 @@ namespace screens {
         const int16_t thirdRowY = static_cast<int16_t>(secondRowY + rowHeight + gap);
         const int16_t halfWidth = static_cast<int16_t>((content.w - gap) / 2);
         if (ui.setting({content.x, thirdRowY, halfWidth, rowHeight}, ui.text(UiText::OtaOwner),
-                       settings.updates.repositoryOwner.empty() ? ui.text(UiText::Default)
-                                                                : std::string_view{settings.updates.repositoryOwner})) {
+                       settings.updates.repositoryOwner.empty() ? settings::kDefaultRepositoryOwner
+                                                                 : std::string_view{settings.updates.repositoryOwner})) {
             editField_ = EditField::Owner;
             editValue_ = settings.updates.repositoryOwner;
             keyboard_ = {};

src/update/OtaUpdater.cpp

@@ -17,7 +17,6 @@
 
 namespace {
 
-    constexpr size_t kMaxReleaseJsonBytes = 32768;
     constexpr const char* kStatusTitle = "OTA";
     const char* kRedirectHeaderKeys[] = {
         "Location",
@@ -75,7 +74,7 @@ namespace {
     }
 
     ReleaseSource releaseSourceForSettings(const settings::UpdateSettings& settings) {
-        ReleaseSource source{settings.repositoryOwner.empty() ? std::string{"ionutdecebal"}
+        ReleaseSource source{settings.repositoryOwner.empty() ? std::string{settings::kDefaultRepositoryOwner}
                                                               : std::string{AsciiText::trim(settings.repositoryOwner)},
                              "rsvpnano", std::string{AsciiText::trim(settings.releaseTag)}};
 
@@ -103,6 +102,12 @@ namespace {
         return std::string{prefix} + " " + std::string{detail.c_str(), detail.length()};
     }
 
+    bool isRedirectStatus(int statusCode) {
+        return statusCode == HTTP_CODE_MOVED_PERMANENTLY || statusCode == HTTP_CODE_FOUND
+            || statusCode == HTTP_CODE_SEE_OTHER || statusCode == HTTP_CODE_TEMPORARY_REDIRECT
+            || statusCode == HTTP_CODE_PERMANENT_REDIRECT;
+    }
+
     std::string readBodyLimited(HTTPClient& http, size_t maxBytes) {
         WiFiClient* stream = http.getStreamPtr();
         if (stream == nullptr) {
@@ -178,57 +183,68 @@ std::string_view OtaUpdater::currentVersion() {
 
 static void reportStatus(OtaUpdater::StatusCallback callback, void* context, const char* title, const char* line1,
                          const char* line2, int progressPercent);
+static std::expected<std::string, std::string> resolveDownloadUrl(std::string_view assetUrl, std::string_view version,
+                                                                  OtaUpdater::StatusCallback callback, void* context);
 
 static std::expected<LatestRelease, std::string> fetchRelease(const settings::UpdateSettings& settings,
-                                                              OtaUpdater::StatusCallback callback, void* context) {
+                                                               OtaUpdater::StatusCallback callback, void* context) {
     const std::string installedVersion{OtaUpdater::currentVersion()};
     const ReleaseSource source = releaseSourceForSettings(settings);
     if (source.owner.empty() || source.repo.empty())
         return std::unexpected(std::string{"GitHub source missing"});
 
-    const std::string releasePath = source.tag.empty() ? "latest" : "tags/" + urlEncodePathSegment(source.tag);
-    const std::string url =
-        "https://api.github.com/repos/" + source.owner + "/" + source.repo + "/releases/" + releasePath;
     const std::string sourceLabel = source.tag.empty() ? source.repo : source.repo + ":" + source.tag;
 
     reportStatus(callback, context, kStatusTitle, "Checking GitHub", sourceLabel.c_str(), 22);
 
     WiFiClientSecure client;
-    // GitHub release metadata and assets can redirect across multiple hosts, so
-    // keep the transport flexible for now. A signed manifest is the best
-    // follow-up hardening step.
+    // GitHub release assets redirect across multiple hosts, so keep the
+    // transport flexible for now. A signed manifest is the best follow-up
+    // hardening step.
     client.setInsecure();
     client.setHandshakeTimeout(15);
 
     HTTPClient http;
+    http.collectHeaders(kRedirectHeaderKeys, 1);
     http.setUserAgent(userAgentForVersion(installedVersion).c_str());
-    http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
+    http.setFollowRedirects(HTTPC_DISABLE_FOLLOW_REDIRECTS);
     http.setTimeout(15000);
-    if (!http.begin(client, url.c_str()))
-        return std::unexpected(std::string{"HTTP begin failed"});
 
-    http.addHeader("Accept", "application/vnd.github+json");
-    const int statusCode = http.GET();
-    if (statusCode != HTTP_CODE_OK) {
-        const std::string errorDetail = statusCode == HTTP_CODE_NOT_FOUND
-                                          ? (source.tag.empty() ? "No published release" : "Release tag not found")
-                                          : httpClientErrorDetail("GitHub", statusCode);
+    const std::string assetName = urlEncodePathSegment(Board::Config::OTA_ASSET_NAME);
+    std::string releaseTag = source.tag;
+    std::string assetUrl = "https://github.com/" + source.owner + "/" + source.repo + "/releases/";
+    if (releaseTag.empty())
+        assetUrl += "latest/download/" + assetName;
+    else
+        assetUrl += "download/" + urlEncodePathSegment(releaseTag) + "/" + assetName;
+
+    if (releaseTag.empty()) {
+        if (!http.begin(client, assetUrl.c_str()))
+            return std::unexpected(std::string{"HTTP begin failed"});
+
+        http.addHeader("Accept", "application/octet-stream");
+        const int statusCode = http.GET();
+        if (!isRedirectStatus(statusCode)) {
+            const std::string errorDetail = statusCode == HTTP_CODE_NOT_FOUND
+                                              ? "No latest OTA asset"
+                                              : httpClientErrorDetail("GitHub", statusCode);
+            http.end();
+            return std::unexpected(errorDetail);
+        }
+
+        const String location = http.header("Location");
         http.end();
-        return std::unexpected(errorDetail);
+        const std::string_view resolvedUrl{location.c_str(), static_cast<size_t>(location.length())};
+        auto parsedTag = releaseparser::tagFromAssetLocation(resolvedUrl, Board::Config::OTA_ASSET_NAME);
+        if (!parsedTag)
+            return std::unexpected(std::string{"Release redirect invalid"});
+        releaseTag = std::move(*parsedTag);
+        assetUrl.assign(resolvedUrl);
     }
 
-    const std::string body = readBodyLimited(http, kMaxReleaseJsonBytes);
-    http.end();
-
-    auto parsed = releaseparser::parse(body, Board::Config::OTA_ASSET_NAME);
-    if (!parsed)
-        return std::unexpected(std::string{"Release tag missing"});
-    LatestRelease release;
-    release.assetUrl = parsed->assetUrl;
-
-    reportStatus(callback, context, kStatusTitle, "Checking version", parsed->tagName.c_str(), 25);
+    reportStatus(callback, context, kStatusTitle, "Checking version", releaseTag.c_str(), 25);
     const std::string commitUrl = "https://api.github.com/repos/" + source.owner + "/" + source.repo + "/commits/"
-                                + urlEncodePathSegment(parsed->tagName);
+                                + urlEncodePathSegment(releaseTag);
     if (!http.begin(client, commitUrl.c_str()))
         return std::unexpected(std::string{"Commit lookup failed"});
     http.addHeader("Accept", "application/vnd.github.sha");
@@ -240,22 +256,25 @@ static std::expected<LatestRelease, std::string> fetchRelease(const settings::Up
     }
     std::string commitSha = readBodyLimited(http, 64);
     http.end();
-    auto releaseVersion = releaseparser::versionForCommit(parsed->tagName, commitSha);
+    auto releaseVersion = releaseparser::versionForCommit(releaseTag, commitSha);
     if (!releaseVersion)
         return std::unexpected(std::string{"Tag commit invalid"});
-    release.version = std::move(*releaseVersion);
 
-    if (release.assetUrl.empty())
-        return std::unexpected(std::string{Board::Config::OTA_ASSET_NAME} + " missing");
+    auto resolvedAssetUrl = resolveDownloadUrl(assetUrl, *releaseVersion, callback, context);
+    if (!resolvedAssetUrl)
+        return std::unexpected(std::move(resolvedAssetUrl.error()));
 
-    return release;
+    return LatestRelease{
+        .version = std::move(*releaseVersion),
+        .assetUrl = std::move(*resolvedAssetUrl),
+    };
 }
 
 static std::expected<std::string, std::string> resolveDownloadUrl(std::string_view assetUrl, std::string_view version,
                                                                   OtaUpdater::StatusCallback callback, void* context) {
     const std::string assetUrlString{assetUrl};
     const std::string versionString{version};
-    reportStatus(callback, context, kStatusTitle, "Resolving asset", versionString.c_str(), 29);
+    reportStatus(callback, context, kStatusTitle, "Resolving asset", versionString.c_str(), 27);
 
     WiFiClientSecure client;
     client.setInsecure();
@@ -276,8 +295,7 @@ static std::expected<std::string, std::string> resolveDownloadUrl(std::string_vi
         return std::string{assetUrl};
     }
 
-    if (statusCode == HTTP_CODE_MOVED_PERMANENTLY || statusCode == HTTP_CODE_FOUND || statusCode == HTTP_CODE_SEE_OTHER
-        || statusCode == HTTP_CODE_TEMPORARY_REDIRECT || statusCode == HTTP_CODE_PERMANENT_REDIRECT) {
+    if (isRedirectStatus(statusCode)) {
         String resolvedUrl = http.header("Location");
         http.end();
         if (!resolvedUrl.isEmpty())
@@ -386,15 +404,6 @@ OtaUpdater::Result OtaUpdater::checkAndInstall(const settings::DeviceSettings& s
     const std::string detail = versionDetail(installedVersion, release->version);
     reportStatus(callback, context, kStatusTitle, "Preparing update", detail.c_str(), 28);
 
-    auto resolvedAssetUrl = resolveDownloadUrl(release->assetUrl, release->version, callback, context);
-    if (!resolvedAssetUrl) {
-        net::disconnect();
-        return {
-            .summary = "Asset failed",
-            .detail = std::move(resolvedAssetUrl.error()),
-        };
-    }
-
     WiFiClientSecure client;
     // Match the metadata request behavior until the update path gains certificate
     // pinning or signature verification above the transport layer.
@@ -422,7 +431,7 @@ OtaUpdater::Result OtaUpdater::checkAndInstall(const settings::DeviceSettings& s
     });
 
     const String version = installedVersion.data();
-    const String resolvedUrl = resolvedAssetUrl->c_str();
+    const String resolvedUrl = release->assetUrl.c_str();
     const t_httpUpdate_return updateResult = updater.update(client, resolvedUrl, version, [version](HTTPClient* http) {
         http->setUserAgent(userAgentForVersion({version.c_str(), version.length()}).c_str());
         http->addHeader("Accept", "application/octet-stream");

src/update/ReleaseParser.cpp

@@ -2,38 +2,27 @@
 
 #include <algorithm>
 #include <cctype>
-#include <vector>
-
-#include <glaze/json.hpp>
 
 #include "text/AsciiText.h"
 
 namespace releaseparser {
-    namespace {
-
-        struct Asset {
-            std::string name;
-            std::string browser_download_url;
-        };
-
-        struct Release {
-            std::string tag_name;
-            std::vector<Asset> assets;
-        };
-
-    } // namespace
-
-    std::expected<ReleaseInfo, std::error_code> parse(std::string_view json, std::string_view assetName) {
-        Release release;
-        if (glz::read<glz::opts{.error_on_unknown_keys = false}>(release, json) || release.tag_name.empty()) {
+    std::expected<std::string, std::error_code> tagFromAssetLocation(std::string_view location,
+                                                                    std::string_view assetName) {
+        constexpr std::string_view marker = "/releases/download/";
+        const size_t start = location.find(marker);
+        if (start == std::string_view::npos || assetName.empty() || !location.ends_with(assetName)) {
             return std::unexpected(std::make_error_code(std::errc::invalid_argument));
         }
 
-        const auto asset = std::ranges::find(release.assets, assetName, &Asset::name);
-        return ReleaseInfo{
-            .tagName = std::move(release.tag_name),
-            .assetUrl = asset == release.assets.end() ? std::string{} : std::move(asset->browser_download_url),
-        };
+        const size_t tagStart = start + marker.size();
+        const size_t assetStart = location.size() - assetName.size();
+        if (assetStart <= tagStart || location[assetStart - 1] != '/')
+            return std::unexpected(std::make_error_code(std::errc::invalid_argument));
+
+        const std::string_view tag = location.substr(tagStart, assetStart - tagStart - 1);
+        if (tag.empty() || tag.contains('/'))
+            return std::unexpected(std::make_error_code(std::errc::invalid_argument));
+        return std::string{tag};
     }
 
     std::expected<std::string, std::error_code> versionForCommit(std::string_view tagName, std::string_view commitSha) {

src/update/ReleaseParser.h

@@ -5,17 +5,11 @@
 #include <string_view>
 #include <system_error>
 
-// Pure parsing of a GitHub release JSON payload. No networking, no
-// SD access -- safe to unit test on the host.
+// Pure parsing of GitHub release metadata. No networking or SD access.
 namespace releaseparser {
 
-    struct ReleaseInfo {
-        std::string tagName; // empty if the payload had no usable tag_name
-        std::string assetUrl; // empty if no asset matched assetName
-    };
-
-    // Extracts the release tag and the browser_download_url of the matching asset.
-    std::expected<ReleaseInfo, std::error_code> parse(std::string_view json, std::string_view assetName);
+    std::expected<std::string, std::error_code> tagFromAssetLocation(std::string_view location,
+                                                                    std::string_view assetName);
 
     // Published builds use the release tag plus a stable abbreviated commit.
     std::expected<std::string, std::error_code> versionForCommit(std::string_view tagName, std::string_view commitSha);

test/test_release_parser/test_main.cpp

@@ -2,70 +2,26 @@
 
 #include "update/ReleaseParser.h"
 
-namespace {
-
-    const char* kAsset = "rsvp-nano-esp32-s3-touch-lcd-3.49-ota.bin";
-
-    releaseparser::ReleaseInfo parseJson(std::string_view json) {
-        return releaseparser::parse(json, kAsset).value();
-    }
-
-} // namespace
-
 void setUp() {}
 
 void tearDown() {}
 
-void test_extracts_tag_and_matching_asset_url() {
-    constexpr std::string_view json = "{\"tag_name\":\"v0.0.6\",\"assets\":["
-                        "{\"name\":\"other.bin\",\"browser_download_url\":\"https://example.com/other.bin\"},"
-                        "{\"name\":\"rsvp-nano-esp32-s3-touch-lcd-3.49-ota.bin\",\"browser_download_url\":\"https://"
-                        "example.com/ota.bin\"}"
-                        "]}";
-    const auto out = releaseparser::parse(json, kAsset);
-    TEST_ASSERT_TRUE(out.has_value());
-    TEST_ASSERT_EQUAL_STRING("v0.0.6", out->tagName.c_str());
-    TEST_ASSERT_EQUAL_STRING("https://example.com/ota.bin", out->assetUrl.c_str());
-}
-
-void test_returns_false_when_tag_missing() {
-    constexpr std::string_view json = "{\"assets\":[]}";
-    const auto out = releaseparser::parse(json, kAsset);
-    TEST_ASSERT_FALSE(out.has_value());
-    TEST_ASSERT_TRUE(out.error() == std::errc::invalid_argument);
-}
-
-void test_asset_url_empty_when_no_asset_matches() {
-    constexpr std::string_view json = "{\"tag_name\":\"v1.2.3\",\"assets\":["
-                        "{\"name\":\"wrong.bin\",\"browser_download_url\":\"https://example.com/wrong.bin\"}]}";
-    const releaseparser::ReleaseInfo out = parseJson(json);
-    TEST_ASSERT_EQUAL_STRING("v1.2.3", out.tagName.c_str());
-    TEST_ASSERT_TRUE(out.assetUrl.empty());
-}
-
-void test_handles_whitespace_after_colon() {
-    constexpr std::string_view json = "{ \"tag_name\" :   \"v9\" }";
-    const releaseparser::ReleaseInfo out = parseJson(json);
-    TEST_ASSERT_EQUAL_STRING("v9", out.tagName.c_str());
-}
-
-void test_unescapes_forward_slashes_in_url() {
-    constexpr std::string_view json = "{\"tag_name\":\"v2\",\"assets\":["
-                        "{\"name\":\"rsvp-nano-esp32-s3-touch-lcd-3.49-ota.bin\","
-                        "\"browser_download_url\":\"https:\\/\\/example.com\\/path\\/ota.bin\"}]}";
-    const releaseparser::ReleaseInfo out = parseJson(json);
-    TEST_ASSERT_EQUAL_STRING("https://example.com/path/ota.bin", out.assetUrl.c_str());
+void test_extracts_tag_from_asset_redirect() {
+    constexpr std::string_view asset = "rsvp-nano-esp32-s3-touch-lcd-3.49-ota.bin";
+    const auto tag = releaseparser::tagFromAssetLocation(
+        "https://github.com/ionutdecebal/rsvpnano/releases/download/v0.0.9/"
+        "rsvp-nano-esp32-s3-touch-lcd-3.49-ota.bin",
+        asset);
+    TEST_ASSERT_TRUE(tag.has_value());
+    TEST_ASSERT_EQUAL_STRING("v0.0.9", tag->c_str());
 }
 
-void test_picks_url_after_matching_name_not_a_neighbor() {
-    // The matching asset is the second entry; its url must come from after its name.
-    constexpr std::string_view json = "{\"tag_name\":\"v3\",\"assets\":["
-                        "{\"name\":\"a.bin\",\"browser_download_url\":\"https://example.com/a.bin\"},"
-                        "{\"name\":\"rsvp-nano-esp32-s3-touch-lcd-3.49-ota.bin\",\"browser_download_url\":\"https://"
-                        "example.com/correct.bin\"}"
-                        "]}";
-    const releaseparser::ReleaseInfo out = parseJson(json);
-    TEST_ASSERT_EQUAL_STRING("https://example.com/correct.bin", out.assetUrl.c_str());
+void test_rejects_invalid_asset_redirects() {
+    TEST_ASSERT_FALSE(releaseparser::tagFromAssetLocation("https://github.com/releases/latest", "firmware.bin")
+                          .has_value());
+    TEST_ASSERT_FALSE(releaseparser::tagFromAssetLocation("https://github.com/releases/download/v1/other.bin",
+                                                          "firmware.bin")
+                          .has_value());
 }
 
 void test_builds_version_from_release_tag_and_commit() {
@@ -78,12 +34,8 @@ void test_builds_version_from_release_tag_and_commit() {
 
 int main(void) {
     UNITY_BEGIN();
-    RUN_TEST(test_extracts_tag_and_matching_asset_url);
-    RUN_TEST(test_returns_false_when_tag_missing);
-    RUN_TEST(test_asset_url_empty_when_no_asset_matches);
-    RUN_TEST(test_handles_whitespace_after_colon);
-    RUN_TEST(test_unescapes_forward_slashes_in_url);
-    RUN_TEST(test_picks_url_after_matching_name_not_a_neighbor);
+    RUN_TEST(test_extracts_tag_from_asset_redirect);
+    RUN_TEST(test_rejects_invalid_asset_redirects);
     RUN_TEST(test_builds_version_from_release_tag_and_commit);
     return UNITY_END();
 }