livie
/
phoenix-firestorm
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

CHOP-951, IQA-1477: Validate args for numeric command-line switches. 

The logic in llcommandlineparser.cpp's setControlValueCB() callback function
for converting command-line switch argument values from string to the actual
type of the map-to settings variable had a couple special cases for boolean
and LLSD array. But for S32, U32 and F32, it simply used default LLSD
string-to-numeric conversion. LLSD's string-to-numeric conversion is a bit
lame: for non-numeric strings, it shrugs and returns 0.
Introduce onevalue() and badvalue() helper functions that, like certain errors
during command-line parsing, throw LLCLPError. Use them to streamline certain
redundancies in setControlValueCB(). Introduce convertTo<T>() helper function
that uses boost::lexical_cast() for slightly more stringent conversions. Add
cases for U32, S32 and F32 targets.
setControlValueCB() is actually called only by LLControlGroupCLP::notify(),
not during actual command-line parsing.
Make LLControlGroupCLP::notify() return bool. Make it catch LLCLPError, set
the error for getErrorMessage() and return false on that exception.
Package LLAppViewer::initConfiguration()'s response to initParseCommandLine()
returning false as a new handleCommandLineError() function; invoke it both
there and when LLControlGroupCLP::notify() returns false.
meow-7.2.2
Nat Goodspeed 2013-08-05 19:43:06 -04:00
parent acda43ed38
commit 7f6e7fc0cb
3 changed files with 153 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
indra/newview/llappviewer.cpp
View File

