From b63d8971c6a133a663880fc59707c200d5ef0585 Mon Sep 17 00:00:00 2001
From: Carol Yang <clyang@apple.com>
Date: Wed, 13 Apr 2022 12:02:25 -0700
Subject: [PATCH] [OTA] Make ImageURI configurable for the OTA Provider app
 (#17222)

---
 .github/.wordlist.txt                         |  2 +
 examples/ota-provider-app/linux/README.md     |  1 +
 examples/ota-provider-app/linux/main.cpp      | 35 ++++++++-------
 .../OTAProviderExample.cpp                    | 43 ++++++++++++++-----
 .../ota-provider-common/OTAProviderExample.h  |  3 ++
 src/protocols/bdx/BdxUri.cpp                  |  7 ++-
 src/protocols/bdx/BdxUri.h                    |  9 ++++
 7 files changed, 71 insertions(+), 29 deletions(-)

diff --git a/.github/.wordlist.txt b/.github/.wordlist.txt
index 3eb6cef2f112c1..3938e78ea731fa 100644
--- a/.github/.wordlist.txt
+++ b/.github/.wordlist.txt
@@ -618,6 +618,8 @@ IlluminanceMeasurement
 IM
 imager
 imagetool
+ImageURI
+imageUri
 img
 Impl
 ImplClass
diff --git a/examples/ota-provider-app/linux/README.md b/examples/ota-provider-app/linux/README.md
index c993f77c1081e0..af21d26be9e773 100644
--- a/examples/ota-provider-app/linux/README.md
+++ b/examples/ota-provider-app/linux/README.md
@@ -18,6 +18,7 @@ scripts/examples/gn_build_example.sh examples/ota-provider-app/linux out/debug c
 | -a, --applyUpdateAction \<proceed \| awaitNextAction \| discontinue\>    | Value for the Action field in the first ApplyUpdateResponse.<br>For all subsequent responses, the value of proceed will be used.                                                                                                                                                                                                                                                                                                       |
 | -c, --userConsentNeeded                                                  | If supplied, value of the UserConsentNeeded field in the QueryImageResponse is set to true. This is only applicable if value of the RequestorCanConsent field in QueryImage Command is true.<br>Otherwise, value of the UserConsentNeeded field is false.                                                                                                                                                                              |
 | -f, --filepath \<file path\>                                             | Path to a file containing an OTA image                                                                                                                                                                                                                                                                                                                                                                                                 |
+| -i, --imageUri \<uri\>                                                   | Value for the ImageURI field in the QueryImageResponse. If none is supplied, a valid URI is generated.                                                                                                                                                                                                                                                                                                                                 |
 | -o, --otaImageList \<file path\>                                         | Path to a file containing a list of OTA images                                                                                                                                                                                                                                                                                                                                                                                         |
 | -p, --delayedApplyActionTimeSec \<time in seconds\>                      | Value for the DelayedActionTime field in the first ApplyUpdateResponse.<br>For all subsequent responses, the value of zero will be used.                                                                                                                                                                                                                                                                                               |
 | -q, --queryImageStatus \<updateAvailable \| busy \| updateNotAvailable\> | Value for the Status field in the first QueryImageResponse.<br>For all subsequent responses, the value of updateAvailable will be used.                                                                                                                                                                                                                                                                                                |
diff --git a/examples/ota-provider-app/linux/main.cpp b/examples/ota-provider-app/linux/main.cpp
index 52064236127e3c..f2a526a4bb9e55 100644
--- a/examples/ota-provider-app/linux/main.cpp
+++ b/examples/ota-provider-app/linux/main.cpp
@@ -46,6 +46,7 @@ constexpr chip::EndpointId kOtaProviderEndpoint = 0;
 constexpr uint16_t kOptionUpdateAction              = 'a';
 constexpr uint16_t kOptionUserConsentNeeded         = 'c';
 constexpr uint16_t kOptionFilepath                  = 'f';
+constexpr uint16_t kOptionImageUri                  = 'i';
 constexpr uint16_t kOptionOtaImageList              = 'o';
 constexpr uint16_t kOptionDelayedApplyActionTimeSec = 'p';
 constexpr uint16_t kOptionQueryImageStatus          = 'q';
@@ -64,6 +65,7 @@ static uint32_t gDelayedQueryActionTimeSec           = 0;
 static uint32_t gDelayedApplyActionTimeSec           = 0;
 static const char * gOtaFilepath                     = nullptr;
 static const char * gOtaImageListFilepath            = nullptr;
+static const char * gImageUri                        = nullptr;
 static chip::ota::UserConsentState gUserConsentState = chip::ota::UserConsentState::kUnknown;
 static bool gUserConsentNeeded                       = false;
 static uint32_t gIgnoreQueryImageCount               = 0;
@@ -147,6 +149,9 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
             gOtaFilepath = aValue;
         }
         break;
