From f90c8559b3dac7e6c78c8da5727c0bf08f045fb8 Mon Sep 17 00:00:00 2001 From: Rye Date: Mon, 2 Nov 2026 16:44:05 -0500 Subject: [PATCH 01/10] LLMessageSystem acks a reliable packet once A packet taken into the inbound queue, as drainUdpSocket does when a frame's messages run over, is acked as it comes off the socket, before it waits there. checkMessages acked it a second time when it read it, as a packet handled or as a duplicate, so every such packet went into the acks twice. The packet now carries whether it was acked, and checkMessages acks only one that was not. Acking on arrival is deliberate (a1b68fb8ed, "Drop fewer packets on UDP packet flood"): the sender stops resending a packet while it waits in the queue, however long a flood keeps it there. So a queued packet that checkMessages then refuses, as one no template has or as a trusted message on an untrusted circuit, stays acked, as it has been since that change; only the second ack goes. A packet read straight from the socket is acked as it is read, and not when it is refused, as before. Co-Authored-By: Claude Opus 5.5 --- indra/llmessage/llpacketbuffer.h | 6 ++ indra/llmessage/message.cpp | 28 ++++-- indra/llmessage/message.h | 5 +- indra/llmessage/tests/message_test.cpp | 122 +++++++++++++++++++++++++ 4 files changed, 152 insertions(+), 9 deletions(-) diff --git a/indra/llmessage/llpacketbuffer.h b/indra/llmessage/llpacketbuffer.h index a08e6c22e5e..64950123629 100644 --- a/indra/llmessage/llpacketbuffer.h +++ b/indra/llmessage/llpacketbuffer.h @@ -55,12 +55,18 @@ class LLPacketBuffer bool getPacketIDChecked() const { return mPacketIDChecked; } void setPacketIDChecked(bool checked) { mPacketIDChecked = checked; } + // Whether this packet, a reliable one, was acked as it was read off the + // socket, before it was sorted into the inbound queue. + bool getAcked() const { return mAcked; } + void setAcked(bool acked) { mAcked = acked; } + protected: char mData[NET_BUFFER_SIZE]; // packet data /* Flawfinder : ignore */ S32 mSize; // size of buffer in bytes LLHost mHost; // source/dest IP and port LLHost mReceivingIF; // source/dest IP and port bool mPacketIDChecked = false; + bool mAcked = false; }; #endif diff --git a/indra/llmessage/message.cpp b/indra/llmessage/message.cpp index 4ea1654b7a3..c27cd9ed599 100644 --- a/indra/llmessage/message.cpp +++ b/indra/llmessage/message.cpp @@ -521,12 +521,13 @@ bool LLMessageSystem::checkMessages(LockMessageChecker&, S64 frame_count, S32 num_acks = 0; S32 true_rcv_size = 0; bool recv_packet_id_checked = false; + bool recv_acked = false; U8* buffer = mTrueReceiveBuffer; if (!faked_message) { - mTrueReceiveSize = receivePacketOrDrop((char *)mTrueReceiveBuffer, recv_packet_id_checked); + mTrueReceiveSize = receivePacketOrDrop((char *)mTrueReceiveBuffer, recv_packet_id_checked, recv_acked); receive_size = mTrueReceiveSize; } else @@ -649,8 +650,12 @@ bool LLMessageSystem::checkMessages(LockMessageChecker&, S64 frame_count, { // We need to ACK here to suppress // further resends of packets we've - // already seen. - cdp->collectRAck(mCurrentRecvPacketID); + // already seen, unless this copy was + // acked as it came in. + if (!recv_acked) + { + cdp->collectRAck(mCurrentRecvPacketID); + } LL_DEBUGS("Messaging") << "Discarding duplicate from " << host << LL_ENDL; if(mVerboseLog) @@ -731,8 +736,12 @@ bool LLMessageSystem::checkMessages(LockMessageChecker&, S64 frame_count, // Add to the recently received list for duplicate suppression cdp->mRecentlyReceivedReliablePackets[mCurrentRecvPacketID] = getMessageTimeUsecs(); - // Put it onto the list of packets to be acked - cdp->collectRAck(mCurrentRecvPacketID); + // Put it onto the list of packets to be acked, unless + // it was acked as it came in + if (!recv_acked) + { + cdp->collectRAck(mCurrentRecvPacketID); + } mReliablePacketsIn++; } } @@ -965,9 +974,10 @@ void LLMessageSystem::setNetImpairment(F32 loss_percent, F32 burst_packets, F32 } #endif -S32 LLMessageSystem::receivePacketOrDrop(char* datap, bool& packet_id_already_checked) +S32 LLMessageSystem::receivePacketOrDrop(char* datap, bool& packet_id_already_checked, bool& already_acked) { packet_id_already_checked = false; + already_acked = false; #if AL_NET_IMPAIRMENT // Steady-state traffic arrives here, one direct socket read per call. The drain that feeds the @@ -995,6 +1005,7 @@ S32 LLMessageSystem::receivePacketOrDrop(char* datap, bool& packet_id_already_ch mLastSender = pkt.getHost(); mLastReceivingIF = pkt.getReceivingInterface(); packet_id_already_checked = pkt.getPacketIDChecked(); + already_acked = pkt.getAcked(); if (packet_size > 0) { @@ -1150,10 +1161,13 @@ S32 LLMessageSystem::bufferInboundPacket() if (cdp) { - // ACK inbound reliable packet ASAP + // ACK inbound reliable packet ASAP, so its sender stops resending + // it while it waits in the queue, whatever checkMessages() makes + // of it, which does not ack it again. if ((data[0] & LL_RELIABLE_FLAG)) { cdp->collectRAck(recv_packet_id); + pkt.setAcked(true); } // Check packet sequencing here, in true socket-arrival order, before diff --git a/indra/llmessage/message.h b/indra/llmessage/message.h index 2cb5f60e94e..29c536ccbb1 100644 --- a/indra/llmessage/message.h +++ b/indra/llmessage/message.h @@ -983,9 +983,10 @@ class LLMessageSystem : public LLMessageSenderInterface // Receive one packet: pop from ring if buffered, else read from mSocket. // Sets mLastSender and mLastReceivingIF. // Sets packet_id_already_checked to whether checkPacketInID() was already - // run for this packet back when it was buffered (see bufferInboundPacket()). + // run for this packet back when it was buffered (see bufferInboundPacket()), + // and already_acked to whether it was acked then. // Returns packet_size, or 0 if no packet or packet was dropped. - S32 receivePacketOrDrop(char* datap, bool& packet_id_already_checked); + S32 receivePacketOrDrop(char* datap, bool& packet_id_already_checked, bool& already_acked); public: #if AL_NET_IMPAIRMENT diff --git a/indra/llmessage/tests/message_test.cpp b/indra/llmessage/tests/message_test.cpp index 8ab7421f612..ce233038a87 100644 --- a/indra/llmessage/tests/message_test.cpp +++ b/indra/llmessage/tests/message_test.cpp @@ -35,6 +35,8 @@ #include "message_prehash.h" #include "net.h" +#include + #if !LL_WINDOWS #include #include @@ -59,6 +61,12 @@ namespace ll_test { return circuit.mAcks.size(); } + + // How many of the acks waiting are for packet `id`. + static size_t acksWaitingFor(const LLCircuitData& circuit, TPACKETID id) + { + return std::count(circuit.mAcks.begin(), circuit.mAcks.end(), id); + } }; } @@ -622,5 +630,119 @@ namespace CHECK(sDuplicateTestsHandled == 2); } } + + // Takes what has come in into the inbound queue, as the viewer does when + // a frame's messages run over, until `packets` wait there or a second + // has passed. + void queue_until(LLMessageSystem& system, S32 packets) + { + const F64 give_up = LLTimer::getTotalSeconds() + 1.0; + while (system.drainUdpSocket() < packets && LLTimer::getTotalSeconds() < give_up) + { + ms_sleep(1); + } + } + + // Reads every packet waiting in the inbound queue. + void read_queued(LLMessageSystem& system) + { + LockMessageChecker lmc(&system); + while (system.getNumBufferedPackets() > 0) + { + lmc.checkMessages(); + } + } + + // Reads what has come in until the system has refused `packets` packets + // in all, or a second has passed. + void read_until_refused(LLMessageSystem& system, U32 packets) + { + LockMessageChecker lmc(&system); + const F64 give_up = LLTimer::getTotalSeconds() + 1.0; + while (system.mInvalidOnCircuitPackets < packets && LLTimer::getTotalSeconds() < give_up) + { + if (!lmc.checkMessages()) + { + ms_sleep(1); + } + } + } + + // A packet taken into the inbound queue is acked as it comes in, before + // it is read, so that its sender stops resending it while it waits; one + // read straight from the socket is acked as it is read, unless it is + // refused. + TEST_CASE_METHOD(LLMessageSystemTestData, "LLMessageSystem: a reliable packet is acked once whether or not it waited in the queue", "[LLMessageSystem]") + { + const std::string template_path = mTestConfigDir + mSep + "message_template.msg"; + const TemplateFile message_template(template_path, DUPLICATE_TEST_TEMPLATE); + LLMessageSystem system(template_path, NET_USE_OS_ASSIGNED_PORT, 1, 0, 0, false, 5.f, 100.f); + REQUIRE(system.isOK()); + const UseMessageSystem use(system); + system.setHandlerFunc("DuplicateTest", handle_duplicate_test); + sDuplicateTestsHandled = 0; + + const SenderSocket sender; + REQUIRE(sender.mResult == 0); + const LLHost sender_host("127.0.0.1", sender.mPort); + system.enableCircuit(sender_host, true); + const LLCircuitData* circuit = system.mCircuitInfo.findCircuit(sender_host); + REQUIRE(circuit != nullptr); + const LLHost receiver("127.0.0.1", system.getListenPort()); + + // Low message `message` numbered `id`: 1 is DuplicateTest, and 2 is + // no message the template has, which the system refuses. + auto send = [&](U8 id, U8 message) + { + const U8 packet[] = { LL_RELIABLE_FLAG, 0, 0, 0, id, 0, 0xff, 0xff, 0x00, message, 1, 2, 3, 4 }; + REQUIRE(send_packet(sender.mSocket, (const char*)packet, (int)sizeof(packet), receiver.getAddress(), receiver.getPort())); + }; + + { + INFO("read straight from the socket"); + send(1, 1); + read_until(system, 1); + REQUIRE(system.mPacketsIn == 1); + CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, 1) == 1); + } + { + INFO("taken into the queue and then read"); + send(2, 1); + queue_until(system, 1); + REQUIRE(system.getNumBufferedPackets() == 1); + CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, 2) == 1); + read_queued(system); + REQUIRE(system.mPacketsIn == 2); + CHECK(sDuplicateTestsHandled == 2); + CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, 2) == 1); + } + { + INFO("a copy of it taken into the queue: each copy acked once"); + send(2, 1); + queue_until(system, 1); + REQUIRE(system.getNumBufferedPackets() == 1); + read_queued(system); + REQUIRE(system.mPacketsIn == 3); + CHECK(sDuplicateTestsHandled == 2); + CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, 2) == 2); + } + { + INFO("a refused packet that waited in the queue was acked as it came in"); + send(3, 2); + queue_until(system, 1); + REQUIRE(system.getNumBufferedPackets() == 1); + read_queued(system); + REQUIRE(system.mInvalidOnCircuitPackets == 1); + CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, 3) == 1); + } + { + INFO("one read straight from the socket is not acked"); + send(4, 2); + read_until_refused(system, 2); + REQUIRE(system.mInvalidOnCircuitPackets == 2); + CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, 4) == 0); + } + CHECK(ll_test::CircuitProbe::acksWaiting(*circuit) == 4); + } } From 7832a4d50fd337b6e1b1bd3ab4485c85851f5be7 Mon Sep 17 00:00:00 2001 From: Rye Date: Mon, 2 Nov 2026 16:44:17 -0500 Subject: [PATCH 02/10] LLUrlRegistry finds the link after one that comes straight after @ findUrl took a link straight after @ for an email with an empty username (MAINT-5371) and answered that the text held no link at all. A text widget, which goes on from each link findUrl finds, stopped there, so "@http://a.example and http://b.example" linked neither. findUrl now searches the text after such a link as a text of its own, as the widget searches the text after a link, and answers the first link it finds there, at its place in the whole text. Co-Authored-By: Claude Opus 5.5 --- indra/llui/llurlregistry.cpp | 38 +++++++++++++++++++------ indra/llui/llurlregistry.h | 5 ++++ indra/llui/tests/llurlregistry_test.cpp | 17 +++++++++++ 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/indra/llui/llurlregistry.cpp b/indra/llui/llurlregistry.cpp index 4d799cfb3e7..7927ea0c1c2 100644 --- a/indra/llui/llurlregistry.cpp +++ b/indra/llui/llurlregistry.cpp @@ -181,7 +181,8 @@ static bool matchRegex(const std::string& text, const LLUrlEntryBase& entry, U32 return true; } -bool LLUrlRegistry::findUrl(const std::string &text, LLUrlMatch &match, const LLUrlLabelCallback &cb, bool is_content_trusted, bool skip_non_mentions) +LLUrlEntryBase* LLUrlRegistry::firstUrlEntry(const std::string& text, bool is_content_trusted, bool skip_non_mentions, + U32& match_start, U32& match_end) { // which entries' patterns match anywhere in the text, in one pass, // where the set can say: the others cannot match, and are passed over, @@ -192,13 +193,14 @@ bool LLUrlRegistry::findUrl(const std::string &text, LLUrlMatch &match, const LL { if (could_match.empty() && mUrlSetHasAll) { - return false; + return nullptr; } std::sort(could_match.begin(), could_match.end()); } // find the first matching regex from all url entries in the registry - U32 match_start = 0, match_end = 0; + match_start = 0; + match_end = 0; LLUrlEntryBase *match_entry = NULL; std::vector::iterator it; @@ -253,17 +255,35 @@ bool LLUrlRegistry::findUrl(const std::string &text, LLUrlMatch &match, const LL } } + return match_entry; +} + +bool LLUrlRegistry::findUrl(const std::string &text, LLUrlMatch &match, const LLUrlLabelCallback &cb, bool is_content_trusted, bool skip_non_mentions) +{ + U32 match_start = 0, match_end = 0; + LLUrlEntryBase* match_entry = firstUrlEntry(text, is_content_trusted, skip_non_mentions, match_start, match_end); + + // A link straight after @ is an email with an empty username, and no + // link (see MAINT-5371). The text after it is searched as a text of its + // own, as a text widget searches the text after a link it found. + std::string rest; + const std::string* searched = &text; + size_t offset = 0; + while (match_entry && match_start > 0 && (*searched)[match_start - 1] == '@') + { + offset += match_end + 1; + rest = text.substr(offset); + searched = &rest; + match_entry = firstUrlEntry(rest, is_content_trusted, skip_non_mentions, match_start, match_end); + } + // did we find a match? if so, return its details in the match object if (match_entry) { - // Skip if link is an email with an empty username (starting with @). See MAINT-5371. - if (match_start > 0 && text.substr(match_start - 1, 1) == "@") - return false; - // fill in the LLUrlMatch object and return it - std::string url = text.substr(match_start, match_end - match_start + 1); + std::string url = searched->substr(match_start, match_end - match_start + 1); - match.setValues(match_start, match_end, + match.setValues(static_cast(offset) + match_start, static_cast(offset) + match_end, match_entry->getUrl(url), match_entry->getLabel(url, cb), match_entry->getQuery(url), diff --git a/indra/llui/llurlregistry.h b/indra/llui/llurlregistry.h index 72a9f38584a..e90c6dd78a1 100644 --- a/indra/llui/llurlregistry.h +++ b/indra/llui/llurlregistry.h @@ -94,6 +94,11 @@ class LLUrlRegistry : public LLSingleton // a Url, which reads each pattern as its entry does void buildUrlSet(); + // The entry whose Url comes first in the text, and where it starts and + // ends; null where none has one there + LLUrlEntryBase* firstUrlEntry(const std::string& text, bool is_content_trusted, bool skip_non_mentions, + U32& match_start, U32& match_end); + std::vector mUrlEntry; ALRegexSet mUrlSet; // Each entry's index in the set, in the order of mUrlEntry; -1 for one diff --git a/indra/llui/tests/llurlregistry_test.cpp b/indra/llui/tests/llurlregistry_test.cpp index 05991af4647..11205fce512 100644 --- a/indra/llui/tests/llurlregistry_test.cpp +++ b/indra/llui/tests/llurlregistry_test.cpp @@ -85,4 +85,21 @@ namespace CHECK(found("say hello ok") == std::string("hello")); CHECK(found("say hello ok") == std::string("hello")); } + + // A link straight after @ is an email with no username, and no link; a + // text widget goes on from what findUrl finds, so one that finds nothing + // there linked nothing after it either. + TEST_CASE_METHOD(llurlregistry_data, "llurlregistry: findUrl finds the link after one that comes straight after an at", "[llurlregistry]") + { + if (!ui.ok()) + { + SKIP("no UI: LLUI_TEST_APP_DIR does not point at the source tree"); + } + CHECK(found("@http://a.example.com and http://b.example.com") == std::string("http://b.example.com")); + UNSCOPED_INFO("after two"); + CHECK(found("@http://a.example.com @http://b.example.com/x see http://c.example.com/y") == std::string("http://c.example.com/y")); + UNSCOPED_INFO("and none where there is no other"); + CHECK(found("@http://a.example.com alone") == std::string("")); + CHECK(found("x@http://a.example.com") == std::string("")); + } } From 0c3405f940d8a481daa440e1d3f990c43c3732ac Mon Sep 17 00:00:00 2001 From: Rye Date: Mon, 2 Nov 2026 16:44:19 -0500 Subject: [PATCH 03/10] LLSDMessageReader sizes a URI as its text getSize answered sizeof(LLURI) for a URI in a variable field, the size of the class's five strings (160 bytes in a 64-bit build of MSVC's or libstdc++'s library, 120 of libc++'s), so a handler that reads as many bytes as getSize says got a size that has nothing to do with the URI. It now answers the length of the URI's text, which getString reads. Co-Authored-By: Claude Opus 5.5 --- indra/llmessage/llsdmessagereader.cpp | 3 ++- indra/llmessage/tests/llsdmessagereader_test.cpp | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/indra/llmessage/llsdmessagereader.cpp b/indra/llmessage/llsdmessagereader.cpp index 6ade7c0dad9..4db64ba14c9 100644 --- a/indra/llmessage/llsdmessagereader.cpp +++ b/indra/llmessage/llsdmessagereader.cpp @@ -282,7 +282,8 @@ S32 getElementSize(const LLSD& llsd) case LLSD::TypeDate: return sizeof(LLDate); case LLSD::TypeURI: - return sizeof(LLURI); + // its text, as getString() reads it + return static_cast(llsd.asString().size()); case LLSD::TypeBinary: { std::vector data = llsd; diff --git a/indra/llmessage/tests/llsdmessagereader_test.cpp b/indra/llmessage/tests/llsdmessagereader_test.cpp index 2490bfd265f..71b66bf2b74 100644 --- a/indra/llmessage/tests/llsdmessagereader_test.cpp +++ b/indra/llmessage/tests/llsdmessagereader_test.cpp @@ -211,6 +211,21 @@ namespace outValue = buffer; CHECK(outValue == inValue); } + // A handler reads a variable field by asking its size and reading as + // many bytes. + TEST_CASE_METHOD(LLSDMessageReaderTestData, "LLSDMessageReader: a URI in a variable field is sized and read as its text", "[LLSDMessageReader]") + { + const std::string text = "https://example.com/cap/1234?x=1"; + LLSDMessageReader msg = testType(LLURI(text)); + + const S32 size = msg.getSize("block", "var"); + CHECK(size == (S32)text.size()); + CHECK(msg.getSize("block", 0, "var") == size); + + std::vector buffer(size + 1, 'x'); + msg.getString("block", "var", size + 1, buffer.data()); + CHECK(std::string(buffer.data()) == text); + } TEST_CASE_METHOD(LLSDMessageReaderTestData, "LLSDMessageReader: getVector3 reads back an LLVector3", "[LLSDMessageReader]") { LLVector3 outValue, inValue = LLVector3(1,2,3); From 9c1699dd2e921f8fb2673b2be8a3dcea089b9e9b Mon Sep 17 00:00:00 2001 From: Rye Date: Mon, 2 Nov 2026 16:46:07 -0500 Subject: [PATCH 04/10] LLSLURL keeps an app SLURL's query escaped, so it is unescaped once An app SLURL's query was unescaped twice: LLSLURL kept it unescaped whole, and the dispatcher's LLURI::queryMap unescaped each key and value of that again. A value holding an escaped & was split in two there, and %2525 read as %. LLSLURL now keeps the query as the link writes it, so the dispatcher's map is the link's, and the SLURL's string gives the link back rather than its query unescaped. Co-Authored-By: Claude Opus 5.5 --- indra/newview/llslurl.cpp | 5 +++-- indra/newview/llslurl.h | 1 + indra/newview/tests/llslurl_test.cpp | 30 ++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/indra/newview/llslurl.cpp b/indra/newview/llslurl.cpp index e273d058eb2..17e288ad91f 100644 --- a/indra/newview/llslurl.cpp +++ b/indra/newview/llslurl.cpp @@ -293,8 +293,9 @@ LLSLURL::LLSLURL(const std::string& slurl) // Grab the parameters mAppPath = path_array; - // and the query - mAppQuery = slurl_uri.query(); + // and the query, as the link writes it: the dispatcher reads its + // keys and values out of it, unescaping each once + mAppQuery = slurl_uri.escapedQuery(); mAppQueryMap = slurl_uri.queryMap(); return; } diff --git a/indra/newview/llslurl.h b/indra/newview/llslurl.h index c7419778da6..6c512ff1c2f 100644 --- a/indra/newview/llslurl.h +++ b/indra/newview/llslurl.h @@ -82,6 +82,7 @@ class LLSLURL std::string getRegion() const { return mRegion; } LLVector3 getPosition() const { return mPosition; } std::string getAppCmd() const { return mAppCmd; } + // an app SLURL's query as the link writes it, escaped std::string getAppQuery() const { return mAppQuery; } LLSD getAppQueryMap() const { return mAppQueryMap; } LLSD getAppPath() const { return mAppPath; } diff --git a/indra/newview/tests/llslurl_test.cpp b/indra/newview/tests/llslurl_test.cpp index ac2fcd736e7..c432ddd6432 100644 --- a/indra/newview/tests/llslurl_test.cpp +++ b/indra/newview/tests/llslurl_test.cpp @@ -31,6 +31,7 @@ #include "../llslurl.h" #include "../../llxml/llcontrol.h" #include "llsdserialize.h" +#include "lluri.h" namespace { @@ -462,6 +463,35 @@ namespace CHECK(slurl.getType() == LLSLURL::LOCATION); } + // The dispatcher reads an app SLURL's keys and values out of its query, + // as LLURI::queryMap reads them, unescaping each. + TEST_CASE_METHOD(slurlTest, "LLSlurl: an app SLURL's query is unescaped once", "[LLSlurl]") + { + llofstream gridfile(TEST_FILENAME); + gridfile << gSampleGridFile; + gridfile.close(); + + LLGridManager::getInstance()->initialize(TEST_FILENAME); + + LLGridManager::getInstance()->setGridChoice("util.agni.lindenlab.com"); + const LLSLURL slurl("secondlife:///app/foo/bar?name=a%26b%20c&pct=%2525"); + REQUIRE(slurl.getType() == LLSLURL::APP); + CHECK(slurl.getAppQuery() == "name=a%26b%20c&pct=%2525"); + + const LLSD query = LLURI::queryMap(slurl.getAppQuery()); + CHECK(query.size() == 2); + CHECK(query["name"].asString() == "a&b c"); + CHECK(query["pct"].asString() == "%25"); + CHECK(slurl.getAppQueryMap()["name"].asString() == "a&b c"); + CHECK(slurl.getAppQueryMap()["pct"].asString() == "%25"); + + UNSCOPED_INFO("and its string has the query as the link wrote it"); + const std::string written = slurl.getSLURLString(); + const std::string::size_type path = written.find("/foo/bar"); + REQUIRE(path != std::string::npos); + CHECK(written.substr(path) == "/foo/bar?name=a%26b%20c&pct=%2525"); + } + TEST_CASE_METHOD(slurlTest, "LLSlurl: construction from grid/region/vector combos", "[LLSlurl]") { llofstream gridfile(TEST_FILENAME); From 53654b47811edd09884c0a9e967a7d71256da93e Mon Sep 17 00:00:00 2001 From: Rye Date: Mon, 2 Nov 2026 16:51:50 -0500 Subject: [PATCH 05/10] openfloater checks the floater it opens against its lists LLFloaterOpenHandler checked a link that is not trusted against its lists of floaters such a link may not open by the name as the link writes it, escaped, and opened the name unescaped, so openfloater/upload%5Fimage clicked in chat opened a floater the list refuses. Both now read the name through ALAppLinks, which unescapes it once: the lists are held to the floater the handler would open. ALAppLinks is where the app link handlers' reading of a link goes, so that a test can reach it without the handlers' floaters and singletons. Co-Authored-By: Claude Opus 5.5 --- indra/newview/CMakeLists.txt | 4 ++ indra/newview/alapplinks.cpp | 43 +++++++++++++++++ indra/newview/alapplinks.h | 49 ++++++++++++++++++++ indra/newview/llviewerfloaterreg.cpp | 9 ++-- indra/newview/tests/alapplinks_test.cpp | 61 +++++++++++++++++++++++++ 5 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 indra/newview/alapplinks.cpp create mode 100644 indra/newview/alapplinks.h create mode 100644 indra/newview/tests/alapplinks_test.cpp diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index d060cc1c5b1..8f50c9784d5 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -132,6 +132,7 @@ if(AL_BUILD_VIEWER) ao.cpp aoengine.cpp aoset.cpp + alapplinks.cpp alassetblocklist.cpp alavataractions.cpp alavatargroups.cpp @@ -1006,6 +1007,7 @@ if(AL_BUILD_VIEWER) ao.h aoengine.h aoset.h + alapplinks.h alassetblocklist.h alavataractions.h alavatargroups.h @@ -2531,6 +2533,7 @@ if(AL_BUILD_TESTS) PROJECT ${test_project} SOURCES tests/alagentlocationformat_test.cpp + tests/alapplinks_test.cpp tests/albakeproxies_test.cpp tests/alchattimestamp_test.cpp tests/alcolorwheelmodel_test.cpp @@ -2609,6 +2612,7 @@ if(AL_BUILD_TESTS) tests/noise_test.cpp UNIT alagentlocationformat.cpp + alapplinks.cpp albakeproxies.cpp alchattimestamp.cpp alcolorwheelmodel.cpp diff --git a/indra/newview/alapplinks.cpp b/indra/newview/alapplinks.cpp new file mode 100644 index 00000000000..d6910091bd0 --- /dev/null +++ b/indra/newview/alapplinks.cpp @@ -0,0 +1,43 @@ +/** + * @file alapplinks.cpp + * @brief What the viewer's app link handlers read from a link's parts + * + * $LicenseInfo:firstyear=2026&license=viewerlgpl$ + * Alchemy Viewer Source Code + * Copyright (C) 2026, Rye + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; + * version 2.1 of the License only. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * $/LicenseInfo$ + */ + +#include "linden_common.h" + +#include "alapplinks.h" + +#include "lluri.h" + +#include + +//static +std::string ALAppLinks::floaterName(const LLSD& params) +{ + return LLURI::unescape(params[0].asString()); +} + +//static +bool ALAppLinks::floaterNameIn(const LLSD& params, const std::list& floater_names) +{ + return std::find(floater_names.begin(), floater_names.end(), floaterName(params)) != floater_names.end(); +} diff --git a/indra/newview/alapplinks.h b/indra/newview/alapplinks.h new file mode 100644 index 00000000000..e3b77cd294e --- /dev/null +++ b/indra/newview/alapplinks.h @@ -0,0 +1,49 @@ +/** + * @file alapplinks.h + * @brief What the viewer's app link handlers read from a link's parts + * + * $LicenseInfo:firstyear=2026&license=viewerlgpl$ + * Alchemy Viewer Source Code + * Copyright (C) 2026, Rye + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; + * version 2.1 of the License only. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * $/LicenseInfo$ + */ + +#ifndef AL_ALAPPLINKS_H +#define AL_ALAPPLINKS_H + +#include "llsd.h" + +#include +#include + +// What the handlers of secondlife:///app links read from a link, apart from +// what they do with it. A handler is given the link's path after the +// command as the link writes it, each part escaped, and its query's keys +// and values unescaped. +class ALAppLinks +{ +public: + // The floater an openfloater link names, unescaped, as the handler opens + // it. + static std::string floaterName(const LLSD& params); + + // Whether the floater an openfloater link would open is one of + // `floater_names`. + static bool floaterNameIn(const LLSD& params, const std::list& floater_names); +}; + +#endif // AL_ALAPPLINKS_H diff --git a/indra/newview/llviewerfloaterreg.cpp b/indra/newview/llviewerfloaterreg.cpp index 91dfebec909..e2bb2cd5adb 100644 --- a/indra/newview/llviewerfloaterreg.cpp +++ b/indra/newview/llviewerfloaterreg.cpp @@ -32,6 +32,7 @@ #include "llviewerfloaterreg.h" #include "ao.h" +#include "alapplinks.h" #include "alchatbar.h" // [SL:KB] - Patch: World-Derender | Checked: Catznip-3.2 #include "alfloaterblocked.h" @@ -232,8 +233,6 @@ class LLFloaterOpenHandler : public LLCommandHandler return true; // will fail silently } - std::string fl_name = params[0].asString(); - // External browsers explicitly ask user about opening links // so treat "external" same as "clicked" in this case, // despite it being treated as untrusted. @@ -264,7 +263,7 @@ class LLFloaterOpenHandler : public LLCommandHandler "upload_sound", "bulk_upload" }; - return std::find(blacklist_clicked.begin(), blacklist_clicked.end(), fl_name) == blacklist_clicked.end(); + return !ALAppLinks::floaterNameIn(params, blacklist_clicked); } else { @@ -314,7 +313,7 @@ class LLFloaterOpenHandler : public LLCommandHandler "bulk_upload", "slapp_test" }; - return std::find(blacklist_untrusted.begin(), blacklist_untrusted.end(), fl_name) == blacklist_untrusted.end(); + return !ALAppLinks::floaterNameIn(params, blacklist_untrusted); } @@ -332,7 +331,7 @@ class LLFloaterOpenHandler : public LLCommandHandler return false; } - const std::string floater_name = LLURI::unescape(params[0].asString()); + const std::string floater_name = ALAppLinks::floaterName(params); LLSD key = query_map; if (floater_name == FLOATER_PROFILE) { diff --git a/indra/newview/tests/alapplinks_test.cpp b/indra/newview/tests/alapplinks_test.cpp new file mode 100644 index 00000000000..c1ba91655de --- /dev/null +++ b/indra/newview/tests/alapplinks_test.cpp @@ -0,0 +1,61 @@ +/** + * @file alapplinks_test.cpp + * @brief Tests for what the viewer's app link handlers read from a link + * + * $LicenseInfo:firstyear=2026&license=viewerlgpl$ + * Alchemy Viewer Source Code + * Copyright (C) 2026, Rye + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; + * version 2.1 of the License only. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * $/LicenseInfo$ + */ + +#include "linden_common.h" + +#include "../alapplinks.h" + +#include + +#include + +namespace +{ + // A link's path after its command, as the dispatcher hands it over. + LLSD path(std::initializer_list parts) + { + LLSD params = LLSD::emptyArray(); + for (const char* part : parts) + { + params.append(part); + } + return params; + } + + // The handler opens the floater a link names unescaped, so a list of + // floaters a link may not open is held to that name, however the link + // escapes it. + TEST_CASE("ALAppLinks: an openfloater link is checked against the floater it opens", "[ALAppLinks]") + { + const std::list refused = { "upload_image", "god_tools" }; + + CHECK(ALAppLinks::floaterName(path({ "upload%5Fimage" })) == "upload_image"); + CHECK(ALAppLinks::floaterNameIn(path({ "upload%5Fimage" }), refused)); + CHECK(ALAppLinks::floaterNameIn(path({ "%67od_tools" }), refused)); + CHECK(ALAppLinks::floaterNameIn(path({ "god_tools" }), refused)); + CHECK_FALSE(ALAppLinks::floaterNameIn(path({ "preferences" }), refused)); + UNSCOPED_INFO("an escape that opens no floater on the list"); + CHECK_FALSE(ALAppLinks::floaterNameIn(path({ "upload%2520image" }), refused)); + } +} From d44be85c6699973a0814d19264b0005549b9de36 Mon Sep 17 00:00:00 2001 From: Rye Date: Mon, 2 Nov 2026 16:53:33 -0500 Subject: [PATCH 06/10] unblock links read the object's name unescaped, as block links do The agent and object handlers unescaped a block link's object name but read an unblock link's as the link writes it, though LLUrlAction escapes both. A mute is found by its name, so an object whose name has a space in it was never unblocked by its link. Both verbs now read the name through ALAppLinks::objectMute, which unescapes it. Co-Authored-By: Claude Opus 5.5 --- indra/newview/alapplinks.cpp | 11 ++++++++ indra/newview/alapplinks.h | 13 ++++++++++ indra/newview/llagenthandler.cpp | 30 ++++++++++------------ indra/newview/llchatitemscontainerctrl.cpp | 30 ++++++++++------------ indra/newview/tests/alapplinks_test.cpp | 22 ++++++++++++++++ 5 files changed, 73 insertions(+), 33 deletions(-) diff --git a/indra/newview/alapplinks.cpp b/indra/newview/alapplinks.cpp index d6910091bd0..d71d1ed9ac2 100644 --- a/indra/newview/alapplinks.cpp +++ b/indra/newview/alapplinks.cpp @@ -41,3 +41,14 @@ bool ALAppLinks::floaterNameIn(const LLSD& params, const std::list& { return std::find(floater_names.begin(), floater_names.end(), floaterName(params)) != floater_names.end(); } + +//static +std::optional ALAppLinks::objectMute(const LLSD& params) +{ + const std::string verb = params[1].asString(); + if ((verb != "block" && verb != "unblock") || params.size() <= 2) + { + return std::nullopt; + } + return ObjectMute{ verb == "block", LLURI::unescape(params[2].asString()) }; +} diff --git a/indra/newview/alapplinks.h b/indra/newview/alapplinks.h index e3b77cd294e..b8df0d02b15 100644 --- a/indra/newview/alapplinks.h +++ b/indra/newview/alapplinks.h @@ -28,6 +28,7 @@ #include "llsd.h" #include +#include #include // What the handlers of secondlife:///app links read from a link, apart from @@ -44,6 +45,18 @@ class ALAppLinks // Whether the floater an openfloater link would open is one of // `floater_names`. static bool floaterNameIn(const LLSD& params, const std::list& floater_names); + + // What an agent or object link's block or unblock asks for: the link's + // second part is the verb and its third the object's name, which both + // verbs' links escape. + struct ObjectMute + { + bool mBlock = false; + std::string mName; + }; + + // Nothing for another verb, or for a block or unblock with no name. + static std::optional objectMute(const LLSD& params); }; #endif // AL_ALAPPLINKS_H diff --git a/indra/newview/llagenthandler.cpp b/indra/newview/llagenthandler.cpp index 75dd1425627..188f02f4e04 100644 --- a/indra/newview/llagenthandler.cpp +++ b/indra/newview/llagenthandler.cpp @@ -26,6 +26,7 @@ #include "llviewerprecompiledheaders.h" +#include "alapplinks.h" #include "alfloaterblocked.h" #include "llavataractions.h" #include "llcommandhandler.h" @@ -147,25 +148,20 @@ class LLAgentHandler : public LLCommandHandler return true; } - if (verb == "block") + if (verb == "block" || verb == "unblock") { - if (params.size() > 2) + if (const std::optional request = ALAppLinks::objectMute(params)) { - const std::string object_name = LLURI::unescape(params[2].asString()); - LLMute mute(avatar_id, object_name, LLMute::OBJECT); - LLMuteList::getInstance()->add(mute); - ALFloaterBlocked::showMuteAndSelect(mute.mID); - } - return true; - } - - if (verb == "unblock") - { - if (params.size() > 2) - { - const std::string object_name = params[2].asString(); - LLMute mute(avatar_id, object_name, LLMute::OBJECT); - LLMuteList::getInstance()->remove(mute); + LLMute mute(avatar_id, request->mName, LLMute::OBJECT); + if (request->mBlock) + { + LLMuteList::getInstance()->add(mute); + ALFloaterBlocked::showMuteAndSelect(mute.mID); + } + else + { + LLMuteList::getInstance()->remove(mute); + } } return true; } diff --git a/indra/newview/llchatitemscontainerctrl.cpp b/indra/newview/llchatitemscontainerctrl.cpp index 0ead73e79c4..5730fb899ea 100644 --- a/indra/newview/llchatitemscontainerctrl.cpp +++ b/indra/newview/llchatitemscontainerctrl.cpp @@ -31,6 +31,8 @@ #include "llchatmsgbox.h" #include "lltextbox.h" +#include "alapplinks.h" + #include "llavatariconctrl.h" #include "llcommandhandler.h" #include "llfloaterreg.h" @@ -102,24 +104,20 @@ class LLObjectHandler : public LLCommandHandler } return true; } - if (verb == "block") - { - if (params.size() > 2) - { - const std::string object_name = LLURI::unescape(params[2].asString()); - LLMute mute(object_id, object_name, LLMute::OBJECT); - LLMuteList::getInstance()->add(mute); - ALFloaterBlocked::showMuteAndSelect(mute.mID); - } - return true; - } - if (verb == "unblock") + if (verb == "block" || verb == "unblock") { - if (params.size() > 2) + if (const std::optional request = ALAppLinks::objectMute(params)) { - const std::string object_name = params[2].asString(); - LLMute mute(object_id, object_name, LLMute::OBJECT); - LLMuteList::getInstance()->remove(mute); + LLMute mute(object_id, request->mName, LLMute::OBJECT); + if (request->mBlock) + { + LLMuteList::getInstance()->add(mute); + ALFloaterBlocked::showMuteAndSelect(mute.mID); + } + else + { + LLMuteList::getInstance()->remove(mute); + } } return true; } diff --git a/indra/newview/tests/alapplinks_test.cpp b/indra/newview/tests/alapplinks_test.cpp index c1ba91655de..a8da85f0080 100644 --- a/indra/newview/tests/alapplinks_test.cpp +++ b/indra/newview/tests/alapplinks_test.cpp @@ -58,4 +58,26 @@ namespace UNSCOPED_INFO("an escape that opens no floater on the list"); CHECK_FALSE(ALAppLinks::floaterNameIn(path({ "upload%2520image" }), refused)); } + + // The viewer's block and unblock links both escape the object's name, + // as LLUrlAction writes them, and a mute is found by its name, so + // unblock has to read the name block added. + TEST_CASE("ALAppLinks: block and unblock read the same object name", "[ALAppLinks]") + { + const char* const id = "c3c1b8f4-1b54-4b05-9c47-8b7a3fe2a3f1"; + + const std::optional block = ALAppLinks::objectMute(path({ id, "block", "Some%20Object%26Co" })); + REQUIRE(block.has_value()); + CHECK(block->mBlock); + CHECK(block->mName == "Some Object&Co"); + + const std::optional unblock = ALAppLinks::objectMute(path({ id, "unblock", "Some%20Object%26Co" })); + REQUIRE(unblock.has_value()); + CHECK_FALSE(unblock->mBlock); + CHECK(unblock->mName == "Some Object&Co"); + + UNSCOPED_INFO("nothing without a name or for another verb"); + CHECK_FALSE(ALAppLinks::objectMute(path({ id, "unblock" })).has_value()); + CHECK_FALSE(ALAppLinks::objectMute(path({ id, "inspect", "Some%20Object" })).has_value()); + } } From 39677191adcde161dc4b0f511960d75c9e360042 Mon Sep 17 00:00:00 2001 From: Rye Date: Mon, 2 Nov 2026 16:54:59 -0500 Subject: [PATCH 07/10] A teleport link with no coordinates goes to its region's centre LLTeleportHandler read a link's x, y and z from its three parts after the region, whether the link had them or not, so teleport/ went to the region's corner, (0, 0, 0), where a link with more than three coordinates went to its centre. A coordinate the link leaves out is now the centre's, 128, 128 and 0, as the handler's teleport event reads them, so a link with only x goes to (x, 128, 0) rather than (x, 0, 0). Links with two or three coordinates, and with more than three, go where they did. ALAppLinks::teleportPosition reads them. Co-Authored-By: Claude Opus 5.5 --- indra/newview/alapplinks.cpp | 14 ++++++++++++++ indra/newview/alapplinks.h | 7 +++++++ indra/newview/llurldispatcher.cpp | 9 ++------- indra/newview/tests/alapplinks_test.cpp | 10 ++++++++++ 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/indra/newview/alapplinks.cpp b/indra/newview/alapplinks.cpp index d71d1ed9ac2..b3d6a30bc41 100644 --- a/indra/newview/alapplinks.cpp +++ b/indra/newview/alapplinks.cpp @@ -52,3 +52,17 @@ std::optional ALAppLinks::objectMute(const LLSD& params) } return ObjectMute{ verb == "block", LLURI::unescape(params[2].asString()) }; } + +//static +LLVector3 ALAppLinks::teleportPosition(const LLSD& tokens) +{ + LLVector3 coords(128, 128, 0); + if (tokens.size() <= 4) + { + for (LLSD::Integer axis = 0; axis < 3 && axis + 1 < static_cast(tokens.size()); ++axis) + { + coords.mV[axis] = (F32)tokens[axis + 1].asReal(); + } + } + return coords; +} diff --git a/indra/newview/alapplinks.h b/indra/newview/alapplinks.h index b8df0d02b15..04224c9bb72 100644 --- a/indra/newview/alapplinks.h +++ b/indra/newview/alapplinks.h @@ -26,6 +26,7 @@ #define AL_ALAPPLINKS_H #include "llsd.h" +#include "v3math.h" #include #include @@ -57,6 +58,12 @@ class ALAppLinks // Nothing for another verb, or for a block or unblock with no name. static std::optional objectMute(const LLSD& params); + + // Where in its region a teleport link goes: its first part is the region + // and the three after it x, y and z. A coordinate the link leaves out is + // the region's centre's, as the teleport event's are, and a link with + // more than three goes to the centre. + static LLVector3 teleportPosition(const LLSD& tokens); }; #endif // AL_ALAPPLINKS_H diff --git a/indra/newview/llurldispatcher.cpp b/indra/newview/llurldispatcher.cpp index 39a9f0f8bc8..d35c16701fd 100644 --- a/indra/newview/llurldispatcher.cpp +++ b/indra/newview/llurldispatcher.cpp @@ -28,6 +28,7 @@ #include "llurldispatcher.h" // viewer includes +#include "alapplinks.h" #include "llagent.h" // teleportViaLocation() #include "llcommandhandler.h" #include "llfloaterhelpbrowser.h" @@ -303,13 +304,7 @@ class LLTeleportHandler : public LLCommandHandler, public LLEventAPI // a global position, and teleport to it if (tokens.size() < 1) return false; - LLVector3 coords(128, 128, 0); - if (tokens.size() <= 4) - { - coords = LLVector3((F32)tokens[1].asReal(), - (F32)tokens[2].asReal(), - (F32)tokens[3].asReal()); - } + const LLVector3 coords = ALAppLinks::teleportPosition(tokens); // Region names may be %20 escaped. std::string region_name = LLURI::unescape(tokens[0]); diff --git a/indra/newview/tests/alapplinks_test.cpp b/indra/newview/tests/alapplinks_test.cpp index a8da85f0080..c481e09453a 100644 --- a/indra/newview/tests/alapplinks_test.cpp +++ b/indra/newview/tests/alapplinks_test.cpp @@ -80,4 +80,14 @@ namespace CHECK_FALSE(ALAppLinks::objectMute(path({ id, "unblock" })).has_value()); CHECK_FALSE(ALAppLinks::objectMute(path({ id, "inspect", "Some%20Object" })).has_value()); } + + TEST_CASE("ALAppLinks: a teleport link goes to its region's centre where it gives no position", "[ALAppLinks]") + { + CHECK(ALAppLinks::teleportPosition(path({ "Ahern" })) == LLVector3(128, 128, 0)); + CHECK(ALAppLinks::teleportPosition(path({ "Ahern", "50" })) == LLVector3(50, 128, 0)); + CHECK(ALAppLinks::teleportPosition(path({ "Ahern", "50", "60" })) == LLVector3(50, 60, 0)); + CHECK(ALAppLinks::teleportPosition(path({ "Ahern", "50", "60", "70" })) == LLVector3(50, 60, 70)); + UNSCOPED_INFO("more than three coordinates"); + CHECK(ALAppLinks::teleportPosition(path({ "Ahern", "50", "60", "70", "80" })) == LLVector3(128, 128, 0)); + } } From d8bacaba309e5ca928600bf411ca7e5aaf9a5e12 Mon Sep 17 00:00:00 2001 From: Rye Date: Mon, 2 Nov 2026 16:56:53 -0500 Subject: [PATCH 08/10] An object chat link's rlv_shownames reads false as false The viewer writes rlv_shownames into an object chat link from a boolean, and LLObjectIMHandler handed the query's value, a string, on to the remote object inspector, which read it as a boolean: any string that is not empty, "false" and "0" with them, read as true, and the inspector hid the names. The handler now reads the value through ALAppLinks::queryFlag, which reads "false", "0" and their kin as false and "true", "1" and theirs as true. Anything else reads as it did, so what the viewer writes, "true" or nothing, reads as before. Co-Authored-By: Claude Opus 5.5 --- indra/newview/alapplinks.cpp | 12 ++++++++++++ indra/newview/alapplinks.h | 7 +++++++ indra/newview/llchathistory.cpp | 3 ++- indra/newview/tests/alapplinks_test.cpp | 23 +++++++++++++++++++++++ 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/indra/newview/alapplinks.cpp b/indra/newview/alapplinks.cpp index b3d6a30bc41..9354b8dbf61 100644 --- a/indra/newview/alapplinks.cpp +++ b/indra/newview/alapplinks.cpp @@ -26,6 +26,7 @@ #include "alapplinks.h" +#include "llstring.h" #include "lluri.h" #include @@ -66,3 +67,14 @@ LLVector3 ALAppLinks::teleportPosition(const LLSD& tokens) } return coords; } + +//static +bool ALAppLinks::queryFlag(const LLSD& value) +{ + bool flag = false; + if (value.isString() && LLStringUtil::convertToBOOL(value.asString(), flag)) + { + return flag; + } + return value.asBoolean(); +} diff --git a/indra/newview/alapplinks.h b/indra/newview/alapplinks.h index 04224c9bb72..fa1a5e6350c 100644 --- a/indra/newview/alapplinks.h +++ b/indra/newview/alapplinks.h @@ -64,6 +64,13 @@ class ALAppLinks // the region's centre's, as the teleport event's are, and a link with // more than three goes to the centre. static LLVector3 teleportPosition(const LLSD& tokens); + + // A flag in a link's query, which the query holds as text: "false" and + // "0" and their kin read as false, "true" and "1" and theirs as true, + // and anything else as LLSD reads a string, true unless it is empty, so + // that a flag written from a boolean, "true" or empty, reads as that + // boolean. A key with no value reads as true. + static bool queryFlag(const LLSD& value); }; #endif // AL_ALAPPLINKS_H diff --git a/indra/newview/llchathistory.cpp b/indra/newview/llchathistory.cpp index 9c20f4f905c..bf276d79bee 100644 --- a/indra/newview/llchathistory.cpp +++ b/indra/newview/llchathistory.cpp @@ -33,6 +33,7 @@ #include "llavatarnamecache.h" #include "llinstantmessage.h" +#include "alapplinks.h" #include "alavataractions.h" #include "alchattimestamp.h" #include "alfloaterblocked.h" @@ -114,7 +115,7 @@ class LLObjectIMHandler : public LLCommandHandler payload["owner_id"] = query_map["owner"]; // [RLVa:KB] - Checked: 2010-11-02 (RLVa-1.2.2a) | Modified: RLVa-1.2.2a if (query_map.has("rlv_shownames")) - payload["rlv_shownames"] = query_map["rlv_shownames"]; + payload["rlv_shownames"] = ALAppLinks::queryFlag(query_map["rlv_shownames"]); // [/RLVa:KB] payload["name"] = query_map["name"]; payload["slurl"] = LLWeb::escapeURL(query_map["slurl"]); diff --git a/indra/newview/tests/alapplinks_test.cpp b/indra/newview/tests/alapplinks_test.cpp index c481e09453a..6f59d17145b 100644 --- a/indra/newview/tests/alapplinks_test.cpp +++ b/indra/newview/tests/alapplinks_test.cpp @@ -26,6 +26,8 @@ #include "../alapplinks.h" +#include "lluri.h" + #include #include @@ -90,4 +92,25 @@ namespace UNSCOPED_INFO("more than three coordinates"); CHECK(ALAppLinks::teleportPosition(path({ "Ahern", "50", "60", "70", "80" })) == LLVector3(128, 128, 0)); } + + // The viewer writes rlv_shownames into an object chat link from a + // boolean, and the handler hands on what the link's query holds. + TEST_CASE("ALAppLinks: a flag in a link's query reads as the boolean it says", "[ALAppLinks]") + { + LLSD written; + written["hide"] = true; + written["show"] = false; + const std::string query = LLURI::mapToQueryString(written); + REQUIRE(!query.empty()); + const LLSD read = LLURI::queryMap(query.substr(1)); + CHECK(ALAppLinks::queryFlag(read["hide"])); + CHECK_FALSE(ALAppLinks::queryFlag(read["show"])); + + UNSCOPED_INFO("a link that says false"); + CHECK_FALSE(ALAppLinks::queryFlag(LLURI::queryMap("hide=false")["hide"])); + CHECK_FALSE(ALAppLinks::queryFlag(LLURI::queryMap("hide=0")["hide"])); + CHECK(ALAppLinks::queryFlag(LLURI::queryMap("hide=1")["hide"])); + UNSCOPED_INFO("a key with no value"); + CHECK(ALAppLinks::queryFlag(LLURI::queryMap("hide")["hide"])); + } } From 84a7c7fc7bfd6e298311d193598c15e214729199 Mon Sep 17 00:00:00 2001 From: Rye Date: Mon, 2 Nov 2026 17:28:55 -0500 Subject: [PATCH 09/10] A compressed update cut short after its extra parameters is refused A compressed update whose extra parameters run past its end has been refused since 9330f85903, but the fields after them were read on unchecked, each failed read leaving its value as it was: in LLViewerObject the attached sound and the name values; in LLVOVolume the volume's parameters, read from locals the packer never set and then applied, the texture entry, whose failure flushed the region's cache, and the texture animation, whose result was ignored and whose block was read from a buffer nothing had filled where it named sixteen bytes and the update held fewer. Each group is now read whole before any of it is applied, by ALCompressedSoundAndNames and ALCompressedVolume, and an update cut short in one is refused as one cut short in its extra parameters is: the object is dropped and asked for whole again. An update that ends before the end of its texture entry still flushes the region's cache, the sign of a corrupt cache the viewer has taken it for. The extended particle system after those fields is read as before, so one cut short reads as no system. LLTextureAnim::unpackTAMessage now says whether its block was there, and LLPrimitive::unpackTEEntry applies a texture entry read whole. The two handlers have no test; the readers they now call are tested. Co-Authored-By: Claude Opus 5.5 --- indra/llprimitive/CMakeLists.txt | 3 + indra/llprimitive/alcompressedupdate.cpp | 78 ++++++ indra/llprimitive/alcompressedupdate.h | 74 ++++++ indra/llprimitive/llprimitive.cpp | 9 +- indra/llprimitive/llprimitive.h | 11 + indra/llprimitive/lltextureanim.cpp | 13 +- indra/llprimitive/lltextureanim.h | 5 +- indra/llprimitive/llvolumemessage.h | 4 + .../tests/alcompressedupdate_test.cpp | 246 ++++++++++++++++++ indra/newview/llviewerobject.cpp | 27 +- indra/newview/llvovolume.cpp | 72 ++--- 11 files changed, 494 insertions(+), 48 deletions(-) create mode 100644 indra/llprimitive/alcompressedupdate.cpp create mode 100644 indra/llprimitive/alcompressedupdate.h create mode 100644 indra/llprimitive/tests/alcompressedupdate_test.cpp diff --git a/indra/llprimitive/CMakeLists.txt b/indra/llprimitive/CMakeLists.txt index 2ac7b51bd35..c2245beddfc 100644 --- a/indra/llprimitive/CMakeLists.txt +++ b/indra/llprimitive/CMakeLists.txt @@ -5,6 +5,7 @@ add_library(llprimitive STATIC) target_sources( llprimitive PRIVATE + alcompressedupdate.cpp alobjectmotion.cpp lldaeloader.cpp llgltfmaterial.cpp @@ -24,6 +25,7 @@ target_sources( PUBLIC FILE_SET HEADERS FILES + alcompressedupdate.h alobjectmotion.h lldaeloader.h llgltfmaterial.h @@ -65,6 +67,7 @@ if(AL_BUILD_TESTS) llprimitive_tests PROJECT llprimitive SOURCES + tests/alcompressedupdate_test.cpp tests/alobjectmotion_test.cpp tests/llgltfmaterial_test.cpp tests/llmaterial_test.cpp diff --git a/indra/llprimitive/alcompressedupdate.cpp b/indra/llprimitive/alcompressedupdate.cpp new file mode 100644 index 00000000000..2768c928019 --- /dev/null +++ b/indra/llprimitive/alcompressedupdate.cpp @@ -0,0 +1,78 @@ +/** + * @file alcompressedupdate.cpp + * @brief The fields of a compressed object update after its extra parameters + * + * $LicenseInfo:firstyear=2026&license=viewerlgpl$ + * Alchemy Viewer Source Code + * Copyright (C) 2026, Rye + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; + * version 2.1 of the License only. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * $/LicenseInfo$ + */ + +#include "linden_common.h" + +#include "alcompressedupdate.h" + +#include "lldatapacker.h" +#include "llvolumemessage.h" + +bool ALCompressedSoundAndNames::unpack(LLDataPacker& dp, U32 flags) +{ + *this = ALCompressedSoundAndNames(); + if (flags & 0x10) + { + if (!dp.unpackUUID(mSoundID, "SoundUUID") + || !dp.unpackF32(mSoundGain, "SoundGain") + || !dp.unpackU8(mSoundFlags, "SoundFlags") + || !dp.unpackF32(mSoundRadius, "SoundRadius")) + { + return false; + } + } + if (flags & 0x100) + { + if (!dp.unpackString(mNameValues, "NV")) + { + return false; + } + } + return true; +} + +ALCompressedVolume::ECut ALCompressedVolume::unpack(LLDataPacker& dp, U32 flags) +{ + // A volume's parameters take a run of bytes of one size: it is read + // whole, and the parameters unpacked from what was read. + U8 params[LLVolumeMessage::PACKED_VOLUME_PARAMS_SIZE]; + if (!dp.unpackBinaryDataFixed(params, sizeof(params), "VolumeParams")) + { + return ECut::VOLUME_PARAMS; + } + LLDataPackerBinaryBuffer params_dp(params, sizeof(params)); + mVolumeParamsInRange = LLVolumeMessage::unpackVolumeParams(&mVolumeParams, params_dp); + + if (!dp.unpackBinaryData(mTextureEntry, "TextureEntry")) + { + return ECut::TEXTURE_ENTRY; + } + + mHasTextureAnim = (flags & 0x40) != 0; + if (mHasTextureAnim && !mTextureAnim.unpackTAMessage(dp)) + { + return ECut::TEXTURE_ANIM; + } + return ECut::NONE; +} diff --git a/indra/llprimitive/alcompressedupdate.h b/indra/llprimitive/alcompressedupdate.h new file mode 100644 index 00000000000..5a32daaffd2 --- /dev/null +++ b/indra/llprimitive/alcompressedupdate.h @@ -0,0 +1,74 @@ +/** + * @file alcompressedupdate.h + * @brief The fields of a compressed object update after its extra parameters + * + * $LicenseInfo:firstyear=2026&license=viewerlgpl$ + * Alchemy Viewer Source Code + * Copyright (C) 2026, Rye + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; + * version 2.1 of the License only. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * $/LicenseInfo$ + */ + +#pragma once + +#include "llprimitive.h" +#include "lltextureanim.h" +#include "lluuid.h" +#include "llvolume.h" + +#include + +class LLDataPacker; + +// The fields of a compressed object update after its extra parameters, each +// group read whole before any of it is applied. The fields sit in one stream, +// so an update cut short in one holds nothing after it to read, and is +// refused, as one cut short in its extra parameters is. + +// The fields there that every object's update holds: its attached sound, +// under the update's 0x10, and its name values, under 0x100. Where the +// update holds neither, they are as here: no sound, and no name values. +struct ALCompressedSoundAndNames +{ + // False where one runs past the end of the update. + bool unpack(LLDataPacker& dp, U32 flags); + + LLUUID mSoundID; + F32 mSoundGain = 0.f; + U8 mSoundFlags = 0; + F32 mSoundRadius = 0.f; + std::string mNameValues; +}; + +// What a volume's update holds after those: the volume's parameters, its +// texture entry, and its texture animation, under the update's 0x40. The +// particle system after them, under 0x400, is not one of them: one cut short +// reads as no system. +struct ALCompressedVolume +{ + // Where the fields run past the end of the update: nowhere, or in the + // first of them that does. + enum class ECut { NONE, VOLUME_PARAMS, TEXTURE_ENTRY, TEXTURE_ANIM }; + ECut unpack(LLDataPacker& dp, U32 flags); + + LLVolumeParams mVolumeParams; + // Whether the parameters were in range, as unpackVolumeParams says. + bool mVolumeParamsInRange = false; + // As LLPrimitive::unpackTEEntry applies it. + LLPrimitive::te_bytes_t mTextureEntry; + bool mHasTextureAnim = false; + LLTextureAnim mTextureAnim; +}; diff --git a/indra/llprimitive/llprimitive.cpp b/indra/llprimitive/llprimitive.cpp index f588508dbe3..c4b4447a231 100644 --- a/indra/llprimitive/llprimitive.cpp +++ b/indra/llprimitive/llprimitive.cpp @@ -1307,11 +1307,6 @@ S32 LLPrimitive::parseTEMessage(LLMessageSystem* mesgsys, char const* block_name namespace { - // Room for a texture entry as long as the packers write for the most - // faces they pack, and the zero the parse puts after it. - constexpr size_t TE_INLINE_BYTES = 4608; - typedef boost::container::small_vector te_bytes_t; - // Parses a packed TextureEntry field of any length into tec, for // face_count faces. S32 parse_te_entry(boost::container::small_vector_base& packed, LLTEContents& tec, U8 face_count) @@ -1460,7 +1455,11 @@ S32 LLPrimitive::unpackTEMessage(LLDataPacker &dp) LL_WARNS() << "Bad texture entry block! Abort!" << LL_ENDL; return TEM_INVALID; } + return unpackTEEntry(packed); +} +S32 LLPrimitive::unpackTEEntry(boost::container::small_vector_base& packed) +{ LLTEContents tec; if (!parse_te_entry(packed, tec, getNumTEs())) { diff --git a/indra/llprimitive/llprimitive.h b/indra/llprimitive/llprimitive.h index 47ff1c47f83..a1f79e0e559 100644 --- a/indra/llprimitive/llprimitive.h +++ b/indra/llprimitive/llprimitive.h @@ -543,6 +543,17 @@ class LLPrimitive : public LLXform // takes the bytes its data holds whatever the size before it names. Empty // for neither, or where the data is cut short. static void unpackCompressedData(LLDataPacker& dp, U32 flags, boost::container::small_vector_base& data); + + // A texture entry as a packer or a TextureEntry field holds it, with room + // in place for as long as the packers write for the most faces they pack, + // and the zero the parse puts after it. + static constexpr size_t TE_INLINE_BYTES = 4608; + typedef boost::container::small_vector te_bytes_t; + + // Applies a texture entry read whole from a packer, without the size + // before it, as unpackTEMessage applies one it reads. + S32 unpackTEEntry(boost::container::small_vector_base& packed); + S32 applyParsedTEMessage(LLTEContents& tec); #ifdef CHECK_FOR_FINITE diff --git a/indra/llprimitive/lltextureanim.cpp b/indra/llprimitive/lltextureanim.cpp index 963b7456b87..fef2d303135 100644 --- a/indra/llprimitive/lltextureanim.cpp +++ b/indra/llprimitive/lltextureanim.cpp @@ -151,11 +151,15 @@ void LLTextureAnim::unpackTAMessage(LLMessageSystem *mesgsys, const S32 block_nu htolememcpy(&mRate, data + 12, MVT_F32, sizeof(F32)); } -void LLTextureAnim::unpackTAMessage(LLDataPacker &dp) +bool LLTextureAnim::unpackTAMessage(LLDataPacker &dp) { - S32 size; + S32 size = 0; U8 data[TA_BLOCK_SIZE]; - dp.unpackBinaryData(data, TA_BLOCK_SIZE, size, "TextureAnimation"); + if (!dp.unpackBinaryData(data, TA_BLOCK_SIZE, size, "TextureAnimation")) + { + mMode = 0; + return false; + } if (size != TA_BLOCK_SIZE) { if (size) @@ -163,7 +167,7 @@ void LLTextureAnim::unpackTAMessage(LLDataPacker &dp) LL_WARNS() << "Bad size " << size << " for TA block, ignoring." << LL_ENDL; } mMode = 0; - return; + return true; } mMode = data[0]; @@ -173,6 +177,7 @@ void LLTextureAnim::unpackTAMessage(LLDataPacker &dp) htolememcpy(&mStart, data + 4, MVT_F32, sizeof(F32)); htolememcpy(&mLength, data + 8, MVT_F32, sizeof(F32)); htolememcpy(&mRate, data + 12, MVT_F32, sizeof(F32)); + return true; } LLSD LLTextureAnim::asLLSD() const diff --git a/indra/llprimitive/lltextureanim.h b/indra/llprimitive/lltextureanim.h index 2d101530430..ff1e90a948b 100644 --- a/indra/llprimitive/lltextureanim.h +++ b/indra/llprimitive/lltextureanim.h @@ -43,7 +43,10 @@ class LLTextureAnim void packTAMessage(LLMessageSystem *mesgsys) const; void packTAMessage(LLDataPacker &dp) const; void unpackTAMessage(LLMessageSystem *mesgsys, const S32 block_num); - void unpackTAMessage(LLDataPacker &dp); + // False where the block runs past the end of the data, which leaves the + // animation off. A block of another size than an animation's is read + // past, and leaves it off too. + bool unpackTAMessage(LLDataPacker &dp); bool equals(const LLTextureAnim &other) const; LLSD asLLSD() const; operator LLSD() const { return asLLSD(); } diff --git a/indra/llprimitive/llvolumemessage.h b/indra/llprimitive/llvolumemessage.h index 609608023ad..4fe427ed57b 100644 --- a/indra/llprimitive/llvolumemessage.h +++ b/indra/llprimitive/llvolumemessage.h @@ -65,6 +65,10 @@ class LLVolumeMessage static bool unpackPathParams(LLPathParams* params, LLDataPacker& dp); public: + // The bytes packVolumeParams writes to a binary packer: a path's sixteen, + // then a profile's seven. + static constexpr S32 PACKED_VOLUME_PARAMS_SIZE = 23; + /** * @brief This method constrains any volume params to make them valid. * diff --git a/indra/llprimitive/tests/alcompressedupdate_test.cpp b/indra/llprimitive/tests/alcompressedupdate_test.cpp new file mode 100644 index 00000000000..4083119914b --- /dev/null +++ b/indra/llprimitive/tests/alcompressedupdate_test.cpp @@ -0,0 +1,246 @@ +/** + * @file alcompressedupdate_test.cpp + * @brief Tests for the fields of a compressed object update after its extra parameters + * + * $LicenseInfo:firstyear=2026&license=viewerlgpl$ + * Alchemy Viewer Source Code + * Copyright (C) 2026, Rye + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; + * version 2.1 of the License only. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * $/LicenseInfo$ + */ + +#include "linden_common.h" + +#include "../alcompressedupdate.h" +#include "../llvolumemessage.h" + +#include "lldatapacker.h" + +#include + +#include + +namespace +{ + // A compressed update's sound and name values, as a simulator packs them + // where flags hold them. + std::vector pack_sound_and_names(const ALCompressedSoundAndNames& fields, U32 flags) + { + std::vector buffer(256); + LLDataPackerBinaryBuffer dp(buffer.data(), (S32)buffer.size()); + if (flags & 0x10) + { + REQUIRE(dp.packUUID(fields.mSoundID, "SoundUUID")); + REQUIRE(dp.packF32(fields.mSoundGain, "SoundGain")); + REQUIRE(dp.packU8(fields.mSoundFlags, "SoundFlags")); + REQUIRE(dp.packF32(fields.mSoundRadius, "SoundRadius")); + } + if (flags & 0x100) + { + REQUIRE(dp.packString(fields.mNameValues, "NV")); + } + buffer.resize(dp.getCurrentSize()); + return buffer; + } + + ALCompressedSoundAndNames some_sound_and_names() + { + ALCompressedSoundAndNames fields; + fields.mSoundID.generate(); + fields.mSoundGain = 0.5f; + fields.mSoundFlags = 3; + fields.mSoundRadius = 20.f; + fields.mNameValues = "AttachItemID STRING RW SV c3c1b8f4-1b54-4b05-9c47-8b7a3fe2a3f1"; + return fields; + } + + TEST_CASE("ALCompressedSoundAndNames: an update's sound and name values are read whole", "[ALCompressedSoundAndNames]") + { + const ALCompressedSoundAndNames sent = some_sound_and_names(); + std::vector buffer = pack_sound_and_names(sent, 0x110); + + LLDataPackerBinaryBuffer dp(buffer.data(), (S32)buffer.size()); + ALCompressedSoundAndNames read; + REQUIRE(read.unpack(dp, 0x110)); + CHECK(read.mSoundID == sent.mSoundID); + CHECK(read.mSoundGain == sent.mSoundGain); + CHECK(read.mSoundFlags == sent.mSoundFlags); + CHECK(read.mSoundRadius == sent.mSoundRadius); + CHECK(read.mNameValues == sent.mNameValues); + CHECK_FALSE(dp.hasNext()); + + UNSCOPED_INFO("an update that holds neither reads no sound and no name values"); + LLDataPackerBinaryBuffer neither(buffer.data(), (S32)buffer.size()); + REQUIRE(read.unpack(neither, 0)); + CHECK(read.mSoundID.isNull()); + CHECK(read.mSoundGain == 0.f); + CHECK(read.mNameValues.empty()); + CHECK(neither.getCurrentSize() == 0); + } + + TEST_CASE("ALCompressedSoundAndNames: an update cut short in its sound or its name values is refused", "[ALCompressedSoundAndNames]") + { + const ALCompressedSoundAndNames sent = some_sound_and_names(); + ALCompressedSoundAndNames read; + for (U32 flags : { 0x10u, 0x100u, 0x110u }) + { + std::vector buffer = pack_sound_and_names(sent, flags); + for (S32 length = 0; length < (S32)buffer.size(); ++length) + { + CAPTURE(flags, length); + LLDataPackerBinaryBuffer cut(buffer.data(), length); + CHECK_FALSE(read.unpack(cut, flags)); + } + } + } + + // A volume's fields of a compressed update as a simulator packs them, + // with a byte of the particle system that follows them after them. + struct PackedVolume + { + std::vector mBytes; + S32 mParamsEnd = 0; + S32 mTextureEntryEnd = 0; + S32 mTextureAnimEnd = 0; + }; + + PackedVolume pack_volume(const LLVolumeParams& params, const LLPrimitive& faces, const LLTextureAnim& anim) + { + PackedVolume packed; + packed.mBytes.resize(4096); + LLDataPackerBinaryBuffer dp(packed.mBytes.data(), (S32)packed.mBytes.size()); + REQUIRE(LLVolumeMessage::packVolumeParams(¶ms, dp)); + packed.mParamsEnd = dp.getCurrentSize(); + REQUIRE(faces.packTEMessage(dp)); + packed.mTextureEntryEnd = dp.getCurrentSize(); + anim.packTAMessage(dp); + packed.mTextureAnimEnd = dp.getCurrentSize(); + REQUIRE(dp.packU8(0xab, "PartSys")); + packed.mBytes.resize(dp.getCurrentSize()); + return packed; + } + + LLTextureAnim some_animation() + { + LLTextureAnim anim; + anim.mMode = LLTextureAnim::ON | LLTextureAnim::LOOP; + anim.mFace = 1; + anim.mSizeX = 4; + anim.mSizeY = 2; + anim.mStart = 0.f; + anim.mLength = 8.f; + anim.mRate = 8.f; + return anim; + } + + TEST_CASE("ALCompressedVolume: a volume's parameters take as many bytes as the update reads for them", "[ALCompressedVolume]") + { + LLVolumeParams params; + params.setHollow(0.5f); + LLDataPackerBinaryBuffer counter; + REQUIRE(LLVolumeMessage::packVolumeParams(¶ms, counter)); + CHECK(counter.getCurrentSize() == LLVolumeMessage::PACKED_VOLUME_PARAMS_SIZE); + } + + TEST_CASE("ALCompressedVolume: a volume's fields are read whole", "[ALCompressedVolume]") + { + LLVolumeParams params; + params.setHollow(0.5f); + LLPrimitive faces; + faces.setNumTEs(3); + LLUUID texture; + texture.generate(); + faces.setTETexture(1, texture); + const LLTextureAnim anim = some_animation(); + const PackedVolume packed = pack_volume(params, faces, anim); + + // What the parameters read as from the update alone. + std::vector bytes = packed.mBytes; + LLVolumeParams expected; + LLDataPackerBinaryBuffer params_dp(bytes.data(), packed.mParamsEnd); + const bool in_range = LLVolumeMessage::unpackVolumeParams(&expected, params_dp); + + LLDataPackerBinaryBuffer dp(bytes.data(), (S32)bytes.size()); + ALCompressedVolume read; + REQUIRE(read.unpack(dp, 0x40) == ALCompressedVolume::ECut::NONE); + CHECK(read.mVolumeParams == expected); + CHECK(read.mVolumeParamsInRange == in_range); + CHECK(read.mHasTextureAnim); + CHECK(read.mTextureAnim.equals(anim)); + UNSCOPED_INFO("the particle system after them is left to read"); + CHECK(dp.getCurrentSize() == packed.mTextureAnimEnd); + + LLPrimitive applied; + applied.setNumTEs(3); + applied.unpackTEEntry(read.mTextureEntry); + REQUIRE(applied.getTE(1) != nullptr); + CHECK(applied.getTE(1)->getID() == texture); + + UNSCOPED_INFO("an update with no animation reads none"); + LLDataPackerBinaryBuffer no_anim(bytes.data(), (S32)bytes.size()); + REQUIRE(read.unpack(no_anim, 0) == ALCompressedVolume::ECut::NONE); + CHECK_FALSE(read.mHasTextureAnim); + CHECK(no_anim.getCurrentSize() == packed.mTextureEntryEnd); + } + + // The viewer flushes the region's cache where an update ends before the + // end of its texture entry, and refuses the update wherever it ends. + TEST_CASE("ALCompressedVolume: an update cut short says which field it ends in", "[ALCompressedVolume]") + { + LLVolumeParams params; + LLPrimitive faces; + faces.setNumTEs(2); + const PackedVolume packed = pack_volume(params, faces, some_animation()); + std::vector bytes = packed.mBytes; + + ALCompressedVolume read; + for (S32 length = 0; length < packed.mTextureAnimEnd; ++length) + { + CAPTURE(length); + LLDataPackerBinaryBuffer cut(bytes.data(), length); + const ALCompressedVolume::ECut expected = length < packed.mParamsEnd ? ALCompressedVolume::ECut::VOLUME_PARAMS + : length < packed.mTextureEntryEnd ? ALCompressedVolume::ECut::TEXTURE_ENTRY + : ALCompressedVolume::ECut::TEXTURE_ANIM; + CHECK(read.unpack(cut, 0x40) == expected); + } + } + + TEST_CASE("LLTextureAnim: unpackTAMessage refuses a block past the end of the data", "[LLTextureAnim]") + { + const LLTextureAnim anim = some_animation(); + std::vector buffer(64); + LLDataPackerBinaryBuffer dp(buffer.data(), (S32)buffer.size()); + anim.packTAMessage(dp); + const S32 size = dp.getCurrentSize(); + + LLTextureAnim read; + read.mMode = LLTextureAnim::LOOP; + LLDataPackerBinaryBuffer cut(buffer.data(), size - 1); + CHECK_FALSE(read.unpackTAMessage(cut)); + CHECK(read.mMode == 0); + + LLDataPackerBinaryBuffer whole(buffer.data(), size); + CHECK(read.unpackTAMessage(whole)); + CHECK(read.equals(anim)); + + UNSCOPED_INFO("a block of another size is read past and leaves the animation off"); + U8 other[] = { 4, 0, 0, 0, 1, 2, 3, 4 }; + LLDataPackerBinaryBuffer other_dp(other, sizeof(other)); + CHECK(read.unpackTAMessage(other_dp)); + CHECK(read.mMode == 0); + CHECK_FALSE(other_dp.hasNext()); + } +} diff --git a/indra/newview/llviewerobject.cpp b/indra/newview/llviewerobject.cpp index 3488f38d8a4..57d3e5bcbcc 100644 --- a/indra/newview/llviewerobject.cpp +++ b/indra/newview/llviewerobject.cpp @@ -29,6 +29,8 @@ #include "llviewerobject.h" #include "llscripteditorws.h" +#include "alcompressedupdate.h" + #include "llaudioengine.h" #include "indra_constants.h" #include "llmath.h" @@ -1886,6 +1888,18 @@ U32 LLViewerObject::processUpdateMessage(LLMessageSystem *mesgsys, return retval; } + // So is one whose sound or name values run past its end, + // before any of what follows the entries is applied. + ALCompressedSoundAndNames sound_and_names; + if (!sound_and_names.unpack(*dp, value)) + { + LL_WARNS("UpdateFail") << "Sound or name values past the end of the compressed update for " << getID() + << ", refusing the update" << LL_ENDL; + mRegionp->addCacheMissFull(getLocalID()); + gObjectList.killObject(this); + return retval; + } + // Mark all extra parameters not used for (auto& entry : mExtraParameterList) { @@ -1909,18 +1923,15 @@ U32 LLViewerObject::processUpdateMessage(LLMessageSystem *mesgsys, if (value & 0x10) { - dp->unpackUUID(sound_uuid, "SoundUUID"); - dp->unpackF32(gain, "SoundGain"); - dp->unpackU8(sound_flags, "SoundFlags"); - dp->unpackF32(cutoff, "SoundRadius"); + sound_uuid = sound_and_names.mSoundID; + gain = sound_and_names.mSoundGain; + sound_flags = sound_and_names.mSoundFlags; + cutoff = sound_and_names.mSoundRadius; } if (value & 0x100) { - std::string name_value_list; - dp->unpackString(name_value_list, "NV"); - - setNameValueList(name_value_list); + setNameValueList(sound_and_names.mNameValues); } mTotalCRC = crc; diff --git a/indra/newview/llvovolume.cpp b/indra/newview/llvovolume.cpp index 3c70bee5c78..5ac98499dc8 100644 --- a/indra/newview/llvovolume.cpp +++ b/indra/newview/llvovolume.cpp @@ -34,6 +34,7 @@ #include #include +#include "alcompressedupdate.h" #include "llviewercontrol.h" #include "lldir.h" #include "llimagepng.h" @@ -436,49 +437,60 @@ U32 LLVOVolume::processUpdateMessage(LLMessageSystem *mesgsys, { if (update_type != OUT_TERSE_IMPROVED) { - LLVolumeParams volume_params; - bool res = LLVolumeMessage::unpackVolumeParams(&volume_params, *dp); - if (!res) + U32 value = dp->getPassFlags(); + + // The volume's fields are read whole before any is applied. An + // update cut short in one of them holds nothing after it to read, + // so it is refused, as the base class refuses one cut short in + // its extra parameters: the object is dropped and asked for whole + // again. + ALCompressedVolume volume; + const ALCompressedVolume::ECut cut = volume.unpack(*dp, value); + if (cut != ALCompressedVolume::ECut::NONE) + { + if (cut != ALCompressedVolume::ECut::TEXTURE_ANIM) + { + // An update that ends before the end of its texture + // entry is bogus data, which flushes the region's cache. + dp->dumpBufferToLog(); + LL_WARNS() << "Flushing cache files" << LL_ENDL; + + if(LLVOCache::instanceExists() && getRegion()) + { + LLVOCache::getInstance()->removeEntry(getRegion()->getHandle()) ; + } + } + LL_WARNS("UpdateFail") << (cut == ALCompressedVolume::ECut::VOLUME_PARAMS ? "Volume parameters" + : cut == ALCompressedVolume::ECut::TEXTURE_ENTRY ? "Texture entry" + : "Texture animation") + << " past the end of the compressed update for " << getID() << ", refusing the update" + << LL_ENDL; + mRegionp->addCacheMissFull(getLocalID()); + gObjectList.killObject(this); + return retval; + } + + if (!volume.mVolumeParamsInRange) { LL_WARNS() << "Bogus volume parameters in object " << getID() << LL_ENDL; LL_WARNS() << getRegion()->getOriginGlobal() << LL_ENDL; } - volume_params.setSculptID(sculpt_id, sculpt_type); + volume.mVolumeParams.setSculptID(sculpt_id, sculpt_type); - if (setVolume(volume_params, 0)) + if (setVolume(volume.mVolumeParams, 0)) { markForUpdate(); } - S32 res2 = unpackTEMessage(*dp); - if (TEM_INVALID == res2) + S32 res2 = unpackTEEntry(volume.mTextureEntry); + if (res2 & TEM_CHANGE_MEDIA) { - // There's something bogus in the data that we're unpacking. - dp->dumpBufferToLog(); - LL_WARNS() << "Flushing cache files" << LL_ENDL; - - if(LLVOCache::instanceExists() && getRegion()) - { - LLVOCache::getInstance()->removeEntry(getRegion()->getHandle()) ; - } - - LL_WARNS() << "Bogus TE data in " << getID() << LL_ENDL; + retval |= MEDIA_FLAGS_CHANGED; } - else - { - if (res2 & TEM_CHANGE_MEDIA) - { - retval |= MEDIA_FLAGS_CHANGED; - } - } - - U32 value = dp->getPassFlags(); - if (value & 0x40) + if (volume.mHasTextureAnim) { - LLTextureAnim ta; - ta.unpackTAMessage(*dp); // always consume the block from the stream - applyTextureAnim(ta); + applyTextureAnim(volume.mTextureAnim); } else { From 87812d50ed9aaf03147828ff6fd2cfb3fc5c91a0 Mon Sep 17 00:00:00 2001 From: Rye Date: Tue, 3 Nov 2026 02:25:16 -0500 Subject: [PATCH 10/10] Tests cover a duplicate's ack, a larger animation block and an icon after @ A second copy of a reliable packet read straight from the socket is acked as it is read, since it was not acked as it came in; the ack test read no such copy, so it passed with that ack gone. A texture animation block larger than an animation's is read past and leaves the animation off, as a smaller one does; the test had only the smaller one. The text after a link straight after @ is searched as a text of its own, as a widget searches the text after a link it found, so a Hand before the link no longer keeps trusted icon tags after it unlinked, and one after it still does. Co-Authored-By: Claude Opus 5.5 --- indra/llmessage/tests/message_test.cpp | 14 +++++++++++--- .../llprimitive/tests/alcompressedupdate_test.cpp | 8 ++++++++ indra/llui/tests/llurlregistry_test.cpp | 12 ++++++++++-- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/indra/llmessage/tests/message_test.cpp b/indra/llmessage/tests/message_test.cpp index ce233038a87..fd889d9ba95 100644 --- a/indra/llmessage/tests/message_test.cpp +++ b/indra/llmessage/tests/message_test.cpp @@ -705,6 +705,14 @@ namespace REQUIRE(system.mPacketsIn == 1); CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, 1) == 1); } + { + INFO("a copy of it read straight from the socket is acked as it is read"); + send(1, 1); + read_until(system, 2); + REQUIRE(system.mPacketsIn == 2); + CHECK(sDuplicateTestsHandled == 1); + CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, 1) == 2); + } { INFO("taken into the queue and then read"); send(2, 1); @@ -712,7 +720,7 @@ namespace REQUIRE(system.getNumBufferedPackets() == 1); CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, 2) == 1); read_queued(system); - REQUIRE(system.mPacketsIn == 2); + REQUIRE(system.mPacketsIn == 3); CHECK(sDuplicateTestsHandled == 2); CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, 2) == 1); } @@ -722,7 +730,7 @@ namespace queue_until(system, 1); REQUIRE(system.getNumBufferedPackets() == 1); read_queued(system); - REQUIRE(system.mPacketsIn == 3); + REQUIRE(system.mPacketsIn == 4); CHECK(sDuplicateTestsHandled == 2); CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, 2) == 2); } @@ -742,7 +750,7 @@ namespace REQUIRE(system.mInvalidOnCircuitPackets == 2); CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, 4) == 0); } - CHECK(ll_test::CircuitProbe::acksWaiting(*circuit) == 4); + CHECK(ll_test::CircuitProbe::acksWaiting(*circuit) == 5); } } diff --git a/indra/llprimitive/tests/alcompressedupdate_test.cpp b/indra/llprimitive/tests/alcompressedupdate_test.cpp index 4083119914b..a8934c2ea2e 100644 --- a/indra/llprimitive/tests/alcompressedupdate_test.cpp +++ b/indra/llprimitive/tests/alcompressedupdate_test.cpp @@ -242,5 +242,13 @@ namespace CHECK(read.unpackTAMessage(other_dp)); CHECK(read.mMode == 0); CHECK_FALSE(other_dp.hasNext()); + + UNSCOPED_INFO("a larger one too"); + read.mMode = LLTextureAnim::LOOP; + U8 larger[4 + 17] = { 17, 0, 0, 0, LLTextureAnim::ON | LLTextureAnim::LOOP }; + LLDataPackerBinaryBuffer larger_dp(larger, sizeof(larger)); + CHECK(read.unpackTAMessage(larger_dp)); + CHECK(read.mMode == 0); + CHECK_FALSE(larger_dp.hasNext()); } } diff --git a/indra/llui/tests/llurlregistry_test.cpp b/indra/llui/tests/llurlregistry_test.cpp index 11205fce512..1f61bd47063 100644 --- a/indra/llui/tests/llurlregistry_test.cpp +++ b/indra/llui/tests/llurlregistry_test.cpp @@ -38,10 +38,10 @@ namespace ll_test::HeadlessUI& ui = ll_test::HeadlessUI::get(); // The first Url in the text, from where it starts to where it ends. - static std::string found(const char* text) + static std::string found(const char* text, bool is_content_trusted = false) { LLUrlMatch match; - if (!LLUrlRegistry::instance().findUrl(text, match)) + if (!LLUrlRegistry::instance().findUrl(text, match, &LLUrlRegistryNullCallback, is_content_trusted)) { return ""; } @@ -101,5 +101,13 @@ namespace UNSCOPED_INFO("and none where there is no other"); CHECK(found("@http://a.example.com alone") == std::string("")); CHECK(found("x@http://a.example.com") == std::string("")); + + // Trusted icon tags are not linked in a text with a Hand in it, and + // the text after the link is a text of its own, as after a link a + // widget found. + UNSCOPED_INFO("an icon after it in trusted text with a Hand before it"); + CHECK(found("Hand @http://a.example.com Inv_Object", true) == std::string("Inv_Object")); + UNSCOPED_INFO("and none with a Hand after it"); + CHECK(found("@http://a.example.com Hand Inv_Object", true) == std::string("")); } }