Просмотр исходного кода

[7] fix(protocol): decode every replacement an opcode-1901 batch carries

Review: opcode 1901 does not actually handle batches.
Resolution: the codec reads the count and decodes up to the twelve
replacements the native array holds.

The descriptor is a counted run of replacements followed by the one item
they apply to, and its count field is four bits wide. The codec required
that count to be exactly one and the payload to be exactly twenty-four
bytes, so a batch was not declined, it failed to parse as though it were
malformed.

Reads the count, decodes that many replacements, and lets the exact bit
length prove the payload rather than a fixed size. A replacement is 123
bits, so only a single one leaves the payload byte aligned; every other
count ends mid-byte and pads to the next boundary, which is why the
payload is now required to be spent to within a byte rather than
exactly.

Applying a batch is not attempted. A request prepares at most one State
mutation, and each becomes one queuez transaction and one Family-4 push,
so a run naming several sockets cannot be applied as the single
transaction it would have to be. A batch is therefore understood and
declined, with the count in the log and the refusal now carried in the
reply. Nothing in this build emits one.

Thomas Shields 3 недель назад
Родитель
Сommit
35127c3a60

+ 18 - 2
Sunrise/src/middleware/web_service/messages/opcode1901.h

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <array>
+#include <cstddef>
 #include <cstdint>
 
 #include "../web_service_envelope.h"
@@ -12,13 +14,27 @@ inline constexpr std::uint16_t kOpcode = 1901;
 /** The equipment selector carries only the low 62 bits of the item instance's identity. */
 inline constexpr std::uint64_t kInstanceIdentityMask = 0x3FFFFFFFFFFFFFFFULL;
 