+    case kOptionImageUri:
+        gImageUri = aValue;
+        break;
     case kOptionOtaImageList:
         kOptionOtaImageListSelected = true;
         if (0 != access(aValue, R_OK))
@@ -165,12 +170,7 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
         }
         break;
     case kOptionQueryImageStatus:
-        if (aValue == NULL)
-        {
-            PrintArgError("%s: ERROR: NULL queryImageStatus parameter\n", aProgram);
-            retval = false;
-        }
-        else if (strcmp(aValue, "updateAvailable") == 0)
+        if (strcmp(aValue, "updateAvailable") == 0)
         {
             gQueryImageStatus = OTAQueryStatus::kUpdateAvailable;
         }
@@ -195,12 +195,7 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
         gIgnoreApplyUpdateCount = static_cast<uint32_t>(strtoul(aValue, NULL, 0));
         break;
     case kOptionUpdateAction:
-        if (aValue == NULL)
-        {
-            PrintArgError("%s: ERROR: NULL applyUpdateAction parameter\n", aProgram);
-            retval = false;
-        }
-        else if (strcmp(aValue, "proceed") == 0)
+        if (strcmp(aValue, "proceed") == 0)
         {
             gOptionUpdateAction = OTAApplyUpdateAction::kProceed;
         }
@@ -225,12 +220,7 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
         gDelayedApplyActionTimeSec = static_cast<uint32_t>(strtoul(aValue, NULL, 0));
         break;
     case kOptionUserConsentState:
-        if (aValue == NULL)
-        {
-            PrintArgError("%s: ERROR: NULL UserConsent parameter\n", aProgram);
-            retval = false;
-        }
-        else if (strcmp(aValue, "granted") == 0)
+        if (strcmp(aValue, "granted") == 0)
         {
             gUserConsentState = chip::ota::UserConsentState::kGranted;
         }
@@ -264,6 +254,7 @@ OptionDef cmdLineOptionsDef[] = {
     { "applyUpdateAction", chip::ArgParser::kArgumentRequired, kOptionUpdateAction },
     { "userConsentNeeded", chip::ArgParser::kNoArgument, kOptionUserConsentNeeded },
     { "filepath", chip::ArgParser::kArgumentRequired, kOptionFilepath },
+    { "imageUri", chip::ArgParser::kArgumentRequired, kOptionImageUri },
     { "otaImageList", chip::ArgParser::kArgumentRequired, kOptionOtaImageList },
     { "delayedApplyActionTimeSec", chip::ArgParser::kArgumentRequired, kOptionDelayedApplyActionTimeSec },
     { "queryImageStatus", chip::ArgParser::kArgumentRequired, kOptionQueryImageStatus },
@@ -285,6 +276,9 @@ OptionSet cmdLineOptions = { HandleOptions, cmdLineOptionsDef, "PROGRAM OPTIONS"
                              "        Otherwise, value of the UserConsentNeeded field is false.\n"
                              "  -f, --filepath <file path>\n"
                              "        Path to a file containing an OTA image\n"
+                             "  -i, --imageUri <uri>\n"
+                             "        Value for the ImageURI field in the QueryImageResponse.\n"
+                             "        If none is supplied, a valid URI is generated.\n"
                              "  -o, --otaImageList <file path>\n"
                              "        Path to a file containing a list of OTA images\n"
                              "  -p, --delayedApplyActionTimeSec <time in seconds>\n"
@@ -331,6 +325,11 @@ void ApplicationInit()
         gOtaProvider.SetOTAFilePath(gOtaFilepath);
     }
 
+    if (gImageUri != nullptr)
+    {
+        gOtaProvider.SetImageUri(gImageUri);
+    }
+
     gOtaProvider.SetIgnoreQueryImageCount(gIgnoreQueryImageCount);
     gOtaProvider.SetIgnoreApplyUpdateCount(gIgnoreApplyUpdateCount);
     gOtaProvider.SetQueryImageStatus(gQueryImageStatus);
diff --git a/examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp b/examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp
index 1b3266fc42638f..636e4d4cc08303 100644
--- a/examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp
+++ b/examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp
@@ -52,7 +52,6 @@ using namespace chip::app::Clusters::OtaSoftwareUpdateProvider::Commands;
 
 constexpr uint8_t kUpdateTokenLen    = 32;                      // must be between 8 and 32
 constexpr uint8_t kUpdateTokenStrLen = kUpdateTokenLen * 2 + 1; // Hex string needs 2 hex chars for every byte
-constexpr size_t kUriMaxLen          = 256;
 constexpr size_t kOtaHeaderMaxSize   = 1024;
 
 // Arbitrary BDX Transfer Params
@@ -80,7 +79,8 @@ void GenerateUpdateToken(uint8_t * buf, size_t bufSize)
 
 OTAProviderExample::OTAProviderExample()
 {
-    memset(mOTAFilePath, 0, kFilepathBufLen);
+    memset(mOTAFilePath, 0, sizeof(mOTAFilePath));
+    memset(mImageUri, 0, sizeof(mImageUri));
     mIgnoreQueryImageCount     = 0;
     mIgnoreApplyUpdateCount    = 0;
     mQueryImageStatus          = OTAQueryStatus::kNotAvailable;
@@ -100,7 +100,19 @@ void OTAProviderExample::SetOTAFilePath(const char * path)
     }
     else
     {
-        memset(mOTAFilePath, 0, kFilepathBufLen);
+        memset(mOTAFilePath, 0, sizeof(mOTAFilePath));
+    }
+}
+
+void OTAProviderExample::SetImageUri(const char * imageUri)
+{
+    if (imageUri != nullptr)
+    {
+        chip::Platform::CopyString(mImageUri, imageUri);
+    }
+    else
+    {
+        memset(mImageUri, 0, sizeof(mImageUri));
     }
 }
 
