1
0
Fork 0

Add FGGlobals::autosaveFilePath(); improve code related to {load,save}UserSettings()

Apart from providing a public method giving a path to the autosave file,
the main idea of this commit is to reduce redundancy where
globals->get_fg_home() was so far used in every place where the autosave
file is needed or saved. Use an optional argument for
FGGlobals::loadUserSettings() and FGGlobals::saveUserSettings()[1],
since it should be exceptional to access an autosave file in another
location than $FG_HOME.

Also add comments explaining how to avoid security pitfalls with
saveUserSettings() (cf. discussion around
<https://sourceforge.net/p/flightgear/mailman/message/35461636/>).

[1] Argument *added* to this method, for consistency with
    FGGlobals::loadUserSettings().
This commit is contained in:
Florent Rougon 2016-11-01 14:04:36 +01:00
parent 1b0a76943f
commit b1b7f07edb
3 changed files with 53 additions and 19 deletions

View file

@ -681,17 +681,31 @@ static std::string autosaveName()
return os.str();
}
// Load user settings from autosave.xml
void
FGGlobals::loadUserSettings(const SGPath& dataPath)
SGPath FGGlobals::autosaveFilePath(SGPath userDataPath) const
{
// remember that we have (tried) to load any existing autsave.xml
if (userDataPath.isNull()) {
userDataPath = get_fg_home();
}
return simgear::Dir(userDataPath).file(autosaveName());
}
// Load user settings from the autosave file (normally in $FG_HOME)
void
FGGlobals::loadUserSettings(SGPath userDataPath)
{
if (userDataPath.isNull()) {
userDataPath = get_fg_home();
}
// Remember that we have (tried) to load any existing autosave file
haveUserSettings = true;
SGPath autosaveFile = simgear::Dir(dataPath).file(autosaveName());
SGPath autosaveFile = autosaveFilePath(userDataPath);
SGPropertyNode autosave;
if (autosaveFile.exists()) {
SG_LOG(SG_INPUT, SG_INFO, "Reading user settings from " << autosaveFile);
SG_LOG(SG_INPUT, SG_INFO,
"Reading user settings from " << autosaveFile);
try {
readProperties(autosaveFile, &autosave, SGPropertyNode::USERARCHIVE);
} catch (sg_exception& e) {
@ -702,10 +716,19 @@ FGGlobals::loadUserSettings(const SGPath& dataPath)
copyProperties(&autosave, globals->get_props());
}
// Save user settings in autosave.xml
// Save user settings to the autosave file.
//
// When calling this method, make sure the value of 'userDataPath' is
// trustworthy. In particular, make sure it can't be influenced by Nasal code,
// not even indirectly via a Nasal-writable place such as the property tree.
//
// Note: the default value, which causes the autosave file to be written to
// $FG_HOME, is safe---if not, it would be a bug.
void
FGGlobals::saveUserSettings()
FGGlobals::saveUserSettings(SGPath userDataPath)
{
if (userDataPath.isNull()) userDataPath = get_fg_home();
// only save settings when we have (tried) to load the previous
// settings (otherwise user data was lost)
if (!haveUserSettings)
@ -715,8 +738,7 @@ FGGlobals::saveUserSettings()
// don't save settings more than once on shutdown
haveUserSettings = false;
SGPath autosaveFile(globals->get_fg_home());
autosaveFile.append(autosaveName());
SGPath autosaveFile = autosaveFilePath(userDataPath);
autosaveFile.create_dir( 0700 );
SG_LOG(SG_IO, SG_INFO, "Saving user settings to " << autosaveFile);
try {

View file

@ -373,15 +373,29 @@ public:
inline void set_channellist( FGTACANList *c ) { channellist = c; }
/**
* Load user settings from autosave.xml
* Return an SGPath instance for the autosave file under 'userDataPath',
* which defaults to $FG_HOME.
*/
void loadUserSettings(const SGPath& datapath);
SGPath autosaveFilePath(SGPath userDataPath = SGPath()) const;
/**
* Load user settings from the autosave file under 'userDataPath',
* which defaults to $FG_HOME.
*/
void loadUserSettings(SGPath userDatapath = SGPath());
/**
* Save user settings in autosave.xml
* Save user settings to the autosave file under 'userDataPath', which
* defaults to $FG_HOME.
*
* When calling this method, make sure the value of 'userDataPath' is
* trustworthy. In particular, make sure it can't be influenced by Nasal
* code, not even indirectly via a Nasal-writable place such as the
* property tree.
*
* Note: the default value is safe---if not, it would be a bug.
*/
void saveUserSettings();
void saveUserSettings(SGPath userDatapath = SGPath());
void addListenerToCleanup(SGPropertyChangeListener* l);
simgear::pkg::Root* packageRoot();

View file

@ -191,10 +191,8 @@ static void fgSetVideoOptions()
}
flightgear::Options* options = flightgear::Options::sharedInstance();
if (!options->isOptionSet("ignore-autosave"))
{
SGPath dataPath = globals->get_fg_home();
globals->loadUserSettings(dataPath);
if (!options->isOptionSet("ignore-autosave")) {
globals->loadUserSettings();
}
}
}