Address code review comments

- Use Reader to parse BDX URI
- Pass in server instance for CASE sessions and BDX transfer usage

examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp

@@ -33,6 +33,7 @@ using chip::ByteSpan;
 using chip::CharSpan;
 using chip::FabricIndex;
 using chip::FabricInfo;
+using chip::MutableCharSpan;
 using chip::NodeId;
 using chip::Optional;
 using chip::Server;
@@ -62,18 +63,18 @@ void GenerateUpdateToken(uint8_t * buf, size_t bufSize)
     }
 }
 
-bool GenerateBdxUri(NodeId nodeId, const Span<char> & fileDesignator, Span<char> outUri, size_t availableSize)
+bool GenerateBdxUri(NodeId nodeId, CharSpan fileDesignator, MutableCharSpan outUri)
 {
     static constexpr char bdxPrefix[] = "bdx://";
     size_t nodeIdHexStrLen            = sizeof(nodeId) * 2;
     size_t expectedLength             = strlen(bdxPrefix) + nodeIdHexStrLen + 1 + fileDesignator.size();
 
-    if (expectedLength >= availableSize)
+    if (expectedLength >= outUri.size())
     {
         return false;
     }
 
-    size_t written = static_cast<size_t>(snprintf(outUri.data(), availableSize, "%s" ChipLogFormatX64 "/%s", bdxPrefix,
+    size_t written = static_cast<size_t>(snprintf(outUri.data(), outUri.size(), "%s" ChipLogFormatX64 "/%s", bdxPrefix,
                                                   ChipLogValueX64(nodeId), fileDesignator.data()));
 
     return expectedLength == written;
@@ -126,8 +127,9 @@ EmberAfStatus OTAProviderExample::HandleQueryImage(chip::app::CommandHandler * c
         NodeId nodeId           = fabricInfo->GetPeerId().GetNodeId();
 
         // Only doing BDX transport for now
-        GenerateBdxUri(nodeId, Span<char>(mOTAFilePath, strlen(mOTAFilePath)), Span<char>(uriBuf, kUriMaxLen), kUriMaxLen);
-        ChipLogDetail(SoftwareUpdate, "generated URI: %.*s", static_cast<int>(strlen(uriBuf)), uriBuf);
+        MutableCharSpan uri(uriBuf, kUriMaxLen);
+        GenerateBdxUri(nodeId, CharSpan(mOTAFilePath, strlen(mOTAFilePath)), uri);
+        ChipLogDetail(SoftwareUpdate, "Generated URI: %.*s", static_cast<int>(uri.size()), uri.data());
     }
 
     // Set Status for the Query Image Response

examples/ota-requestor-app/linux/main.cpp

@@ -16,6 +16,8 @@
  *    limitations under the License.
  */
 
+#include <app/CASEClientPool.h>
+#include <app/OperationalDeviceProxyPool.h>
 #include <app/server/Server.h>
 #include <controller/ExampleOperationalCredentialsIssuer.h>
 #include <credentials/examples/DeviceAttestationCredsExample.h>
@@ -29,6 +31,7 @@
 
 using chip::BDXDownloader;
 using chip::ByteSpan;
+using chip::CASEClientPool;
 using chip::CharSpan;
 using chip::EndpointId;
 using chip::FabricIndex;
@@ -37,6 +40,7 @@ using chip::LinuxOTAImageProcessor;
 using chip::NodeId;
 using chip::OnDeviceConnected;
 using chip::OnDeviceConnectionFailure;
+using chip::OperationalDeviceProxyPool;
 using chip::OTADownloader;
 using chip::OTAImageProcessorParams;
 using chip::OTARequestor;
@@ -50,10 +54,15 @@ using namespace chip::ArgParser;
 using namespace chip::Messaging;
 using namespace chip::app::Clusters::OtaSoftwareUpdateProvider::Commands;
 
-OTARequestor requestorCore;
-LinuxOTARequestorDriver requestorUser;
-BDXDownloader downloader;
-LinuxOTAImageProcessor imageProcessor;
+constexpr size_t kMaxActiveCaseClients = 2;
+constexpr size_t kMaxActiveDevices     = 8;
+
+OTARequestor gRequestorCore;
+LinuxOTARequestorDriver gRequestorUser;
+BDXDownloader gDownloader;
+LinuxOTAImageProcessor gImageProcessor;
+CASEClientPool<kMaxActiveCaseClients> gCASEClientPool;
+OperationalDeviceProxyPool<kMaxActiveDevices> gDevicePool;
 
 bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier, const char * aName, const char * aValue);
 void OnStartDelayTimerHandler(Layer * systemLayer, void * appState);
@@ -191,31 +200,37 @@ int main(int argc, char * argv[])
     SetDeviceAttestationCredentialsProvider(chip::Credentials::Examples::GetExampleDACProvider());
 
     // Initialize and interconnect the Requestor and Image Processor objects -- START
-    SetRequestorInstance(&requestorCore);
+    SetRequestorInstance(&gRequestorCore);
+
+    // Set server instance used for session establishment
+    chip::Server * server = &(chip::Server::GetInstance());
+    server->SetCASEClientPool(&gCASEClientPool);
+    server->SetDevicePool(&gDevicePool);
+    gRequestorCore.SetServerInstance(server);
 
     // Connect the Requestor and Requestor Driver objects
-    requestorCore.SetOtaRequestorDriver(&requestorUser);
+    gRequestorCore.SetOtaRequestorDriver(&gRequestorUser);
 
     // WARNING: this is probably not realistic to know such details of the image or to even have an OTADownloader instantiated at
     // the beginning of program execution. We're using hardcoded values here for now since this is a reference application.
     // TODO: instatiate and initialize these values when QueryImageResponse tells us an image is available
     // TODO: add API for OTARequestor to pass QueryImageResponse info to the application to use for OTADownloader init
     OTAImageProcessorParams ipParams;
     ipParams.imageFile = CharSpan("test.txt");
-    imageProcessor.SetOTAImageProcessorParams(ipParams);
-    imageProcessor.SetOTADownloader(&downloader);
+    gImageProcessor.SetOTAImageProcessorParams(ipParams);
+    gImageProcessor.SetOTADownloader(&gDownloader);
 
     // Connect the Downloader and Image Processor objects
-    downloader.SetImageProcessorDelegate(&imageProcessor);
+    gDownloader.SetImageProcessorDelegate(&gImageProcessor);
 
-    requestorCore.SetBDXDownloader(&downloader);
+    gRequestorCore.SetBDXDownloader(&gDownloader);
     // Initialize and interconnect the Requestor and Image Processor objects -- END
 
     // Test Mode operation: If a delay is provided, QueryImage after the timer expires
     if (delayQueryTimeInSec > 0)
     {
         // In this mode Provider node ID and fabric idx must be supplied explicitly from program args
-        requestorCore.TestModeSetProviderParameters(providerNodeId, providerFabricIndex);
+        gRequestorCore.TestModeSetProviderParameters(providerNodeId, providerFabricIndex);
 
         chip::DeviceLayer::SystemLayer().StartTimer(chip::System::Clock::Milliseconds32(delayQueryTimeInSec * 1000),
                                                     OnStartDelayTimerHandler, nullptr);

src/app/clusters/ota-requestor/BDXDownloader.cpp

@@ -19,6 +19,8 @@
 #include "BDXDownloader.h"
 
 #include <lib/core/CHIPError.h>
+#include <lib/support/BufferReader.h>
+#include <lib/support/BytesToHex.h>
 #include <lib/support/CodeUtils.h>
 #include <protocols/bdx/BdxMessages.h>
 #include <system/SystemClock.h> /* TODO:(#12520) remove */
@@ -215,4 +217,63 @@ CHIP_ERROR BDXDownloader::HandleBdxEvent(const chip::bdx::TransferSession::Outpu
     return CHIP_NO_ERROR;
 }
 
+// TODO: Add unit tests for parsing BDX URI
+CHIP_ERROR BDXDownloader::ParseBdxUri(CharSpan uri, NodeId & nodeId, MutableCharSpan fileDesignator)
+{
+    // Check against minimum length of a valid BDX URI
+    if (uri.size() < kValidBdxUriMinLen)
+    {
+        return CHIP_ERROR_INVALID_STRING_LENGTH;
+    }
+
+    uint8_t readValue[kUriMaxLen];
+    Encoding::LittleEndian::Reader uriReader(reinterpret_cast<const uint8_t *>(uri.data()), uri.size());
+
+    // Check the scheme field matches the BDX prefix
+    memset(readValue, 0, sizeof(readValue));
+    ReturnErrorOnFailure(uriReader.ReadBytes(readValue, sizeof(bdxPrefix)).StatusCode());
+    ByteSpan expectedScheme(bdxPrefix, sizeof(bdxPrefix));
+    ByteSpan actualScheme(readValue, sizeof(bdxPrefix));
+    if (!expectedScheme.data_equal(actualScheme))
+    {
+        return CHIP_ERROR_INVALID_SCHEME_PREFIX;
+    }
+
+    // Extract the node ID from the authority field
+    memset(readValue, 0, sizeof(readValue));
+    ReturnErrorOnFailure(uriReader.ReadBytes(readValue, kNodeIdHexStringLen).StatusCode());
+    uint8_t buffer[kNodeIdHexStringLen];
+    if (Encoding::HexToBytes(reinterpret_cast<const char *>(readValue), kNodeIdHexStringLen, buffer, kNodeIdHexStringLen) == 0)
+    {
+        return CHIP_ERROR_INVALID_DESTINATION_NODE_ID;
+    }
+    nodeId = Encoding::BigEndian::Get64(buffer);
+    if (!IsOperationalNodeId(nodeId))
+    {
+        return CHIP_ERROR_INVALID_DESTINATION_NODE_ID;
+    }
+
+    // Verify the separator between authority and path fields
+    memset(readValue, 0, sizeof(readValue));
+    ReturnErrorOnFailure(uriReader.ReadBytes(readValue, sizeof(bdxSeparator)).StatusCode());
+    ByteSpan expectedSeparator(bdxSeparator, sizeof(bdxSeparator));
+    ByteSpan actualSeparator(readValue, sizeof(bdxSeparator));
+    if (!expectedSeparator.data_equal(actualSeparator))
+    {
+        return CHIP_ERROR_MISSING_URI_SEPARATOR;
+    }
+
+    // Extract file designator from the path field
+    size_t fileDesignatorLength = uriReader.Remaining();
+    memset(readValue, 0, sizeof(readValue));
+    ReturnErrorOnFailure(uriReader.ReadBytes(readValue, fileDesignatorLength).StatusCode());
+    size_t written = static_cast<size_t>(snprintf(fileDesignator.data(), fileDesignator.size(), "%s", readValue));
+    if (written != fileDesignatorLength)
+    {
+        return CHIP_ERROR_INTERNAL;
+    }
+
+    return CHIP_NO_ERROR;
+}
+
 } // namespace chip

src/app/clusters/ota-requestor/BDXDownloader.h

@@ -33,6 +33,13 @@
 
 namespace chip {
 
+// Constants for BDX URI parsing
+constexpr uint8_t bdxPrefix[]         = { 'b', 'd', 'x', ':', '/', '/' };
+constexpr uint8_t bdxSeparator[]      = { '/' };
+constexpr uint8_t kValidBdxUriMinLen  = 24;
+constexpr uint8_t kNodeIdHexStringLen = 16;
+constexpr size_t kUriMaxLen           = 256;
+
 class BDXDownloader : public chip::OTADownloader
 {
 public:
@@ -65,6 +72,11 @@ class BDXDownloader : public chip::OTADownloader
     CHIP_ERROR FetchNextData() override;
     // TODO: override SkipData
 
+    /**
+     * Validate the URI and parse the BDX URI for various fields
+     */
+    CHIP_ERROR ParseBdxUri(CharSpan uri, NodeId & nodeId, MutableCharSpan fileDesignator);
+
 private:
     void PollTransferSession();
     CHIP_ERROR HandleBdxEvent(const chip::bdx::TransferSession::OutputEvent & outEvent);