Index: binaries/data/mods/mod/gui/modio/modio.js =================================================================== --- binaries/data/mods/mod/gui/modio/modio.js +++ binaries/data/mods/mod/gui/modio/modio.js @@ -175,11 +175,11 @@ mod1.filesize - mod2.filesize : String(mod1[modsAvailableList.selected_column]).localeCompare(String(mod2[modsAvailableList.selected_column])))); - modsAvailableList.list_name = displayedMods.map(mod => mod.name); - modsAvailableList.list_name_id = displayedMods.map(mod => mod.name_id); - modsAvailableList.list_version = displayedMods.map(mod => mod.version); - modsAvailableList.list_filesize = displayedMods.map(mod => filesizeToString(mod.filesize)); - modsAvailableList.list_dependencies = displayedMods.map(mod => (mod.dependencies || []).join(" ")); + modsAvailableList.list_name = displayedMods.map(mod => compatibilityColor(mod.name, !mod.invalid)); + modsAvailableList.list_name_id = displayedMods.map(mod => compatibilityColor(mod.name_id, !mod.invalid)); + modsAvailableList.list_version = displayedMods.map(mod => compatibilityColor(mod.version || "", !mod.invalid)); + modsAvailableList.list_filesize = displayedMods.map(mod => compatibilityColor(mod.fileSize !== undefined ? filesizeToString(mod.filesize) : "", !mod.invalid)); + modsAvailableList.list_dependencies = displayedMods.map(mod => compatibilityColor((mod.dependencies || []).join(" "), !mod.invalid)); modsAvailableList.list = displayedMods.map(mod => mod.i); modsAvailableList.selected = modsAvailableList.list.indexOf(selectedMod); } @@ -202,11 +202,19 @@ return +modsAvailableList.list[modsAvailableList.selected]; } +function isSelectedModInvalid(selected) +{ + return selected !== undefined && !!g_ModsAvailableOnline[selected].invalid && g_ModsAvailableOnline[selected].invalid == "true" +} + function showModDescription() { let selected = selectedModIndex(); - Engine.GetGUIObjectByName("downloadButton").enabled = selected !== undefined; - Engine.GetGUIObjectByName("modDescription").caption = selected !== undefined ? g_ModsAvailableOnline[selected].summary : ""; + let isSelected = selected !== undefined; + let isInvalid = isSelectedModInvalid(selected); + Engine.GetGUIObjectByName("downloadButton").enabled = isSelected && !isInvalid; + Engine.GetGUIObjectByName("modDescription").caption = isSelected && !isInvalid ? g_ModsAvailableOnline[selected].summary : ""; + Engine.GetGUIObjectByName("modError").caption = isSelected && isInvalid ? g_ModsAvailableOnline[selected].error : ""; } function cancelModListUpdate() @@ -244,6 +252,9 @@ { let selected = selectedModIndex(); + if (isSelectedModInvalid(selected)) + return; + progressDialog( sprintf(translate("Downloading ā€œ%(modname)sā€"), { "modname": g_ModsAvailableOnline[selected].name Index: binaries/data/mods/mod/gui/modio/modio.xml =================================================================== --- binaries/data/mods/mod/gui/modio/modio.xml +++ binaries/data/mods/mod/gui/modio/modio.xml @@ -66,6 +66,7 @@ + Index: source/ps/ModIo.h =================================================================== --- source/ps/ModIo.h +++ source/ps/ModIo.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2019 Wildfire Games. +/* Copyright (C) 2020 Wildfire Games. * * Permission is hereby granted, free of charge, to any person obtaining * a copy of this software and associated documentation files (the @@ -166,14 +166,14 @@ void TearDownRequest(); bool ParseGameId(const ScriptInterface& scriptInterface, std::string& err); - bool ParseMods(const ScriptInterface& scriptInterface, std::string& err); + bool ParseMods(const ScriptInterface& scriptInterface, std::vector& err); void DeleteDownloadedFile(); bool VerifyDownloadedFile(std::string& err); // Utility methods for parsing mod.io responses and metadata static bool ParseGameIdResponse(const ScriptInterface& scriptInterface, const std::string& responseData, int& id, std::string& err); - static bool ParseModsResponse(const ScriptInterface& scriptInterface, const std::string& responseData, std::vector& modData, const PKStruct& pk, std::string& err); + static bool ParseModsResponse(const ScriptInterface& scriptInterface, const std::string& responseData, std::vector& modData, const PKStruct& pk, std::vector& err); static bool ParseSignature(const std::vector& minisigs, SigStruct& sig, const PKStruct& pk, std::string& err); // Url parts Index: source/ps/ModIo.cpp =================================================================== --- source/ps/ModIo.cpp +++ source/ps/ModIo.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2019 Wildfire Games. +/* Copyright (C) 2020 Wildfire Games. * * Permission is hereby granted, free of charge, to any person obtaining * a copy of this software and associated documentation files (the @@ -446,6 +446,7 @@ // Perform parsing and/or checks std::string error; + std::vector errors; switch (m_DownloadProgressData.status) { case DownloadProgressStatus::GAMEID: @@ -459,14 +460,13 @@ m_DownloadProgressData.status = DownloadProgressStatus::READY; break; case DownloadProgressStatus::LISTING: - if (!ParseMods(scriptInterface, error)) + if (!ParseMods(scriptInterface, errors)) { m_ModData.clear(); // Failed during parsing, make sure we don't provide partial data m_DownloadProgressData.status = DownloadProgressStatus::FAILED_LISTING; - m_DownloadProgressData.error = error; + m_DownloadProgressData.error = errors[0]; break; } - m_DownloadProgressData.status = DownloadProgressStatus::LISTED; break; case DownloadProgressStatus::DOWNLOADING: @@ -505,7 +505,7 @@ return true; } -bool ModIo::ParseMods(const ScriptInterface& scriptInterface, std::string& err) +bool ModIo::ParseMods(const ScriptInterface& scriptInterface, std::vector& err) { bool ret = ParseModsResponse(scriptInterface, m_ResponseData, m_ModData, m_pk, err); m_ResponseData.clear(); @@ -530,7 +530,6 @@ return false; } } - ENSURE(m_CallbackData); // MD5 (because upstream provides it) @@ -573,6 +572,8 @@ } #define FAIL(...) STMT(err = fmt::sprintf(__VA_ARGS__); CLEANUP(); return false;) +#define FAIL2(...) STMT(err.push_back(fmt::sprintf(__VA_ARGS__)); CLEANUP(); return false;) +#define WARN(...) STMT(err.push_back(fmt::sprintf(__VA_ARGS__));); /** * Parses the current content of m_ResponseData to extract m_GameId. @@ -653,7 +654,7 @@ * Only the listed properties are of interest to consumers, and we flatten * the modfile structure as that simplifies handling and there are no conflicts. */ -bool ModIo::ParseModsResponse(const ScriptInterface& scriptInterface, const std::string& responseData, std::vector& modData, const PKStruct& pk, std::string& err) +bool ModIo::ParseModsResponse(const ScriptInterface& scriptInterface, const std::string& responseData, std::vector& modData, const PKStruct& pk, std::vector& err) { // Make sure we don't end up passing partial results back #define CLEANUP() modData.clear(); @@ -664,98 +665,115 @@ JS::RootedValue modResponse(cx); if (!scriptInterface.ParseJSON(responseData, &modResponse)) - FAIL("Failed to parse response as JSON."); + FAIL2("Failed to parse response as JSON."); if (!modResponse.isObject()) - FAIL("response not an object."); + FAIL2("response not an object."); JS::RootedObject modResponseObj(cx, modResponse.toObjectOrNull()); JS::RootedValue dataVal(cx); if (!JS_GetProperty(cx, modResponseObj, "data", &dataVal)) - FAIL("data property not in response."); + FAIL2("data property not in response."); // [modobj1, modobj2, ... ] if (!dataVal.isObject()) - FAIL("data property not an object."); + FAIL2("data property not an object."); - JS::RootedObject data(cx, dataVal.toObjectOrNull()); + JS::RootedObject rData(cx, dataVal.toObjectOrNull()); u32 length; bool isArray; - if (!JS_IsArrayObject(cx, data, &isArray) || !isArray || !JS_GetArrayLength(cx, data, &length) || !length) - FAIL("data property not an array with at least one element."); + if (!JS_IsArrayObject(cx, rData, &isArray) || !isArray || !JS_GetArrayLength(cx, rData, &length) || !length) + FAIL2("data property not an array with at least one element."); modData.clear(); modData.reserve(length); +#define WARN_AND_CONTINUE(...) \ + {\ + data.properties.emplace("invalid", "true");\ + data.properties.emplace("error", __VA_ARGS__);\ + STMT(err.push_back(fmt::sprintf(__VA_ARGS__)););\ + continue;\ + } + for (u32 i = 0; i < length; ++i) { - JS::RootedValue el(cx); - if (!JS_GetElement(cx, data, i, &el) || !el.isObject()) - FAIL("Failed to get array element object."); - modData.emplace_back(); + ModIoModData& data = modData.back(); + JS::RootedValue el(cx); + if (!JS_GetElement(cx, rData, i, &el) || !el.isObject()) + WARN_AND_CONTINUE("Failed to get array element object."); -#define COPY_STRINGS(prefix, obj, ...) \ + bool ok = true; + std::string copyStringError; +#define COPY_STRINGS_ELSE_CONTINUE(prefix, obj, ...) \ for (const std::string& prop : { __VA_ARGS__ }) \ { \ std::string val; \ - if (!ScriptInterface::FromJSProperty(cx, obj, prop.c_str(), val)) \ - FAIL("Failed to get %s from %s.", prop, #obj);\ - modData.back().properties.emplace(prefix+prop, val); \ - } + if (!ScriptInterface::FromJSProperty(cx, obj, prop.c_str(), val, true)) \ + { \ + ok = false; \ + copyStringError = "Failed to get " + prop + " from " + #obj + "."; \ + break; \ + }\ + data.properties.emplace(prefix+prop, val); \ + } \ + if (!ok) \ + WARN_AND_CONTINUE(copyStringError); // TODO: Currently the homepage_url field does not contain a non-null value for any entry. - COPY_STRINGS("", el, "name", "name_id", "summary"); + COPY_STRINGS_ELSE_CONTINUE("", el, "name", "name_id", "summary"); // Now copy over the modfile part, but without the pointless substructure JS::RootedObject elObj(cx, el.toObjectOrNull()); JS::RootedValue modFile(cx); if (!JS_GetProperty(cx, elObj, "modfile", &modFile)) - FAIL("Failed to get modfile data."); + WARN_AND_CONTINUE("Failed to get modfile data."); if (!modFile.isObject()) - FAIL("modfile not an object."); + WARN_AND_CONTINUE("modfile not an object."); - COPY_STRINGS("", modFile, "version", "filesize"); + COPY_STRINGS_ELSE_CONTINUE("", modFile, "version", "filesize"); JS::RootedObject modFileObj(cx, modFile.toObjectOrNull()); JS::RootedValue filehash(cx); if (!JS_GetProperty(cx, modFileObj, "filehash", &filehash)) - FAIL("Failed to get filehash data."); + WARN_AND_CONTINUE("Failed to get filehash data."); - COPY_STRINGS("filehash_", filehash, "md5"); + COPY_STRINGS_ELSE_CONTINUE("filehash_", filehash, "md5"); JS::RootedValue download(cx); if (!JS_GetProperty(cx, modFileObj, "download", &download)) - FAIL("Failed to get download data."); + WARN_AND_CONTINUE("Failed to get download data."); - COPY_STRINGS("", download, "binary_url"); + COPY_STRINGS_ELSE_CONTINUE("", download, "binary_url"); // Parse metadata_blob (sig+deps) std::string metadata_blob; if (!ScriptInterface::FromJSProperty(cx, modFile, "metadata_blob", metadata_blob)) - FAIL("Failed to get metadata_blob from modFile."); + WARN_AND_CONTINUE("Failed to get metadata_blob from modFile."); JS::RootedValue metadata(cx); if (!scriptInterface.ParseJSON(metadata_blob, &metadata)) - FAIL("Failed to parse metadata_blob as JSON."); + WARN_AND_CONTINUE("Failed to parse metadata_blob as JSON."); if (!metadata.isObject()) - FAIL("metadata_blob not decoded as an object."); + WARN_AND_CONTINUE("metadata_blob not decoded as an object."); - if (!ScriptInterface::FromJSProperty(cx, metadata, "dependencies", modData.back().dependencies)) - FAIL("Failed to get dependencies from metadata_blob."); + if (!ScriptInterface::FromJSProperty(cx, metadata, "dependencies", data.dependencies)) + WARN_AND_CONTINUE("Failed to get dependencies from metadata_blob."); std::vector minisigs; if (!ScriptInterface::FromJSProperty(cx, metadata, "minisigs", minisigs)) - FAIL("Failed to get minisigs from metadata_blob."); + WARN_AND_CONTINUE("Failed to get minisigs from metadata_blob."); - // Remove this entry if we did not find a valid matching signature. + // Check we did find a valid matching signature. std::string signatureParsingErr; - if (!ParseSignature(minisigs, modData.back().sig, pk, signatureParsingErr)) - modData.pop_back(); + if (!ParseSignature(minisigs, data.sig, pk, signatureParsingErr)) + WARN_AND_CONTINUE(signatureParsingErr); -#undef COPY_STRINGS +#undef COPY_STRINGS_ELSE_CONTINUE +#undef WARN_AND_CONTINUE } return true; Index: source/ps/tests/test_ModIo.h =================================================================== --- source/ps/tests/test_ModIo.h +++ source/ps/tests/test_ModIo.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2018 Wildfire Games. +/* Copyright (C) 2020 Wildfire Games. * * Permission is hereby granted, free of charge, to any person obtaining * a copy of this software and associated documentation files (the @@ -104,9 +104,9 @@ { \ TestLogger logger; \ std::vector mods; \ - std::string err; \ + std::vector err; \ TS_ASSERT(!ModIo::ParseModsResponse(script, input, mods, pk, err)); \ - TS_ASSERT_STR_EQUALS(err, expected_error); \ + TS_ASSERT_STR_EQUALS(err[0], expected_error); \ TS_ASSERT_EQUALS(mods.size(), 0); \ } @@ -118,6 +118,19 @@ TS_ASSERT_PARSE("{\"data\": null}", "data property not an object."); TS_ASSERT_PARSE("{\"data\": {}}", "data property not an array with at least one element."); TS_ASSERT_PARSE("{\"data\": []}", "data property not an array with at least one element."); + +#undef TS_ASSERT_PARSE + +#define TS_ASSERT_PARSE(input, expected_error) \ + { \ + TestLogger logger; \ + std::vector mods; \ + std::vector err; \ + TS_ASSERT(ModIo::ParseModsResponse(script, input, mods, pk, err)); \ + TS_ASSERT_STR_EQUALS(err[0], expected_error); \ + TS_ASSERT_EQUALS(mods.size(), 0); \ + } + TS_ASSERT_PARSE("{\"data\": [null]}", "Failed to get array element object."); TS_ASSERT_PARSE("{\"data\": [false]}", "Failed to get array element object."); TS_ASSERT_PARSE("{\"data\": [true]}", "Failed to get array element object."); @@ -164,7 +177,7 @@ { TestLogger logger; std::vector mods; - std::string err; + std::vector err; TS_ASSERT(ModIo::ParseModsResponse(script, "{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":\"abc\"}, \"download\":{\"binary_url\":\"\"},\"metadata_blob\":\"{\\\"dependencies\\\":[],\\\"minisigs\\\":[]}\"}}]}", mods, pk, err)); TS_ASSERT(err.empty()); TS_ASSERT_EQUALS(mods.size(), 0); @@ -174,7 +187,7 @@ { TestLogger logger; std::vector mods; - std::string err; + std::vector err; TS_ASSERT(ModIo::ParseModsResponse(script, "{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":\"abc\"}, \"download\":{\"binary_url\":\"\"},\"metadata_blob\":\"{\\\"dependencies\\\":[],\\\"minisigs\\\":[\\\"untrusted comment: signature from minisign secret key\\\\nRUTA6VIoth2Q1HUg5bwwbCUZPcqbQ/reLXqxiaWARH5PNcwxX5vBv/mLPLgdxGsIrOyK90763+rCVTmjeYx5BDz8C0CIbGZTNQs=\\\\ntrusted comment: timestamp:1517285433\\\\tfile:tm.zip\\\\nTHwNMhK4Ogj6XA4305p1K9/ouP/DrxPcDFrPaiu+Ke6/WGlHIzBZHvmHWUedvsK6dzL31Gk8YNzscKWnZqWNCw==\\\"]}\"}}]}", mods, pk, err)); TS_ASSERT(err.empty()); TS_ASSERT_EQUALS(mods.size(), 1); Index: source/scriptinterface/ScriptConversions.h =================================================================== --- source/scriptinterface/ScriptConversions.h +++ source/scriptinterface/ScriptConversions.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2019 Wildfire Games. +/* Copyright (C) 2020 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -87,7 +87,7 @@ return FromJSVal_vector(cx, v, out); \ } -template bool ScriptInterface::FromJSProperty(JSContext* cx, const JS::HandleValue val, const char* name, T& ret) +template bool ScriptInterface::FromJSProperty(JSContext* cx, const JS::HandleValue val, const char* name, T& ret, bool strict) { if (!val.isObject()) return false; @@ -103,6 +103,9 @@ if (!JS_GetProperty(cx, obj, name, &value)) return false; + if (strict && value.isNull()) + return false; + return FromJSVal(cx, value, ret); } Index: source/scriptinterface/ScriptInterface.h =================================================================== --- source/scriptinterface/ScriptInterface.h +++ source/scriptinterface/ScriptInterface.h @@ -310,7 +310,7 @@ /** * Convert a named property of an object to a C++ type. */ - template static bool FromJSProperty(JSContext* cx, const JS::HandleValue val, const char* name, T& ret); + template static bool FromJSProperty(JSContext* cx, const JS::HandleValue val, const char* name, T& ret, bool strict = false); /** * MathRandom (this function) calls the random number generator assigned to this ScriptInterface instance and