Index: ps/trunk/binaries/data/mods/mod/gui/modio/modio.js =================================================================== --- ps/trunk/binaries/data/mods/mod/gui/modio/modio.js +++ ps/trunk/binaries/data/mods/mod/gui/modio/modio.js @@ -167,6 +167,8 @@ let filterColumns = ["name", "name_id", "summary"]; let filterText = Engine.GetGUIObjectByName("modFilter").caption.toLowerCase(); + if (Engine.GetGUIObjectByName("compatibilityFilter").checked) + displayedMods = displayedMods.filter(mod => !mod.invalid); displayedMods = displayedMods.filter(mod => filterColumns.some(column => mod[column].toLowerCase().indexOf(filterText) != -1)); displayedMods.sort((mod1, mod2) => @@ -175,11 +177,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) : 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 +204,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 ? sprintf(translate("Invalid mod: %(error)s"), {"error": g_ModsAvailableOnline[selected].error }) : ""; } function cancelModListUpdate() @@ -244,6 +254,9 @@ { let selected = selectedModIndex(); + if (isSelectedModInvalid(selected)) + return; + progressDialog( sprintf(translate("Downloading “%(modname)s”"), { "modname": g_ModsAvailableOnline[selected].name Index: ps/trunk/binaries/data/mods/mod/gui/modio/modio.xml =================================================================== --- ps/trunk/binaries/data/mods/mod/gui/modio/modio.xml +++ ps/trunk/binaries/data/mods/mod/gui/modio/modio.xml @@ -65,7 +65,21 @@ Dependencies + + + + + + + + + displayMods(); + + + + Filter valid mods + Index: ps/trunk/source/ps/ModIo.cpp =================================================================== --- ps/trunk/source/ps/ModIo.cpp +++ ps/trunk/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 @@ -678,84 +678,100 @@ if (!dataVal.isObject()) FAIL("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) + if (!JS_IsArrayObject(cx, rData, &isArray) || !isArray || !JS_GetArrayLength(cx, rData, &length) || !length) FAIL("data property not an array with at least one element."); modData.clear(); modData.reserve(length); +#define INVALIDATE_DATA_AND_CONTINUE(...) \ + {\ + data.properties.emplace("invalid", "true");\ + data.properties.emplace("error", __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()) + INVALIDATE_DATA_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) \ + INVALIDATE_DATA_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."); + INVALIDATE_DATA_AND_CONTINUE("Failed to get modfile data."); if (!modFile.isObject()) - FAIL("modfile not an object."); + INVALIDATE_DATA_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."); + INVALIDATE_DATA_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."); + INVALIDATE_DATA_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."); + if (!ScriptInterface::FromJSProperty(cx, modFile, "metadata_blob", metadata_blob, true)) + INVALIDATE_DATA_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."); + INVALIDATE_DATA_AND_CONTINUE("Failed to parse metadata_blob as JSON."); if (!metadata.isObject()) - FAIL("metadata_blob not decoded as an object."); + INVALIDATE_DATA_AND_CONTINUE("metadata_blob is 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, true)) + INVALIDATE_DATA_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."); + if (!ScriptInterface::FromJSProperty(cx, metadata, "minisigs", minisigs, true)) + INVALIDATE_DATA_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)) + INVALIDATE_DATA_AND_CONTINUE(signatureParsingErr); -#undef COPY_STRINGS +#undef COPY_STRINGS_ELSE_CONTINUE +#undef INVALIDATE_DATA_AND_CONTINUE } return true; Index: ps/trunk/source/ps/tests/test_ModIo.h =================================================================== --- ps/trunk/source/ps/tests/test_ModIo.h +++ ps/trunk/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 @@ -118,14 +118,28 @@ 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::string err; \ + TS_ASSERT(ModIo::ParseModsResponse(script, input, mods, pk, err)); \ + TS_ASSERT_EQUALS(mods.size(), 1); \ + TS_ASSERT_STR_EQUALS(mods.at(0).properties.at("error"), expected_error); \ + TS_ASSERT_EQUALS(mods.at(0).properties.at("invalid"), "true"); \ + } + 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."); TS_ASSERT_PARSE("{\"data\": [{}]}", "Failed to get name from el."); TS_ASSERT_PARSE("{\"data\": [[]]}", "Failed to get name from el."); TS_ASSERT_PARSE("{\"data\": [{\"foo\":\"bar\"}]}", "Failed to get name from el."); + TS_ASSERT_PARSE("{\"data\": [{\"name\":null}]}", "Failed to get name from el."); - TS_ASSERT_PARSE("{\"data\": [{\"name\":null}]}", "Failed to get name_id from el."); // also some script value conversion check warning TS_ASSERT_PARSE("{\"data\": [{\"name\":42}]}", "Failed to get name_id from el."); // no conversion warning, but converting numbers to strings and vice-versa seems ok TS_ASSERT_PARSE("{\"data\": [{\"name\":false}]}", "Failed to get name_id from el."); // also some script value conversion check warning TS_ASSERT_PARSE("{\"data\": [{\"name\":{}}]}", "Failed to get name_id from el."); // also some script value conversion check warning @@ -136,20 +150,22 @@ TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":null}]}", "modfile not an object."); TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":[]}]}", "Failed to get version from modFile."); TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{}}]}", "Failed to get version from modFile."); + TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":null}}]}", "Failed to get version from modFile."); - TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":null}}]}", "Failed to get filesize from modFile."); // also some script value conversion check warning + TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":1}}]}", "Failed to get filesize from modFile."); TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234}}]}", "Failed to get md5 from filehash."); TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":null}}]}", "Failed to get md5 from filehash."); TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{}}}]}", "Failed to get md5 from filehash."); - TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":null}}}]}", "Failed to get binary_url from download."); // also some script value conversion check warning + TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":null}}}]}", "Failed to get md5 from filehash."); TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":\"abc\"}}}]}", "Failed to get binary_url from download."); TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":\"abc\"}, \"download\":null}}]}", "Failed to get binary_url from download."); // also some script value conversion check warning - TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":\"abc\"}, \"download\":{\"binary_url\":null}}}]}", "Failed to get metadata_blob from modFile."); // also some script value conversion check warning + TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":\"abc\"}, \"download\":{\"binary_url\":null}}}]}", "Failed to get binary_url from download."); TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":\"abc\"}, \"download\":{\"binary_url\":\"\"}}}]}", "Failed to get metadata_blob from modFile."); + TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":\"abc\"}, \"download\":{\"binary_url\":\"\"},\"metadata_blob\":null}}]}", "Failed to get metadata_blob from modFile."); - TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":\"abc\"}, \"download\":{\"binary_url\":\"\"},\"metadata_blob\":null}}]}", "metadata_blob not decoded as an object."); + TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":\"abc\"}, \"download\":{\"binary_url\":\"\"},\"metadata_blob\":1}}]}", "metadata_blob is not decoded as an object."); TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":\"abc\"}, \"download\":{\"binary_url\":\"\"},\"metadata_blob\":\"\"}}]}", "Failed to parse metadata_blob as JSON."); TS_ASSERT_PARSE("{\"data\": [{\"name\":\"\",\"name_id\":\"\",\"summary\":\"\",\"modfile\":{\"version\":\"\",\"filesize\":1234, \"filehash\":{\"md5\":\"abc\"}, \"download\":{\"binary_url\":\"\"},\"metadata_blob\":\"{}\"}}]}", "Failed to get dependencies from metadata_blob."); @@ -160,14 +176,15 @@ #undef TS_ASSERT_PARSE // Correctly formed input, but no signature matching the public key - // Thus all such mods/modfiles are not added, thus we get 0 parsed mods. + // Thus all such mods/modfiles are marked as invalid. { TestLogger logger; std::vector mods; std::string 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); + TS_ASSERT_EQUALS(mods.size(), 1); + TS_ASSERT_EQUALS(mods.at(0).properties.at("invalid"), "true"); } // Correctly formed input (with a signature matching the public key above, and a valid global signature) @@ -178,6 +195,7 @@ 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); + TS_ASSERT_EQUALS(mods.at(0).properties.find("invalid"), mods.at(0).properties.end()); } } Index: ps/trunk/source/scriptinterface/ScriptConversions.h =================================================================== --- ps/trunk/source/scriptinterface/ScriptConversions.h +++ ps/trunk/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: ps/trunk/source/scriptinterface/ScriptInterface.h =================================================================== --- ps/trunk/source/scriptinterface/ScriptInterface.h +++ ps/trunk/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