From 21296b0cf1026a8dc1694a11277c392c63e67745 Mon Sep 17 00:00:00 2001 From: Leland Lucius Date: Sun, 31 Jan 2021 04:24:35 -0600 Subject: [PATCH] New wrapper for wxFileConfig to fix several bugs Bug 2135 - Audacity crashes if launched with a locked pluginregistry.cfg file Bug 2651 - If pluginregistry.cfg gets locked while Audacity is active the Plug-in Manager is blocked - with no help Bug 2650 - Virgin launch fails with no pluginregistry.cfg file created Bug 1264 - Writing to locked audacity.cfg not reported to user Bug 2649 - Launching Audacity with a locked audacity.cfg file gives not one but three error messages - and no help Bug 2652 - User is not warned if they launch Audacity with a locked pluginsettings.cfg file --- src/AudacityApp.cpp | 19 +-- src/CMakeLists.txt | 2 + src/PluginManager.cpp | 21 +-- src/PluginManager.h | 9 +- src/Prefs.cpp | 24 +--- src/Prefs.h | 6 +- src/widgets/FileConfig.cpp | 274 ++++++++++++++++++++++++++++++++++++ src/widgets/FileConfig.h | 47 +++++++ src/widgets/FileHistory.cpp | 1 - 9 files changed, 349 insertions(+), 54 deletions(-) create mode 100644 src/widgets/FileConfig.cpp create mode 100644 src/widgets/FileConfig.h diff --git a/src/AudacityApp.cpp b/src/AudacityApp.cpp index cf69d9b71..86e456126 100644 --- a/src/AudacityApp.cpp +++ b/src/AudacityApp.cpp @@ -106,6 +106,7 @@ It handles initialization and termination by subclassing wxApp. #include "prefs/DirectoriesPrefs.h" #include "prefs/GUIPrefs.h" #include "tracks/ui/Scrubbing.h" +#include "widgets/FileConfig.h" #include "widgets/FileHistory.h" #ifdef EXPERIMENTAL_EASY_CHANGE_KEY_BINDINGS @@ -187,11 +188,11 @@ void PopulatePreferences() { const wxString fullPath{fn.GetFullPath()}; - wxFileConfig ini(wxEmptyString, - wxEmptyString, - fullPath, - wxEmptyString, - wxCONFIG_USE_LOCAL_FILE); + FileConfig ini(wxEmptyString, + wxEmptyString, + fullPath, + wxEmptyString, + wxCONFIG_USE_LOCAL_FILE); wxString lang; if (ini.Read(wxT("/FromInno/Language"), &lang)) @@ -1247,14 +1248,6 @@ bool AudacityApp::OnInit() wxFileName configFileName(FileNames::DataDir(), wxT("audacity.cfg")); InitPreferences( configFileName ); PopulatePreferences(); - // This test must follow PopulatePreferences, because if an error message - // must be shown, we need internationalization to have been initialized - // first, which was done in PopulatePreferences - if ( !CheckWritablePreferences() ) { - ::AudacityMessageBox( - UnwritablePreferencesErrorMessage( configFileName ) ); - return false; - } #if defined(__WXMSW__) && !defined(__WXUNIVERSAL__) && !defined(__CYGWIN__) this->AssociateFileTypes(); diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a5011abda..7c8e1c84f 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -892,6 +892,8 @@ list( APPEND SOURCES widgets/ErrorDialog.h widgets/ExpandingToolBar.cpp widgets/ExpandingToolBar.h + widgets/FileConfig.cpp + widgets/FileConfig.h widgets/FileDialog/FileDialog.cpp widgets/FileDialog/FileDialog.h $<$: diff --git a/src/PluginManager.cpp b/src/PluginManager.cpp index a46031571..3000df355 100644 --- a/src/PluginManager.cpp +++ b/src/PluginManager.cpp @@ -1929,7 +1929,7 @@ bool PluginManager::DropFile(const wxString &fileName) void PluginManager::Load() { // Create/Open the registry - wxFileConfig registry(wxEmptyString, wxEmptyString, FileNames::PluginRegistry()); + FileConfig registry(wxEmptyString, wxEmptyString, FileNames::PluginRegistry()); // If this group doesn't exist then we have something that's not a registry. // We should probably warn the user, but it's pretty unlikely that this will happen. @@ -2008,7 +2008,7 @@ void PluginManager::Load() return; } -void PluginManager::LoadGroup(wxFileConfig *pRegistry, PluginType type) +void PluginManager::LoadGroup(FileConfig *pRegistry, PluginType type) { #ifdef __WXMAC__ // Bug 1590: On Mac, we should purge the registry of Nyquist plug-ins @@ -2269,15 +2269,8 @@ void PluginManager::LoadGroup(wxFileConfig *pRegistry, PluginType type) void PluginManager::Save() { - if (!wxFileName(FileNames::PluginRegistry()).IsFileWritable()) { - AudacityMessageBox( - XO("The plugin registry file is not writable:\n\n%s\n\nSettings were not saved.") - .Format(FileNames::PluginRegistry())); - return; - } - // Create/Open the registry - wxFileConfig registry(wxEmptyString, wxEmptyString, FileNames::PluginRegistry()); + FileConfig registry(wxEmptyString, wxEmptyString, FileNames::PluginRegistry()); // Clear it out registry.DeleteAll(); @@ -2303,7 +2296,7 @@ void PluginManager::Save() registry.Flush(); } -void PluginManager::SaveGroup(wxFileConfig *pRegistry, PluginType type) +void PluginManager::SaveGroup(FileConfig *pRegistry, PluginType type) { wxString group = GetPluginTypeString(type); for (PluginMap::iterator iter = mPlugins.begin(); iter != mPlugins.end(); ++iter) @@ -2780,11 +2773,11 @@ PluginDescriptor & PluginManager::CreatePlugin(const PluginID & id, return plug; } -wxFileConfig *PluginManager::GetSettings() +FileConfig *PluginManager::GetSettings() { if (!mSettings) { - mSettings = std::make_unique(wxEmptyString, wxEmptyString, FileNames::PluginSettings()); + mSettings = std::make_unique(wxEmptyString, wxEmptyString, FileNames::PluginSettings()); // Check for a settings version that we can understand if (mSettings->HasEntry(SETVERKEY)) @@ -2812,7 +2805,7 @@ wxFileConfig *PluginManager::GetSettings() bool PluginManager::HasGroup(const RegistryPath & group) { - wxFileConfig *settings = GetSettings(); + FileConfig *settings = GetSettings(); bool res = settings->HasGroup(group); if (res) diff --git a/src/PluginManager.h b/src/PluginManager.h index b0b746bd9..ab041b672 100644 --- a/src/PluginManager.h +++ b/src/PluginManager.h @@ -21,6 +21,7 @@ #include "audacity/PluginInterface.h" class wxArrayString; +class FileConfig; /////////////////////////////////////////////////////////////////////////////// // @@ -270,13 +271,13 @@ private: ~PluginManager(); void Load(); - void LoadGroup(wxFileConfig *pRegistry, PluginType type); + void LoadGroup(FileConfig *pRegistry, PluginType type); void Save(); - void SaveGroup(wxFileConfig *pRegistry, PluginType type); + void SaveGroup(FileConfig *pRegistry, PluginType type); PluginDescriptor & CreatePlugin(const PluginID & id, ComponentInterface *ident, PluginType type); - wxFileConfig *GetSettings(); + FileConfig *GetSettings(); bool HasGroup(const RegistryPath & group); bool GetSubgroups(const RegistryPath & group, RegistryPaths & subgroups); @@ -312,7 +313,7 @@ private: bool IsDirty(); void SetDirty(bool dirty = true); - std::unique_ptr mSettings; + std::unique_ptr mSettings; bool mDirty; int mCurrentIndex; diff --git a/src/Prefs.cpp b/src/Prefs.cpp index 1a4b869a7..e0da7810e 100755 --- a/src/Prefs.cpp +++ b/src/Prefs.cpp @@ -178,12 +178,12 @@ AudacityPrefs::AudacityPrefs(const wxString& appName, const wxString& globalFilename, long style, const wxMBConv& conv) : - wxFileConfig(appName, - vendorName, - localFilename, - globalFilename, - style, - conv) + FileConfig(appName, + vendorName, + localFilename, + globalFilename, + style, + conv) { } @@ -202,18 +202,6 @@ void InitPreferences( const wxFileName &configFileName ) wxConfigBase::Set(gPrefs); } -bool CheckWritablePreferences() -{ - return gPrefs->Write("/TEST", true) && gPrefs->Flush(); -} - -TranslatableString UnwritablePreferencesErrorMessage( const wxFileName &configFileName ) -{ - return - XO("Audacity cannot start because the settings file at %s is not writable.") - .Format(configFileName.GetFullPath()); -} - void FinishPreferences() { if (gPrefs) { diff --git a/src/Prefs.h b/src/Prefs.h index 5c7391e7c..d16a65fb8 100644 --- a/src/Prefs.h +++ b/src/Prefs.h @@ -33,16 +33,14 @@ #include "../include/audacity/ComponentInterface.h" #include "MemoryX.h" // for wxArrayStringEx +#include "widgets/FileConfig.h" #include -#include // to inherit wxFileConfig #include // to declare custom event types class wxFileName; void InitPreferences( const wxFileName &configFileName ); -bool CheckWritablePreferences(); -TranslatableString UnwritablePreferencesErrorMessage( const wxFileName &configFileName ); void FinishPreferences(); class AudacityPrefs; @@ -55,7 +53,7 @@ extern int gMenusDirty; /// \brief Our own specialisation of wxFileConfig. It is essentially a renaming, /// though it does provide one new access function. Most of the prefs work /// is actually done by the InitPreferences() function. -class AUDACITY_DLL_API AudacityPrefs : public wxFileConfig +class AUDACITY_DLL_API AudacityPrefs : public FileConfig { public: AudacityPrefs(const wxString& appName = {}, diff --git a/src/widgets/FileConfig.cpp b/src/widgets/FileConfig.cpp new file mode 100644 index 000000000..486bae27f --- /dev/null +++ b/src/widgets/FileConfig.cpp @@ -0,0 +1,274 @@ +/********************************************************************** + + Audacity: A Digital Audio Editor + + FileConfig.cpp + + Leland Lucius + +**********************************************************************/ + +#include "../Audacity.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "FileConfig.h" +#include "HelpSystem.h" +#include "wxPanelWrapper.h" +#include "../ShuttleGui.h" + +#include "../../images/Help.xpm" + +#if !defined(F_OK) +#define F_OK 0x00 +#endif +#if !defined(W_OK) +#define W_OK 0x02 +#endif +#if !defined(R_OK) +#define R_OK 0x04 +#endif + +FileConfig::FileConfig(const wxString& appName, + const wxString& vendorName, + const wxString& localFilename, + const wxString& globalFilename, + long style, + const wxMBConv& conv) +: wxFileConfig(appName, vendorName, localFilename, globalFilename, style, conv), + mConfigPath(localFilename), + mDirty(false) +{ + // Prevent wxFileConfig from attempting a Flush() during object deletion. This happens + // because we don't use the wxFileConfig::Flush() method and so the wxFileConfig dirty + // flag never gets reset. During deletiong it is checked a a Flush() performed. This + // can (and probably will) create bogus temporary files. + DisableAutoSave(); + + while (true) + { + bool canRead = false; + bool canWrite = false; + int fd; + + fd = wxOpen(mConfigPath, O_RDONLY, S_IREAD); + if (fd != -1 || errno == ENOENT) + { + canRead = true; + if (fd != -1) + { + wxClose(fd); + } + } + + fd = wxOpen(mConfigPath, O_WRONLY | O_CREAT, S_IWRITE); + if (fd != -1) + { + canWrite = true; + wxClose(fd); + } + + if (canRead && canWrite) + { + break; + } + + // If we can't read an existing config file, we must not allow the user to retry + // since the wxFileConfig initialization will not have read it and the caller + // will assume that the file was didn't exist and possibly initialize it. This + // could lead to wiping out the original contents. + // + // If the wxFileConfig class allowed us to call wxFileConfig::Init(), we wouldn't + // have to do all this mess. + Warn(canRead == true); + } +} + +FileConfig::~FileConfig() +{ + wxASSERT(mDirty == false); +} + +bool FileConfig::Flush(bool WXUNUSED(bCurrentOnly)) +{ + if (!mDirty) + { + return true; + } + + while (true) + { + FilePath backup = mConfigPath + ".bkp"; + + if (!wxFileExists(backup) || (wxRemove(backup) == 0)) + { + if (!wxFileExists(mConfigPath) || (wxRename(mConfigPath, backup) == 0)) + { + wxFileOutputStream stream(mConfigPath); + if (stream.IsOk()) + { + if (Save(stream)) + { + stream.Sync(); + if (stream.IsOk() && stream.Close()) + { + if (!wxFileExists(backup) || (wxRemove(backup) == 0)) + { + mDirty = false; + return true; + } + } + } + } + + if (wxFileExists(backup)) + { + wxRemove(mConfigPath); + wxRename(backup, mConfigPath); + } + } + } + + Warn(); + } + + return false; +} + +bool FileConfig::DoWriteString(const wxString& key, const wxString& szValue) +{ + bool res = wxFileConfig::DoWriteString(key, szValue); + if (res) + { + mDirty = true; + } + return res; +} + +bool FileConfig::DoWriteLong(const wxString& key, long lValue) +{ + bool res = wxFileConfig::DoWriteLong(key, lValue); + if (res) + { + mDirty = true; + } + return res; +} + +#if wxUSE_BASE64 +bool FileConfig::DoWriteBinary(const wxString& key, const wxMemoryBuffer& buf) +{ + bool res = wxFileConfig::DoWriteBinary(key, buf); + if (res) + { + mDirty = true; + } + return res; +} +#endif // wxUSE_BASE64 + +void FileConfig::Warn(bool canRetry) +{ + wxDialogWrapper dlg(nullptr, wxID_ANY, XO("Audacity Configuration Error")); + + ShuttleGui S(&dlg, eIsCreating); + + wxButton *retryButton; + wxButton *quitButton; + + S.SetBorder(5); + S.StartVerticalLay(wxEXPAND, 1); + { + S.SetBorder(15); + S.StartHorizontalLay(wxALIGN_RIGHT, 0); + { + auto cause = canRetry + ? XO("The following configuration file could not be written:") + : XO("The following configuration file could not be read:"); + + auto retryMsg = canRetry + ? XO("You can attempt to correct the issue and then click \"Retry\" to coninue.\n\n") + : XO(""); + + S.AddFixedText( + XO("%s:\n\n" + "\t%s\n\n" + "This could be caused by many reasons, but the most likely are that " + "the disk is full or you do not have write permissions to the file. " + "More information can be obtained by clicking the help button below.\n\n" + "%s" + "If you choose to \"Quit Audacity\", your project may be left in an unsaved " + "state which will be recovered the next time you open it.") + .Format(cause, mConfigPath, retryMsg), + false, + 500); + } + S.EndHorizontalLay(); + + S.SetBorder(5); + S.StartHorizontalLay(wxALIGN_RIGHT, 0); + { + // Can't use themed bitmap since the theme manager might not be + // initialized yet and it requires a configuration file. + wxButton *b = S.Id(wxID_HELP).AddBitmapButton(wxBitmap(Help_xpm)); + b->SetToolTip( XO("Help").Translation() ); + b->SetLabel(XO("Help").Translation()); // for screen readers + + b = S.Id(wxID_CANCEL).AddButton(XXO("&Quit Audacity")); + + if (canRetry) + { + b = S.Id(wxID_OK).AddButton(XXO("&Retry")); + dlg.SetAffirmativeId(wxID_OK); + } + + b->SetDefault(); + b->SetFocus(); + } + S.EndHorizontalLay(); + } + S.EndVerticalLay(); + + dlg.Layout(); + dlg.GetSizer()->Fit(&dlg); + dlg.SetMinSize(dlg.GetSize()); + dlg.Center(); + + auto onButton = [&](wxCommandEvent &e) + { + dlg.EndModal(e.GetId()); + }; + + dlg.Bind(wxEVT_BUTTON, onButton); + + switch (dlg.ShowModal()) + { + case wxID_HELP: + // Can't use the HelpSystem since the theme manager may not + // yet be initialized and it requires a configuration file. + OpenInDefaultBrowser("https://" + + HelpSystem::HelpHostname + + HelpSystem::HelpServerHomeDir + + "Error:_Audacity_settings_file_unwritable"); + break; + + case wxID_CANCEL: + // This REALLY needs to use an exception with decent cleanup and program exit + wxExit(); +#if defined(__WXGTK__) + wxAbort(); +#else + exit(-1); +#endif + break; + } + + dlg.Unbind(wxEVT_BUTTON, onButton); +} diff --git a/src/widgets/FileConfig.h b/src/widgets/FileConfig.h new file mode 100644 index 000000000..7d830fbc4 --- /dev/null +++ b/src/widgets/FileConfig.h @@ -0,0 +1,47 @@ +/********************************************************************** + + Audacity: A Digital Audio Editor + + FileConfig.h + + Leland Lucius + +**********************************************************************/ + +#ifndef __AUDACITY_WIDGETS_FILECONFIG__ +#define __AUDACITY_WIDGETS_FILECONFIG__ + +#include +#include + +#include "audacity/Types.h" + +class FileConfig : public wxFileConfig +{ +public: + FileConfig(const wxString& appName = wxEmptyString, + const wxString& vendorName = wxEmptyString, + const wxString& localFilename = wxEmptyString, + const wxString& globalFilename = wxEmptyString, + long style = wxCONFIG_USE_LOCAL_FILE | wxCONFIG_USE_GLOBAL_FILE, + const wxMBConv& conv = wxConvAuto()); + virtual ~FileConfig(); + + virtual bool Flush(bool bCurrentOnly = false) wxOVERRIDE; + +protected: + virtual bool DoWriteString(const wxString& key, const wxString& szValue) wxOVERRIDE; + virtual bool DoWriteLong(const wxString& key, long lValue) wxOVERRIDE; +#if wxUSE_BASE64 + virtual bool DoWriteBinary(const wxString& key, const wxMemoryBuffer& buf) wxOVERRIDE; +#endif // wxUSE_BASE64 + +private: + void Warn(bool canRetry = true); + + FilePath mConfigPath; + bool mDirty; +}; + +#endif + diff --git a/src/widgets/FileHistory.cpp b/src/widgets/FileHistory.cpp index a59a730b6..98c62b0c6 100644 --- a/src/widgets/FileHistory.cpp +++ b/src/widgets/FileHistory.cpp @@ -17,7 +17,6 @@ #include "FileHistory.h" #include -#include #include #include "../Internat.h"