NorthstarLauncher icon indicating copy to clipboard operation
NorthstarLauncher copied to clipboard

Unchecked method call `GetString()` in `mod.json` reading

Open emma-miler opened this issue 2 years ago • 0 comments

Issue

This issue is a continuation of https://github.com/R2Northstar/NorthstarMods/issues/409 https://github.com/R2Northstar/NorthstarLauncher/blob/85635ad99dbdfa909341e4138f6fdb0c9bf73bf1/NorthstarDedicatedTest/modmanager.cpp#L102 On this line an unchecked call to GetString() is made which crashes if the value is not a string For the likely implementation of this method, see here

Solution

We should check that this value is a string

if (convarObj["DefaultValue"].IsString()) {convar->DefaultValue = convarObj["DefaultValue"].GetString(); }
else {spdlog::warn("Non-String value of {} in mod.json", convar->Name);}

Since the type of convar->DefaultValue is an std::string it is already default initialised, and this would be sufficient.

Verification

I currently have no way of testing this implementation, so I would like to ask somebody else to test it in their environment From the original issue: Create a mod.json with one of the following variables This example should work fine in both cases

        {
            "Name": "fm_welcome_enabled",
            "DefaultValue": "1"
        },

This example crashes, but should not crash and possible give a warning.

        {
            "Name": "fm_welcome_enabled",
            "DefaultValue": 1
        },

emma-miler avatar Jul 11 '22 18:07 emma-miler