@ -2306,6 +2306,20 @@ void LLAppViewer::loadColorSettings()
LLUIColorTable::instance().loadFromSettings();
}
namespace
{
void handleCommandLineError(LLControlGroupCLP& clp)
{
llwarns << "Error parsing command line options. Command Line options ignored." << llendl;
llinfos << "Command line usage:\n" << clp << llendl;
OSMessageBox(STRINGIZE(LLTrans::getString("MBCmdLineError") << clp.getErrorMessage()),
LLStringUtil::null,
OSMB_OK);
}
} // anonymous namespace
bool LLAppViewer::initConfiguration()
{
//Load settings files list
@ -2404,13 +2418,7 @@ bool LLAppViewer::initConfiguration()
if(!initParseCommandLine(clp))
{
llwarns << "Error parsing command line options. Command Line options ignored." << llendl;
llinfos << "Command line usage:\n" << clp << llendl;
std::ostringstream msg;
msg << LLTrans::getString("MBCmdLineError") << clp.getErrorMessage();
OSMessageBox(msg.str(),LLStringUtil::null,OSMB_OK);
handleCommandLineError(clp);
return false;
}
@ -2457,7 +2465,11 @@ bool LLAppViewer::initConfiguration()
loadSettingsFromDirectory("UserSession");
// - apply command line settings
clp.notify();
if (! clp.notify())
{
handleCommandLineError(clp);
return false;
}
// Register the core crash option as soon as we can
// if we want gdb post-mortem on cores we need to be up and running

170
indra/newview/llcommandlineparser.cpp
View File

@ -38,6 +38,7 @@
#endif
#include <boost/program_options.hpp>
#include <boost/lexical_cast.hpp>
#include <boost/bind.hpp>
#include <boost/tokenizer.hpp>
#include <boost/assign/list_of.hpp>
@ -48,10 +49,12 @@
#include "llsdserialize.h"
#include "llerror.h"
#include "stringize.h"
#include <string>
#include <set>
#include <iostream>
#include <sstream>
#include <typeinfo>
#include "llcontrol.h"
@ -82,7 +85,7 @@ namespace
po::options_description gOptionsDesc;
po::positional_options_description gPositionalOptions;
po::variables_map gVariableMap;
const LLCommandLineParser::token_vector_t gEmptyValue;
void read_file_into_string(std::string& str, const std::basic_istream < char >& file)
@ -400,9 +403,19 @@ bool LLCommandLineParser::parseCommandLineFile(const std::basic_istream < char >
return parseCommandLineString(args);
}
void LLCommandLineParser::notify()
bool LLCommandLineParser::notify()
{
po::notify(gVariableMap);
try
{
po::notify(gVariableMap);
return true;
}
catch (const LLCLPError& e)
{
llwarns << "Caught Error: " << e.what() << llendl;
mErrorMsg = e.what();
return false;
}
}
void LLCommandLineParser::printOptions() const
@ -444,43 +457,129 @@ const LLCommandLineParser::token_vector_t& LLCommandLineParser::getOption(const
//----------------------------------------------------------------------------
// LLControlGroupCLP defintions
//----------------------------------------------------------------------------
void setControlValueCB(const LLCommandLineParser::token_vector_t& value,
const std::string& opt_name,
LLControlGroup* ctrlGroup)
namespace {
LLCommandLineParser::token_vector_t::value_type
onevalue(const std::string& option,
const LLCommandLineParser::token_vector_t& value)
{
// *FIX: Do sematic conversion here.
if (value.empty())
{
// What does it mean when the user specifies a command-line switch
// that requires a value, but omits the value? Complain.
throw LLCLPError(STRINGIZE("No value specified for --" << option << "!"));
}
else if (value.size() > 1)
{
llwarns << "Ignoring extra tokens specified for --"
<< option << "." << llendl;
}
return value[0];
}
void badvalue(const std::string& option,
const std::string& varname,
const std::string& type,
const std::string& value)
{
// If the user passes an unusable value for a command-line switch, it
// seems like a really bad idea to just ignore it, even with a log
// warning.
throw LLCLPError(STRINGIZE("Invalid value specified by command-line switch '" << option
<< "' for variable '" << varname << "' of type " << type
<< ": '" << value << "'"));
}
template <typename T>
T convertTo(const std::string& option,
const std::string& varname,
const LLCommandLineParser::token_vector_t::value_type& value)
{
try
{
return boost::lexical_cast<T>(value);
}
catch (const boost::bad_lexical_cast&)
{
badvalue(option, varname, typeid(T).name(), value);
// bogus return; compiler unaware that badvalue() won't return
return T();
}
}
void setControlValueCB(const LLCommandLineParser::token_vector_t& value,
const std::string& option,
LLControlVariable* ctrl)
{
// *FIX: Do semantic conversion here.
// LLSD (ImplString) Is no good for doing string to type conversion for...
// booleans
// compound types
// ?...
LLControlVariable* ctrl = ctrlGroup->getControl(opt_name);
if(NULL != ctrl)
{
switch(ctrl->type())
{
case TYPE_BOOLEAN:
if(value.size() > 1)
if (value.empty())
{
llwarns << "Ignoring extra tokens." << llendl;
}
if(value.size() > 0)
{
// There's a token. check the string for true/false/1/0 etc.
BOOL result = false;
BOOL gotSet = LLStringUtil::convertToBOOL(value[0], result);
if(gotSet)
{
ctrl->setValue(LLSD(result), false);
}
// Boolean-valued command-line switches are unusual. If you
// simply specify the switch without an explicit value, we can
// infer you mean 'true'.
ctrl->setValue(LLSD(true), false);
}
else
{
ctrl->setValue(LLSD(true), false);
// Only call onevalue() AFTER handling value.empty() case!
std::string token(onevalue(option, value));
// There's a token. check the string for true/false/1/0 etc.
BOOL result = false;
BOOL gotSet = LLStringUtil::convertToBOOL(token, result);
if (gotSet)
{
ctrl->setValue(LLSD(result), false);
}
else
{
badvalue(option, ctrl->getName(), "bool", token);
}
}
break;
case TYPE_U32:
{
std::string token(onevalue(option, value));
// To my surprise, for an unsigned target, lexical_cast() doesn't
// complain about an input string such as "-17". In that case, you
// get a very large positive result. So for U32, make sure there's
// no minus sign!
if (token.find('-') == std::string::npos)
{
ctrl->setValue(LLSD::Integer(convertTo<U32>(option, ctrl->getName(), token)),
false);
}
else
{
badvalue(option, ctrl->getName(), "unsigned", token);
}
break;
}
case TYPE_S32:
ctrl->setValue(convertTo<S32>(option, ctrl->getName(),
onevalue(option, value)), false);
break;
case TYPE_F32:
ctrl->setValue(convertTo<F32>(option, ctrl->getName(),
onevalue(option, value)), false);
break;
// It appears that no one has yet tried to define a command-line
// switch mapped to a settings variable of TYPE_VEC3, TYPE_VEC3D,
// TYPE_RECT, TYPE_COL4, TYPE_COL3. Such types would certainly seem to
// call for a bit of special handling here...
default:
{
// For the default types, let llsd do the conversion.
@ -497,16 +596,9 @@ void setControlValueCB(const LLCommandLineParser::token_vector_t& value,
ctrl->setValue(llsdArray, false);
}
else if(value.size() > 0)
else
{
if(value.size() > 1)
{
llwarns << "Ignoring extra tokens mapped to the setting: " << opt_name << "." << llendl;
}
LLSD llsdValue;
llsdValue.assign(LLSD::String(value[0]));
ctrl->setValue(llsdValue, false);
ctrl->setValue(onevalue(option, value), false);
}
}
break;
@ -514,12 +606,14 @@ void setControlValueCB(const LLCommandLineParser::token_vector_t& value,
}
else
{
llwarns << "Command Line option mapping '"
<< opt_name
<< "' not found! Ignoring."
<< llendl;
// This isn't anything a user can affect -- it's a misconfiguration on
// the part of the coder. Rub the coder's nose in the problem right
// away so even preliminary testing will surface it.
llerrs << "Command Line option --" << option
<< " maps to unknown setting!" << llendl;
}
}
} // anonymous namespace
void LLControlGroupCLP::configure(const std::string& config_filename, LLControlGroup* controlGroup)
{
@ -600,14 +694,14 @@ void LLControlGroupCLP::configure(const std::string& config_filename, LLControlG
<< long_name << " (map-to " << controlName << ")" << llendl;
}
if (! controlGroup->getControl(controlName))
LLControlVariable* ctrl = controlGroup->getControl(controlName);
if (! ctrl)
{
llerrs << "Option " << long_name << " specifies map-to " << controlName
<< " which does not exist" << llendl;
}
callback = boost::bind(setControlValueCB, _1,
controlName, controlGroup);
callback = boost::bind(setControlValueCB, _1, long_name, ctrl);
}
this->addOptionDesc(

2
indra/newview/llcommandlineparser.h
View File

@ -86,7 +86,7 @@ public:
*
* Use this to handle the results of parsing.
*/
void notify();
bool notify();
/** @brief Print a description of the configured options.
*