@@ -225,7 +237,6 @@ void OTAProviderExample::SendQueryImageResponse(app::CommandHandler * commandObj
     bool requestorCanConsent             = commandData.requestorCanConsent.ValueOr(false);
     uint8_t updateToken[kUpdateTokenLen] = { 0 };
     char strBuf[kUpdateTokenStrLen]      = { 0 };
-    char uriBuf[kUriMaxLen]              = { 0 };
 
     // Set fields specific for an available status response
     if (mQueryImageStatus == OTAQueryStatus::kUpdateAvailable)
@@ -240,10 +251,22 @@ void OTAProviderExample::SendQueryImageResponse(app::CommandHandler * commandObj
         FabricInfo * fabricInfo = Server::GetInstance().GetFabricTable().FindFabricWithIndex(fabricIndex);
         NodeId nodeId           = fabricInfo->GetPeerId().GetNodeId();
 
-        // Only supporting BDX protocol for now
-        MutableCharSpan uri(uriBuf, kUriMaxLen);
-        chip::bdx::MakeURI(nodeId, CharSpan::fromCharString(mOTAFilePath), uri);
-        ChipLogDetail(SoftwareUpdate, "Generated URI: %.*s", static_cast<int>(uri.size()), uri.data());
+        // Generate the ImageURI if one is not already preset
+        if (strlen(mImageUri) == 0)
+        {
+            // Only supporting BDX protocol for now
+            MutableCharSpan uri(mImageUri);
+            CHIP_ERROR error = chip::bdx::MakeURI(nodeId, CharSpan::fromCharString(mOTAFilePath), uri);
+            if (error != CHIP_NO_ERROR)
+            {
+                ChipLogError(SoftwareUpdate, "Cannot generate URI");
+                memset(mImageUri, 0, sizeof(mImageUri));
+            }
+            else
+            {
+                ChipLogDetail(SoftwareUpdate, "Generated URI: %s", mImageUri);
+            }
+        }
 
         // Initialize the transfer session in prepartion for a BDX transfer
         BitFlags<TransferControlFlags> bdxFlags;
@@ -260,7 +283,7 @@ void OTAProviderExample::SendQueryImageResponse(app::CommandHandler * commandObj
                 return;
             }
 
-            response.imageURI.Emplace(chip::CharSpan::fromCharString(uriBuf));
+            response.imageURI.Emplace(chip::CharSpan::fromCharString(mImageUri));
             response.softwareVersion.Emplace(mSoftwareVersion);
             response.softwareVersionString.Emplace(chip::CharSpan::fromCharString(mSoftwareVersionString));
             response.updateToken.Emplace(chip::ByteSpan(updateToken));
@@ -307,7 +330,7 @@ void OTAProviderExample::HandleQueryImage(app::CommandHandler * commandObj, cons
 
     if (mQueryImageStatus == OTAQueryStatus::kUpdateAvailable)
     {
-        memset(mSoftwareVersionString, 0, SW_VER_STR_MAX_LEN);
+        memset(mSoftwareVersionString, 0, sizeof(mSoftwareVersionString));
 
         if (!mCandidates.empty()) // If list of OTA candidates is supplied
         {
diff --git a/examples/ota-provider-app/ota-provider-common/OTAProviderExample.h b/examples/ota-provider-app/ota-provider-common/OTAProviderExample.h
index a3b63b4398918b..67f67576545bb7 100644
--- a/examples/ota-provider-app/ota-provider-common/OTAProviderExample.h
+++ b/examples/ota-provider-app/ota-provider-common/OTAProviderExample.h
@@ -40,6 +40,7 @@ class OTAProviderExample : public chip::app::Clusters::OTAProviderDelegate
     static constexpr uint16_t SW_VER_STR_MAX_LEN = 64;
     static constexpr uint16_t OTA_URL_MAX_LEN    = 512;
     static constexpr size_t kFilepathBufLen      = 256;
+    static constexpr size_t kUriMaxLen           = 256;
 
     typedef struct DeviceSoftwareVersionModel
     {
@@ -67,6 +68,7 @@ class OTAProviderExample : public chip::app::Clusters::OTAProviderDelegate
 
     //////////// OTAProviderExample public APIs ///////////////
     void SetOTAFilePath(const char * path);
+    void SetImageUri(const char * imageUri);
     BdxOtaSender * GetBdxOtaSender() { return &mBdxOtaSender; }
 
     void SetOTACandidates(std::vector<OTAProviderExample::DeviceSoftwareVersionModel> candidates);
@@ -104,6 +106,7 @@ class OTAProviderExample : public chip::app::Clusters::OTAProviderDelegate
     BdxOtaSender mBdxOtaSender;
     std::vector<DeviceSoftwareVersionModel> mCandidates;
     char mOTAFilePath[kFilepathBufLen]; // null-terminated
+    char mImageUri[kUriMaxLen];
     OTAQueryStatus mQueryImageStatus;
     OTAApplyUpdateAction mUpdateAction;
     uint32_t mIgnoreQueryImageCount;
diff --git a/src/protocols/bdx/BdxUri.cpp b/src/protocols/bdx/BdxUri.cpp
index 85c4f553f3e3df..a47b2de23034d7 100644
--- a/src/protocols/bdx/BdxUri.cpp
+++ b/src/protocols/bdx/BdxUri.cpp
@@ -67,7 +67,12 @@ CHIP_ERROR MakeURI(NodeId nodeId, CharSpan fileDesignator, MutableCharSpan & uri
     char nodeIdHex[sizeof(NodeId) * 2];
     ReturnErrorOnFailure(Encoding::BytesToUppercaseHexBuffer(nodeIdBytes, sizeof(nodeIdBytes), nodeIdHex, sizeof(nodeIdHex)));
 
-    Encoding::BufferWriter writer(Uint8::from_char(uri.data()), uri.size());
+    char * buffer     = uri.data();
+    size_t bufferSize = uri.size();
+    memset(buffer, 0, bufferSize);
+
+    // Reduce the buffer writer size by one to reserve the last byte for the null-terminator
+    Encoding::BufferWriter writer(Uint8::from_char(buffer), bufferSize - 1);
     writer.Put(kScheme, kSchemeLen);
     writer.Put(nodeIdHex, sizeof(nodeIdHex));
     writer.Put("/");
diff --git a/src/protocols/bdx/BdxUri.h b/src/protocols/bdx/BdxUri.h
index de8b4d681adce8..776ccffe0973b0 100644
--- a/src/protocols/bdx/BdxUri.h
+++ b/src/protocols/bdx/BdxUri.h
@@ -24,7 +24,16 @@ namespace bdx {
 
 constexpr const char kScheme[] = "bdx://";
 
+/**
+ * Parses the URI into NodeId and File Designator
+ */
 CHIP_ERROR ParseURI(CharSpan uri, NodeId & nodeId, CharSpan & fileDesignator);
+
+/**
+ * Creates a null-terminated URI in the form of bdx://<16-char-node-id>/<image-file-designator>
+ *
+ * The resulting size of the span will not include the null terminator.
+ */
 CHIP_ERROR MakeURI(NodeId nodeId, CharSpan fileDesignator, MutableCharSpan & uri);
 
 } // namespace bdx