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

Fix for crash in XMLRPC reply decoding on login with large inventories 

Commit 2ea5ac0c43 introduced a crash bug
due to the recursive construction of the XMLTreeNode wrapper class.
The constructor of the said class typically recurses twice as many times
as there are entries in the user's inventory list.

This commit:
- Moves the fromXMLRPCValue() method and its helper functions from the LLSD
  class/module to the LLXMLNode class, where it belongs, thus making
  LLSD::TreeNode (which was a wrapper class to avoid making llcommon
  dependant on llxml, which is still the case after this commit) totally
  moot; the fromXMLRPCValue() call is now done directly on the LLXMLNode.
- Moves the XML and XMLRPC decoding code out of the HTTP coroutine
  LLXMLRPCTransaction::Handler (coroutines got an even smaller and fixed
  stack), and into LLXMLRPCTransaction::Impl::process().
- Removes XMLTreeNode entirely, fixing the crash as a result.
master
Henri Beauchamp 2024-07-08 23:18:02 +02:00
parent 94a66b5584
commit 989cfe2f70
5 changed files with 269 additions and 252 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

147
indra/llcommon/llsd.cpp
View File

@ -1018,153 +1018,6 @@ const LLSD::String& LLSD::asStringRef() const { return safe(impl).asStringRef();
LLSD::String LLSD::asXMLRPCValue() const { return "<value>" + safe(impl).asXMLRPCValue() + "</value>"; }
static bool inline check(bool condition, const char* warning_message)
{
if (!condition)
{
LL_WARNS() << warning_message << LL_ENDL;
}
return condition;
}
static bool parseXMLRPCArrayValue(LLSD& target, LLSD::TreeNode* node)
{
LLSD::TreeNode* data = node->getFirstChild();
if (!check(data, "No array inner XML element (<data> expected)") ||
!check(data->hasName("data"), "Invalid array inner XML element (<data> expected)") ||
!check(!data->getNextSibling(), "Multiple array inner XML elements (single <data> expected)"))
return false;
for (LLSD::TreeNode* item = data->getFirstChild(); item; item = item->getNextSibling())
{
LLSD value;
if (!value.fromXMLRPCValue(item))
return false;
target.append(value);
}
return true;
}
static bool parseXMLRPCStructValue(LLSD& target, LLSD::TreeNode* node)
{
for (LLSD::TreeNode* item = node->getFirstChild(); item; item = item->getNextSibling())
{
if (!check(item->hasName("member"), "Invalid struct inner XML element (<member> expected)"))
return false;
std::string name;
LLSD value;
for (LLSD::TreeNode* subitem = item->getFirstChild(); subitem; subitem = subitem->getNextSibling())
{
if (subitem->hasName("name"))
{
name = LLStringFn::xml_decode(subitem->getTextContents());
}
else if (!value.fromXMLRPCValue(subitem))
{
return false;
}
}
if (!check(!name.empty(), "Empty struct member name"))
return false;
target.insert(name, value);
}
return true;
}
bool LLSD::fromXMLRPCValue(TreeNode* node)
{
clear();
llassert(node);
if (!node)
return false;
if (!check(node->hasName("value"), "Invalid XML element (<value> expected)"))
return false;
TreeNode* inner = node->getFirstChild();
if (!inner)
{
check(false, "No inner XML element (value type expected)");
// Value with no type qualifier is treated as string
assign(LLStringFn::xml_decode(node->getTextContents()));
return true;
}
if (!check(!inner->getNextSibling(), "Multiple inner XML elements (single expected)"))
return false;
if (inner->hasName("string"))
{
assign(LLStringFn::xml_decode(inner->getTextContents()));
return true;
}
if (inner->hasName("int") || inner->hasName("i4"))
{
assign(std::stoi(inner->getTextContents()));
return true;
}
if (inner->hasName("double"))
{
assign(std::stod(inner->getTextContents()));
return true;
}
if (inner->hasName("boolean"))
{
assign(!!std::stoi(inner->getTextContents()));
return true;
}
if (inner->hasName("dateTime.iso8601"))
{
assign(Date(inner->getTextContents()));
return true;
}
if (inner->hasName("base64"))
{
std::string decoded = LLBase64::decodeAsString(inner->getTextContents());
Binary binary(decoded.size());
memcpy(binary.data(), decoded.data(), decoded.size());
assign(binary);
return true;
}
if (inner->hasName("array"))
{
if (!parseXMLRPCArrayValue(*this, inner))
{
clear();
return false;
}
return true;
}
if (inner->hasName("struct"))
{
if (!parseXMLRPCStructValue(*this, inner))
{
clear();
return false;
}
return true;
}
check(false, "Unknown inner XML element (known value type expected)");
// Value with unknown type qualifier is treated as string
assign(LLStringFn::xml_decode(inner->getTextContents()));
return true;
}
// const char * helpers
LLSD::LLSD(const char* v) : impl(0) { ALLOC_LLSD_OBJECT; assign(v); }
void LLSD::assign(const char* v)

10
indra/llcommon/llsd.h
View File

@ -290,16 +290,6 @@ public:
// See http://xmlrpc.com/spec.md
String asXMLRPCValue() const;
struct TreeNode
{
virtual bool hasName(const String& name) const = 0;
virtual String getTextContents() const = 0;
virtual TreeNode* getFirstChild() const = 0;
virtual TreeNode* getNextSibling() const = 0;
};
bool fromXMLRPCValue(TreeNode* node);
operator Boolean() const { return asBoolean(); }
operator Integer() const { return asInteger(); }
operator Real() const { return asReal(); }

172
indra/llxml/llxmlnode.cpp
View File

@ -38,7 +38,9 @@
#include "v3math.h"
#include "v3dmath.h"
#include "v4math.h"
#include "llbase64.h"
#include "llquaternion.h"
#include "llsd.h"
#include "llstring.h"
#include "lluuid.h"
#include "lldir.h"
@ -3263,3 +3265,173 @@ S32 LLXMLNode::getLineNumber()
{
return mLineNumber;
}
bool LLXMLNode::parseXmlRpcArrayValue(LLSD& target)
{
LLXMLNode* datap = getFirstChild().get();
if (!datap)
{
LL_WARNS() << "No inner XML element." << LL_ENDL;
return false;
}
if (!datap->hasName("data"))
{
LL_WARNS() << "No inner XML element (<data> expected, got: "
<< datap->mName->mString << ")" << LL_ENDL;
return false;
}
if (datap->getNextSibling().get())
{
LL_WARNS() << "Multiple inner XML elements (single <data> expected)"
<< LL_ENDL;
return false;
}
U32 i = 0;
for (LLXMLNode* itemp = datap->getFirstChild().get(); itemp;
itemp = itemp->getNextSibling().get())
{
LLSD value;
if (!itemp->fromXMLRPCValue(value))
{
return false;
}
target.append(value);
++i;
}
return true;
}
bool LLXMLNode::parseXmlRpcStructValue(LLSD& target)
{
std::string name;
LLSD value;
for (LLXMLNode* itemp = getFirstChild().get(); itemp;
itemp = itemp->getNextSibling().get())
{
if (!itemp->hasName("member"))
{
LL_WARNS() << "Invalid inner XML element (<member> expected, got: <"
<< itemp->mName->mString << ">" << LL_ENDL;
return false;
}
name.clear();
value.clear();
for (LLXMLNode* chilp = itemp->getFirstChild().get(); chilp;
chilp = chilp->getNextSibling().get())
{
if (chilp->hasName("name"))
{
name = LLStringFn::xml_decode(chilp->getTextContents());
}
else if (!chilp->fromXMLRPCValue(value))
{
return false;
}
}
if (name.empty())
{
LL_WARNS() << "Empty struct member name" << LL_ENDL;
return false;
}
target.insert(name, value);
}
return true;
}
bool LLXMLNode::fromXMLRPCValue(LLSD& target)
{
target.clear();
if (!hasName("value"))
{
LL_WARNS() << "Invalid XML element (<value> expected), got: <"
<< mName->mString << ">" << LL_ENDL;
return false;
}
LLXMLNode* childp = getFirstChild().get();
if (!childp)
{
LL_WARNS() << "No inner XML element (value type expected)" << LL_ENDL;
// Value with no type qualifier is treated as string
target.assign(LLStringFn::xml_decode(getTextContents()));
return true;
}
if (childp->getNextSibling())
{
LL_WARNS() << "Multiple inner XML elements (single expected)"
<< LL_ENDL;
return false;
}
if (childp->hasName("string"))
{
target.assign(LLStringFn::xml_decode(childp->getTextContents()));
return true;
}
if (childp->hasName("int") || childp->hasName("i4"))
{
target.assign(std::stoi(childp->getTextContents()));
return true;
}
if (childp->hasName("double"))
{
target.assign(std::stod(childp->getTextContents()));
return true;
}
if (childp->hasName("boolean"))
{
target.assign(std::stoi(childp->getTextContents()) != 0);
return true;
}
if (childp->hasName("dateTime.iso8601"))
{
target.assign(LLSD::Date(childp->getTextContents()));
return true;
}
if (childp->hasName("base64"))
{
std::string decoded =
LLBase64::decodeAsString(inner->getTextContents());
size_t size = decoded.size();
LLSD::Binary binary(size);
if (size)
{
memcpy((void*)binary.data(), (void*)decoded.data(), size);
}
target.assign(binary);
return true;
}
if (childp->hasName("array"))
{
if (!childp->parseXmlRpcArrayValue(target))
{
target.clear();
return false;
}
return true;
}
if (childp->hasName("struct"))
{
if (!childp->parseXmlRpcStructValue(target))
{
target.clear();
return false;
}
return true;
}
LL_WARNS() << "Unknown inner XML element (known value type expected)"
<< LL_ENDL;
// Value with unknown type qualifier is treated as string
target.assign(LLStringFn::xml_decode(childp->getTextContents()));
return true;
}

9
indra/llxml/llxmlnode.h
View File

@ -50,6 +50,7 @@ class LLVector3d;
class LLQuaternion;
class LLColor4;
class LLColor4U;
class LLSD;
struct CompareAttributes
@ -284,12 +285,18 @@ public:
void setAttributes(ValueType type, U32 precision, Encoding encoding, U32 length);
// void appendValue(const std::string& value); // Unused
bool fromXMLRPCValue(LLSD& target);
// Unit Testing
void createUnitTest(S32 max_num_children);
bool performUnitTest(std::string &error_buffer);
protected:
bool removeChild(LLXMLNode* child);
bool isFullyDefault();
bool parseXmlRpcArrayValue(LLSD& target);
bool parseXmlRpcStructValue(LLSD& target);
public:
std::string mID; // The ID attribute of this node
@ -328,8 +335,6 @@ protected:
static const char *skipNonWhitespace(const char *str);
static const char *parseInteger(const char *str, U64 *dest, bool *is_negative, U32 precision, Encoding encoding);
static const char *parseFloat(const char *str, F64 *dest, U32 precision, Encoding encoding);
bool isFullyDefault();
};
#endif // LL_LLXMLNODE

