[FIXED] hasBehaviourExcept() with an empty option returns false even though a RLV_OPTION_MODIFIER active restriction exists

-> RlvCommand::markRefCounted() is called from RlvBehaviourGenericProcessor<RLV_OPTION_MODIFIER> as expected
  -> However, the RlvCommand reference we pass throughout the entire command processing line is a reference to the originally parsed command and not a reference to the copy that was stored in the RlvObject's command list
  -> The fact that we're mutating a constant is excusable in this case since only the command processor will know whether an option command should end up getting reference counted
  -> This bug only applies to hasBehaviourExcept() since the behaviour reference counter was still incremented/decremented successfully and hasBehaviourExcept() does a deep check on the individual commands instead
  -> Fixes FIRE-24215
meow-7.2.2
Kitty Barnett 2020-06-17 15:19:29 +02:00
parent 68a0bdb03f
commit 6f2009f33d
4 changed files with 47 additions and 40 deletions

View File

@ -438,42 +438,46 @@ bool RlvHandler::notifyCommandHandlers(rlvExtCommandHandler f, const RlvCommand&
}
// Checked: 2009-11-25 (RLVa-1.1.0f) | Modified: RLVa-1.1.0f
ERlvCmdRet RlvHandler::processCommand(const RlvCommand& rlvCmd, bool fFromObj)
ERlvCmdRet RlvHandler::processCommand(std::reference_wrapper<const RlvCommand> rlvCmd, bool fFromObj)
{
RLV_DEBUGS << "[" << rlvCmd.getObjectID() << "]: " << rlvCmd.asString() << RLV_ENDL;
{
const RlvCommand& rlvCmdTmp = rlvCmd; // Reference to the temporary with limited variable scope since we don't want it to leak below
if ( (isBlockedObject(rlvCmd.getObjectID())) && (RLV_TYPE_REMOVE != rlvCmd.getParamType()) && (RLV_TYPE_CLEAR != rlvCmd.getParamType()) )
{
RLV_DEBUGS << "\t-> blocked object" << RLV_ENDL;
return RLV_RET_FAILED_BLOCKED;
}
if (!rlvCmd.isValid())
{
RLV_DEBUGS << "\t-> invalid syntax" << RLV_ENDL;
return RLV_RET_FAILED_SYNTAX;
}
if (rlvCmd.isBlocked())
{
RLV_DEBUGS << "\t-> blocked command" << RLV_ENDL;
return RLV_RET_FAILED_DISABLED;
RLV_DEBUGS << "[" << rlvCmdTmp.getObjectID() << "]: " << rlvCmdTmp.asString() << RLV_ENDL;
if ( (isBlockedObject(rlvCmdTmp.getObjectID())) && (RLV_TYPE_REMOVE != rlvCmdTmp.getParamType()) && (RLV_TYPE_CLEAR != rlvCmdTmp.getParamType()) )
{
RLV_DEBUGS << "\t-> blocked object" << RLV_ENDL;
return RLV_RET_FAILED_BLOCKED;
}
if (!rlvCmdTmp.isValid())
{
RLV_DEBUGS << "\t-> invalid syntax" << RLV_ENDL;
return RLV_RET_FAILED_SYNTAX;
}
if (rlvCmdTmp.isBlocked())
{
RLV_DEBUGS << "\t-> blocked command" << RLV_ENDL;
return RLV_RET_FAILED_DISABLED;
}
}
// Using a stack for executing commands solves a few problems:
// - if we passed RlvObject::m_idObj for idObj somewhere and process a @clear then idObj points to invalid/cleared memory at the end
// - if command X triggers command Y along the way then getCurrentCommand()/getCurrentObject() still return Y even when finished
m_CurCommandStack.push(&rlvCmd); m_CurObjectStack.push(rlvCmd.getObjectID());
m_CurCommandStack.push(rlvCmd); m_CurObjectStack.push(rlvCmd.get().getObjectID());
const LLUUID& idCurObj = m_CurObjectStack.top();
ERlvCmdRet eRet = RLV_RET_UNKNOWN;
switch (rlvCmd.getParamType())
switch (rlvCmd.get().getParamType())
{
case RLV_TYPE_ADD: // Checked: 2009-11-26 (RLVa-1.1.0f) | Modified: RLVa-1.1.0f
{
if ( (m_Behaviours[rlvCmd.getBehaviourType()]) &&
( (RLV_BHVR_SETCAM == rlvCmd.getBehaviourType()) || (RLV_BHVR_SETDEBUG == rlvCmd.getBehaviourType()) || (RLV_BHVR_SETENV == rlvCmd.getBehaviourType()) ) )
ERlvBehaviour eBhvr = rlvCmd.get().getBehaviourType();
if ( (m_Behaviours[eBhvr]) && ( (RLV_BHVR_SETCAM == eBhvr) || (RLV_BHVR_SETDEBUG == eBhvr) || (RLV_BHVR_SETENV == eBhvr) ) )
{
// Some restrictions can only be held by one single object to avoid deadlocks
RLV_DEBUGS << "\t- " << rlvCmd.getBehaviour() << " is already set by another object => discarding" << RLV_ENDL;
RLV_DEBUGS << "\t- " << rlvCmd.get().getBehaviour() << " is already set by another object => discarding" << RLV_ENDL;
eRet = RLV_RET_FAILED_LOCK;
break;
}
@ -481,14 +485,14 @@ ERlvCmdRet RlvHandler::processCommand(const RlvCommand& rlvCmd, bool fFromObj)
rlv_object_map_t::iterator itObj = m_Objects.find(idCurObj); bool fAdded = false;
if (itObj != m_Objects.end())
{
RlvObject& rlvObj = itObj->second;
fAdded = rlvObj.addCommand(rlvCmd);
// Add the command to an existing object
rlvCmd = itObj->second.addCommand(rlvCmd, fAdded);
}
else
{
RlvObject rlvObj(idCurObj);
fAdded = rlvObj.addCommand(rlvCmd);
itObj = m_Objects.insert(std::pair<LLUUID, RlvObject>(idCurObj, rlvObj)).first;
// Create a new RLV object and then add the command to it (and grab its reference)
itObj = m_Objects.insert(std::pair<LLUUID, RlvObject>(idCurObj, RlvObject(idCurObj))).first;
rlvCmd = itObj->second.addCommand(rlvCmd, fAdded);
}
RLV_DEBUGS << "\t- " << ( (fAdded) ? "adding behaviour" : "skipping duplicate" ) << RLV_ENDL;
@ -563,12 +567,13 @@ ERlvCmdRet RlvHandler::processCommand(const RlvCommand& rlvCmd, bool fFromObj)
// Checked: 2009-11-25 (RLVa-1.1.0f) | Modified: RLVa-1.1.0f
ERlvCmdRet RlvHandler::processCommand(const LLUUID& idObj, const std::string& strCommand, bool fFromObj)
{
const RlvCommand rlvCmd(idObj, strCommand);
if (STATE_STARTED != LLStartUp::getStartupState())
{
m_Retained.push_back(RlvCommand(idObj, strCommand));
m_Retained.push_back(rlvCmd);
return RLV_RET_RETAINED;
}
return processCommand(RlvCommand(idObj, strCommand), fFromObj);
return processCommand(std::ref(rlvCmd), fFromObj);
}
// Checked: 2010-02-27 (RLVa-1.2.0a) | Modified: RLVa-1.1.0f
@ -583,7 +588,7 @@ void RlvHandler::processRetainedCommands(ERlvBehaviour eBhvrFilter /*=RLV_BHVR_U
if ( ((RLV_BHVR_UNKNOWN == eBhvrFilter) || (rlvCmd.getBehaviourType() == eBhvrFilter)) &&
((RLV_TYPE_UNKNOWN == eTypeFilter) || (rlvCmd.getParamType() == eTypeFilter)) )
{
processCommand(rlvCmd, true);
processCommand(std::ref(rlvCmd), true);
m_Retained.erase(itCurCmd);
}
}

View File

@ -133,7 +133,7 @@ public:
bool processIMQuery(const LLUUID& idSender, const std::string& strCommand);
// Returns a pointer to the currently executing command (do *not* save this pointer)
const RlvCommand* getCurrentCommand() const { return (!m_CurCommandStack.empty()) ? m_CurCommandStack.top() : NULL; }
const RlvCommand* getCurrentCommand() const { return (!m_CurCommandStack.empty()) ? &m_CurCommandStack.top().get() : nullptr; }
// Returns the UUID of the object we're currently executing a command for
const LLUUID& getCurrentObject() const { return (!m_CurObjectStack.empty()) ? m_CurObjectStack.top() : LLUUID::null; }
@ -203,7 +203,7 @@ public:
* Command processing
*/
protected:
ERlvCmdRet processCommand(const RlvCommand& rlvCmd, bool fFromObj);
ERlvCmdRet processCommand(std::reference_wrapper<const RlvCommand> rlvCmdRef, bool fFromObj);
ERlvCmdRet processClearCommand(const RlvCommand& rlvCmd);
// Command handlers (RLV_TYPE_ADD and RLV_TYPE_CLEAR)
@ -244,7 +244,7 @@ protected:
rlv_command_list_t m_Retained;
RlvGCTimer* m_pGCTimer;
std::stack<const RlvCommand*> m_CurCommandStack;// Convenience (see @tpto)
std::stack<std::reference_wrapper<const RlvCommand>> m_CurCommandStack; // Convenience (see @tpto)
std::stack<LLUUID> m_CurObjectStack; // Convenience (see @tpto)
rlv_behaviour_signal_t m_OnBehaviour;

View File

@ -642,8 +642,9 @@ RlvCommand::RlvCommand(const LLUUID& idObj, const std::string& strCommand)
}
RlvCommand::RlvCommand(const RlvCommand& rlvCmd, ERlvParamType eParamType)
: m_fValid(rlvCmd.m_fValid), m_idObj(rlvCmd.m_idObj), m_strBehaviour(rlvCmd.m_strBehaviour), m_pBhvrInfo(rlvCmd.m_pBhvrInfo),
m_eParamType( (RLV_TYPE_UNKNOWN == eParamType) ? rlvCmd.m_eParamType : eParamType),m_fStrict(rlvCmd.m_fStrict), m_strOption(rlvCmd.m_strOption), m_strParam(rlvCmd.m_strParam), m_fRefCounted(false)
: m_fValid(rlvCmd.m_fValid), m_idObj(rlvCmd.m_idObj), m_strBehaviour(rlvCmd.m_strBehaviour), m_pBhvrInfo(rlvCmd.m_pBhvrInfo)
, m_eParamType( (RLV_TYPE_UNKNOWN == eParamType) ? rlvCmd.m_eParamType : eParamType),m_fStrict(rlvCmd.m_fStrict), m_strOption(rlvCmd.m_strOption)
, m_strParam(rlvCmd.m_strParam), m_fRefCounted(rlvCmd.m_fRefCounted)
{
}
@ -989,7 +990,7 @@ RlvObject::RlvObject(const LLUUID& idObj) : m_idObj(idObj), m_nLookupMisses(0)
m_idRoot = (pObj) ? pObj->getRootEdit()->getID() : LLUUID::null;
}
bool RlvObject::addCommand(const RlvCommand& rlvCmd)
const RlvCommand& RlvObject::addCommand(const RlvCommand& rlvCmd, bool& fAdded)
{
RLV_ASSERT(RLV_TYPE_ADD == rlvCmd.getParamType());
@ -999,14 +1000,15 @@ bool RlvObject::addCommand(const RlvCommand& rlvCmd)
if ( (itCmd->getBehaviour() == rlvCmd.getBehaviour()) && (itCmd->getOption() == rlvCmd.getOption()) &&
(itCmd->isStrict() == rlvCmd.isStrict() ) )
{
return false;
fAdded = false;
return *itCmd;
}
}
// Now that we know it's not a duplicate, add it to the end of the list
m_Commands.push_back(rlvCmd);
return true;
fAdded = true;
return m_Commands.back();
}
bool RlvObject::removeCommand(const RlvCommand& rlvCmd)

View File

@ -434,8 +434,8 @@ public:
* Member functions
*/
public:
bool addCommand(const RlvCommand& rlvCmd);
bool removeCommand(const RlvCommand& rlvCmd);
const RlvCommand& addCommand(const RlvCommand& rlvCmd, bool& fAdded);
bool removeCommand(const RlvCommand& rlvCmd);
std::string getStatusString(const std::string& strFilter, const std::string& strSeparator) const;
bool hasBehaviour(ERlvBehaviour eBehaviour, bool fStrictOnly) const;