-/** Exact logical fields carried by the native 192-bit equipped socket-action descriptor. */
-struct Request {
+/** The native replacement array reserves twelve entries, which its 4-bit count can address. */
+inline constexpr std::size_t kReplacementCapacity = 12;
+
+/** One socket replacement: which plug goes into which socket of the named item. */
+struct Replacement {
     std::uint16_t plugDefinitionIndex{};
     std::uint8_t canonicalSocketKind{};
     std::uint8_t modelSocketKind{};
     std::uint32_t socketIndex{};
     std::uint64_t auxiliary{};
+};
+
+/**
+ * Exact logical fields carried by the native equipped socket-action descriptor.
+ *
+ * The descriptor is a counted run of replacements followed by the one item they all apply to, so
+ * its width grows with the count rather than being fixed.
+ */
+struct Request {
+    std::array<Replacement, kReplacementCapacity> replacements{};
+    std::size_t replacementCount{};
     /** Selector exactly as the descriptor carries it, kept for the request trace. */
     std::uint64_t equipmentSelector{};
     /** The item-instance identity that selector encodes, already decoded. */

+ 57 - 34
Sunrise/src/middleware/web_service/messages/opcode1901_codec.cpp

@@ -6,12 +6,10 @@
 namespace sunrise::middleware::web_service::messages::opcode1901 {
 namespace {
 
-/** The reflected opcode-1901 request occupies exactly 192 bits. */
-constexpr std::size_t kPayloadSize = 24;
-/** The native fixed replacement array reserves twelve entries, so its count uses four bits. */
+/** The native replacement array reserves twelve entries, so its count uses four bits. */
 constexpr std::uint8_t kReplacementCountWidth = 4;
-/** This codec supports the single replacement emitted by one shader Apply action. */
-constexpr std::uint64_t kCanonicalReplacementCount = 1;
+/** A payload is whole bytes, so at most this many bits of it can be trailing pad. */
+constexpr std::size_t kBitsPerByte = 8;
 /** Signed native definition indices use one presence bit followed by fifteen value bits. */
 constexpr std::uint8_t kDefinitionIndexWidth = 15;
 /** The canonical socket-kind byte is signed and biased from INT8_MIN. */
@@ -45,38 +43,69 @@ bool identifies_instance(std::uint64_t instanceIdentityToken,
            && (instanceSoid & kInstanceIdentityMask) == instanceIdentityToken;
 }
 
-/** Parses the complete native equipped shader socket-action descriptor. */
+/** Parses the complete native equipped socket-action descriptor, batch or single. */
 bool parse_request(const Message& message, Request& request) noexcept {
     request = {};
-    if (message.opcode != kOpcode || message.payload.size() != kPayloadSize) {
+    if (message.opcode != kOpcode) {
         return false;
     }
 
     encoding::bits::Reader reader(message.payload);
     std::uint64_t replacementCount = 0;
-    std::uint64_t plugDefinitionPresent = 0;
-    std::uint64_t encodedPlugDefinition = 0;
-    std::uint64_t encodedCanonicalSocketKind = 0;
-    std::uint64_t modelSocketKind = 0;
-    std::uint64_t encodedSocketIndex = 0;
-    std::uint64_t auxiliaryPresent = 0;
+    if (!reader.read(kReplacementCountWidth, replacementCount) || replacementCount == 0
+        || replacementCount > kReplacementCapacity) {
+        return false;
+    }
+
+    // The run is counted rather than fixed, so its width follows the count. Reading exactly that
+    // many and then requiring the payload to be spent is what proves the descriptor's length.
+    for (std::uint64_t index = 0; index < replacementCount; ++index) {
+        std::uint64_t plugDefinitionPresent = 0;
+        std::uint64_t encodedPlugDefinition = 0;
+        std::uint64_t encodedCanonicalSocketKind = 0;
+        std::uint64_t modelSocketKind = 0;
+        std::uint64_t encodedSocketIndex = 0;
+        std::uint64_t auxiliaryPresent = 0;
+        std::uint64_t auxiliary = 0;
+        if (!reader.read(1, plugDefinitionPresent)
+            || !reader.read(kDefinitionIndexWidth, encodedPlugDefinition)
+            || !reader.read(kCanonicalSocketKindWidth, encodedCanonicalSocketKind)
+            || !reader.read(kModelSocketKindWidth, modelSocketKind)
+            || !reader.read(kSocketIndexWidth, encodedSocketIndex)
+            || !reader.read(1, auxiliaryPresent) || !reader.read(kOptionalIdentityWidth, auxiliary)
+            || plugDefinitionPresent == 0 || auxiliaryPresent == 0
+            || encodedCanonicalSocketKind < kCanonicalSocketKindBias
+            || modelSocketKind < kModelSocketKindBias || encodedSocketIndex < kSocketIndexBias
+            || modelSocketKind - kModelSocketKindBias != kShaderModelSocketKind
+            || auxiliary != kShaderAuxiliary) {
+            request = {};
+            return false;
+        }
+        Replacement& replacement = request.replacements[static_cast<std::size_t>(index)];
+        replacement.plugDefinitionIndex = static_cast<std::uint16_t>(encodedPlugDefinition);
+        replacement.canonicalSocketKind =
+            static_cast<std::uint8_t>(encodedCanonicalSocketKind - kCanonicalSocketKindBias);
+        replacement.modelSocketKind =
+            static_cast<std::uint8_t>(modelSocketKind - kModelSocketKindBias);
+        replacement.socketIndex = static_cast<std::uint32_t>(encodedSocketIndex - kSocketIndexBias);
+        replacement.auxiliary = auxiliary;
+        if (replacement.canonicalSocketKind != replacement.socketIndex) {
+            request = {};
+            return false;
+        }
+    }
+    request.replacementCount = static_cast<std::size_t>(replacementCount);
+
+    // A replacement is 123 bits, so only a single one leaves the payload byte aligned. Every
+    // other count ends mid-byte and the descriptor pads out to the next boundary, which is why
+    // the payload has to be spent to within a byte rather than exactly.
     std::uint64_t equipmentSelectorPresent = 0;
-    if (!reader.read(kReplacementCountWidth, replacementCount)
-        || !reader.read(1, plugDefinitionPresent)
-        || !reader.read(kDefinitionIndexWidth, encodedPlugDefinition)
-        || !reader.read(kCanonicalSocketKindWidth, encodedCanonicalSocketKind)
-        || !reader.read(kModelSocketKindWidth, modelSocketKind)
-        || !reader.read(kSocketIndexWidth, encodedSocketIndex) || !reader.read(1, auxiliaryPresent)
-        || !reader.read(kOptionalIdentityWidth, request.auxiliary)
-        || !reader.read(1, equipmentSelectorPresent)
+    if (!reader.read(1, equipmentSelectorPresent)
         || !reader.read(kOptionalIdentityWidth, request.equipmentSelector)
-        || reader.remaining_bits() != 0 || replacementCount != kCanonicalReplacementCount
-        || plugDefinitionPresent == 0 || auxiliaryPresent == 0 || equipmentSelectorPresent == 0
-        || encodedCanonicalSocketKind < kCanonicalSocketKindBias
-        || modelSocketKind < kModelSocketKindBias || encodedSocketIndex < kSocketIndexBias
-        || modelSocketKind - kModelSocketKindBias != kShaderModelSocketKind
-        || request.auxiliary != kShaderAuxiliary) {
+        || reader.remaining_bits() >= kBitsPerByte || equipmentSelectorPresent == 0) {
+        const std::uint64_t selector = request.equipmentSelector;
         request = {};
+        request.equipmentSelector = selector;
         return false;
     }
 
@@ -90,13 +119,7 @@ bool parse_request(const Message& message, Request& request) noexcept {
         return false;
     }
     request.instanceIdentityToken = request.equipmentSelector / kSelectorStride;
-
-    request.plugDefinitionIndex = static_cast<std::uint16_t>(encodedPlugDefinition);
-    request.canonicalSocketKind =
-        static_cast<std::uint8_t>(encodedCanonicalSocketKind - kCanonicalSocketKindBias);
-    request.modelSocketKind = static_cast<std::uint8_t>(modelSocketKind - kModelSocketKindBias);
-    request.socketIndex = static_cast<std::uint32_t>(encodedSocketIndex - kSocketIndexBias);
-    return request.canonicalSocketKind == request.socketIndex;
+    return true;
 }
 
 } // namespace sunrise::middleware::web_service::messages::opcode1901

+ 28 - 21
Sunrise/src/server/web_service/web_service_actions.cpp

@@ -380,26 +380,33 @@ void mutate_socket_plug(const middleware::web_service::Message& message,
 /** Parses and prepares one character-location opcode-1901 socket selection. */
 void mutate_equipped_socket_plug(const middleware::web_service::Message& message,
                                  Outcome& outcome) noexcept {
-    middleware::web_service::messages::opcode1901::Request request{};
-    if (!middleware::web_service::messages::opcode1901::parse_request(message, request)
-        || request.canonicalSocketKind != request.socketIndex
-        || request.modelSocketKind != kEquippedShaderModelSocketKind || request.auxiliary != 0
-        || request.socketIndex >= state::account::inventory::kPlugCapacity
+    namespace opcode1901 = middleware::web_service::messages::opcode1901;
+    opcode1901::Request request{};
+    const bool parsed = opcode1901::parse_request(message, request);
+    // A request prepares at most one State mutation, so a run naming several sockets cannot be
+    // applied as the one transaction it has to be. It is understood and declined rather than
+    // treated as malformed, and the reply now carries that refusal.
+    const opcode1901::Replacement& replacement = request.replacements.front();
+    if (!parsed || request.replacementCount != 1
+        || replacement.modelSocketKind != kEquippedShaderModelSocketKind
+        || replacement.auxiliary != 0
+        || replacement.socketIndex >= state::account::inventory::kPlugCapacity
         || request.instanceIdentityToken == 0) {
         std::array<char, 256> line{};
         const int count = std::snprintf(
             line.data(),
             line.size(),
-            "ev=ws1901 stage=parse result=fail transaction=%u payload_bytes=%zu "
+            "ev=ws1901 stage=parse result=fail transaction=%u payload_bytes=%zu replacements=%zu "
             "plug_definition=%u canonical_kind=%u model_kind=%u socket=%u auxiliary=0x%llX "
             "equipment_selector=%llu",
             static_cast<unsigned>(message.transactionId),
             message.payload.size(),
-            static_cast<unsigned>(request.plugDefinitionIndex),
-            static_cast<unsigned>(request.canonicalSocketKind),
-            static_cast<unsigned>(request.modelSocketKind),
-            request.socketIndex,
-            static_cast<unsigned long long>(request.auxiliary),
+            request.replacementCount,
+            static_cast<unsigned>(replacement.plugDefinitionIndex),
+            static_cast<unsigned>(replacement.canonicalSocketKind),
+            static_cast<unsigned>(replacement.modelSocketKind),
+            replacement.socketIndex,
+            static_cast<unsigned long long>(replacement.auxiliary),
             static_cast<unsigned long long>(request.equipmentSelector));
         if (count > 0) {
             core::log::write(core::log::Channel::server,
@@ -413,8 +420,8 @@ void mutate_equipped_socket_plug(const middleware::web_service::Message& message
     state::PendingSocketPlug mutation{};
     if (!state::prepare_character_selector_socket_plug(
             request.instanceIdentityToken,
-            static_cast<std::uint8_t>(request.socketIndex),
-            request.plugDefinitionIndex,
+            static_cast<std::uint8_t>(replacement.socketIndex),
+            replacement.plugDefinitionIndex,
             mutation)) {
         std::array<char, 224> line{};
         const int count = std::snprintf(
@@ -426,11 +433,11 @@ void mutate_equipped_socket_plug(const middleware::web_service::Message& message
             static_cast<unsigned>(message.transactionId),
             static_cast<unsigned long long>(request.equipmentSelector),
             static_cast<unsigned long long>(identityToken),
-            request.socketIndex,
-            static_cast<unsigned>(request.plugDefinitionIndex),
-            static_cast<unsigned>(request.canonicalSocketKind),
-            static_cast<unsigned>(request.modelSocketKind),
-            static_cast<unsigned long long>(request.auxiliary));
+            replacement.socketIndex,
+            static_cast<unsigned>(replacement.plugDefinitionIndex),
+            static_cast<unsigned>(replacement.canonicalSocketKind),
+            static_cast<unsigned>(replacement.modelSocketKind),
+            static_cast<unsigned long long>(replacement.auxiliary));
         if (count > 0) {
             core::log::write(core::log::Channel::server,
                              core::log::Level::warn,
@@ -458,9 +465,9 @@ void mutate_equipped_socket_plug(const middleware::web_service::Message& message
         static_cast<unsigned>(mutation.socketLane),
         static_cast<unsigned>(mutation.plugDefinitionIndex),
         static_cast<unsigned>(mutation.plugBucketId),
-        static_cast<unsigned>(request.canonicalSocketKind),
-        static_cast<unsigned>(request.modelSocketKind),
-        static_cast<unsigned long long>(request.auxiliary));
+        static_cast<unsigned>(replacement.canonicalSocketKind),
+        static_cast<unsigned>(replacement.modelSocketKind),
+        static_cast<unsigned long long>(replacement.auxiliary));
     if (count > 0) {
         core::log::write(core::log::Channel::server,
                          core::log::Level::info,