183
indra/newview/llxmlrpctransaction.cpp
View File

@ -68,17 +68,13 @@ class LLXMLRPCTransaction::Handler : public LLCore::HttpHandler
{
public:
Handler(LLCore::HttpRequest::ptr_t &request, LLXMLRPCTransaction::Impl *impl);
virtual ~Handler();
virtual void onCompleted(LLCore::HttpHandle handle, LLCore::HttpResponse * response);
void onCompleted(LLCore::HttpHandle handle,
LLCore::HttpResponse* response) override;
typedef std::shared_ptr<LLXMLRPCTransaction::Handler> ptr_t;
private:
bool parseResponse(LLXMLNodePtr root);
bool parseValue(LLSD& target, LLXMLNodePtr source);
LLXMLRPCTransaction::Impl *mImpl;
LLCore::HttpRequest::ptr_t mRequest;
};
@ -104,6 +100,8 @@ public:
std::string mResponseText;
LLSD mResponseData;
bool mHasResponse;
bool mResponseParsed;
std::string mCertStore;
LLSD mErrorCertData;
@ -120,6 +118,10 @@ public:
void setStatus(EStatus code, const std::string& message = "", const std::string& uri = "");
void setHttpStatus(const LLCore::HttpStatus &status);
private:
bool parseResponse(LLXMLNodePtr root);
bool parseValue(LLSD& target, LLXMLNodePtr source);
};
LLXMLRPCTransaction::Handler::Handler(LLCore::HttpRequest::ptr_t &request,
@ -129,10 +131,6 @@ LLXMLRPCTransaction::Handler::Handler(LLCore::HttpRequest::ptr_t &request,
{
}
LLXMLRPCTransaction::Handler::~Handler()
{
}
void LLXMLRPCTransaction::Handler::onCompleted(LLCore::HttpHandle handle,
LLCore::HttpResponse * response)
{
@ -159,7 +157,6 @@ void LLXMLRPCTransaction::Handler::onCompleted(LLCore::HttpHandle handle,
return;
}
mImpl->setStatus(LLXMLRPCTransaction::StatusComplete);
mImpl->mTransferStats = response->getTransferStats();
// The contents of a buffer array are potentially noncontiguous, so we
@ -169,88 +166,12 @@ void LLXMLRPCTransaction::Handler::onCompleted(LLCore::HttpHandle handle,
body->read(0, mImpl->mResponseText.data(), body->size());
LLXMLNodePtr root;
if (!LLXMLNode::parseBuffer(mImpl->mResponseText.data(), body->size(), root, nullptr))
{
LL_WARNS() << "Failed parsing XML response; request URI: " << mImpl->mURI << LL_ENDL;
return;
}
if (!parseResponse(root))
return;
LL_INFOS() << "XML response parsed successfully; request URI: " << mImpl->mURI << LL_ENDL;
}
struct XMLTreeNode final : public LLSD::TreeNode
{
XMLTreeNode(const LLXMLNodePtr impl)
: mImpl(impl)
, mFirstChild(impl ? create(impl->getFirstChild()) : nullptr)
, mNextSibling(impl ? create(impl->getNextSibling()) : nullptr)
{
}
static XMLTreeNode* create(LLXMLNodePtr node) { return node ? new XMLTreeNode(node) : nullptr; }
virtual bool hasName(const LLSD::String& name) const override { return mImpl && mImpl->hasName(name); }
virtual LLSD::String getTextContents() const override { return mImpl ? mImpl->getTextContents() : LLStringUtil::null; }
virtual TreeNode* getFirstChild() const override { return mFirstChild.get(); }
virtual TreeNode* getNextSibling() const override { return mNextSibling.get(); }
private:
const LLXMLNodePtr mImpl;
const std::shared_ptr<XMLTreeNode> mFirstChild;
const std::shared_ptr<XMLTreeNode> mNextSibling;
};
bool LLXMLRPCTransaction::Handler::parseResponse(LLXMLNodePtr root)
{
// We have alreasy checked in LLXMLNode::parseBuffer()
// that root contains exactly one child
if (!root->hasName("methodResponse"))
{
LL_WARNS() << "Invalid root element in XML response; request URI: " << mImpl->mURI << LL_ENDL;
return false;
}
LLXMLNodePtr first = root->getFirstChild();
LLXMLNodePtr second = first->getFirstChild();
if (!first->getNextSibling() && second && !second->getNextSibling())
{
if (first->hasName("fault"))
{
LLSD fault;
if (parseValue(fault, second) &&
fault.isMap() && fault.has("faultCode") && fault.has("faultString"))
{
LL_WARNS() << "Request failed;"
<< " faultCode: '" << fault.get("faultCode").asString() << "',"
<< " faultString: '" << fault.get("faultString").asString() << "',"
<< " request URI: " << mImpl->mURI << LL_ENDL;
return false;
}
}
else if (first->hasName("params") &&
second->hasName("param") && !second->getNextSibling())
{
LLXMLNodePtr third = second->getFirstChild();
if (third && !third->getNextSibling() && parseValue(mImpl->mResponseData, third))
{
return true;
}
}
}
LL_WARNS() << "Invalid response format; request URI: " << mImpl->mURI << LL_ENDL;
return false;
}
bool LLXMLRPCTransaction::Handler::parseValue(LLSD& target, LLXMLNodePtr source)
{
XMLTreeNode tn(source);
return target.fromXMLRPCValue(&tn);
// We do not do the parsing in the HTTP coroutine, since it could exhaust
// the coroutine stack in extreme cases. Instead, we flag the data buffer
// as ready, and let mImpl decode it in its process() method, on the main
// coroutine. HB
mImpl->mHasResponse = true;
mImpl->setStatus(LLXMLRPCTransaction::StatusComplete);
}
//=========================================================================
@ -265,6 +186,8 @@ LLXMLRPCTransaction::Impl::Impl
: mHttpRequest()
, mStatus(LLXMLRPCTransaction::StatusNotStarted)
, mURI(uri)
, mHasResponse(false)
, mResponseParsed(false)
{
LLCore::HttpOptions::ptr_t httpOpts;
LLCore::HttpHeaders::ptr_t httpHeaders;
@ -327,6 +250,57 @@ LLXMLRPCTransaction::Impl::Impl
mURI, body.get(), httpOpts, httpHeaders, mHandler);
}
bool LLXMLRPCTransaction::Impl::parseResponse(LLXMLNodePtr root)
{
// We have already checked in LLXMLNode::parseBuffer() that root contains
// exactly one child.
if (!root->hasName("methodResponse"))
{
LL_WARNS() << "Invalid root element in XML response; request URI: "
<< mURI << LL_ENDL;
return false;
}
LLXMLNodePtr first = root->getFirstChild();
LLXMLNodePtr second = first->getFirstChild();
if (first && !first->getNextSibling() && second &&
!second->getNextSibling())
{
if (first->hasName("fault"))
{
LLSD fault;
if (parseValue(fault, second) && fault.isMap() &&
fault.has("faultCode") && fault.has("faultString"))
{
LL_WARNS() << "Request failed. faultCode: '"
<< fault.get("faultCode").asString()
<< "', faultString: '"
<< fault.get("faultString").asString()
<< "', request URI: " << mURI << LL_ENDL;
return false;
}
}
else if (first->hasName("params") &&
second->hasName("param") && !second->getNextSibling())
{
LLXMLNodePtr third = second->getFirstChild();
if (third && !third->getNextSibling() &&
parseValue(mResponseData, third))
{
return true;
}
}
}
LL_WARNS() << "Invalid response format; request URI: " << mURI << LL_ENDL;
return false;
}
bool LLXMLRPCTransaction::Impl::parseValue(LLSD& target, LLXMLNodePtr src)
{
return src->fromXMLRPCValue(target);
}
bool LLXMLRPCTransaction::Impl::process()
{
if (!mPostH || !mHttpRequest)
@ -335,6 +309,29 @@ bool LLXMLRPCTransaction::Impl::process()
return true; //failed, quit.
}
// Parse the response when we have one and it has not yet been parsed. HB
if (mHasResponse && !mResponseParsed)
{
LLXMLNodePtr root;
if (!LLXMLNode::parseBuffer(mResponseText.data(), mResponseText.size(),
root, nullptr))
{
LL_WARNS() << "Failed parsing XML in response; request URI: "
<< mURI << LL_ENDL;
}
else if (parseResponse(root))
{
LL_INFOS() << "XMLRPC response parsed successfully; request URI: "
<< mURI << LL_ENDL;
}
else
{
LL_WARNS() << "XMLRPC response parsing failed; request URI: "
<< mURI << LL_ENDL;
}
mResponseParsed = true;
}
switch (mStatus)
{
case LLXMLRPCTransaction::StatusComplete: