From ee78c59771c7099f42acc508d249488fc7e34d18 Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Wed, 3 Apr 2024 13:31:27 -0400
Subject: [PATCH] Fix Accessors getters for nullable string/octstr attributes.

Callers have to pass a Nullable<Span> that already points to the buffer to
fill.  But we were calling SetNonNull(), which reset the Nullable to point to an
empty span, after which copying the data in of course failed.

The right thing to do is to ensure that the Nullable has a value, then use that
value.
---
 .../app/attributes/Accessors-src.zapt         | 10 ++-
 .../zap-generated/attributes/Accessors.cpp    | 65 ++++++++++++++++---
 2 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/src/app/zap-templates/templates/app/attributes/Accessors-src.zapt b/src/app/zap-templates/templates/app/attributes/Accessors-src.zapt
index 2dd92f0f7450ec..5db9e3c8fd839b 100644
--- a/src/app/zap-templates/templates/app/attributes/Accessors-src.zapt
+++ b/src/app/zap-templates/templates/app/attributes/Accessors-src.zapt
@@ -14,6 +14,7 @@
 #include <app/util/attribute-storage-null-handling.h>
 #include <app/util/odd-sized-integers.h>
 #include <lib/core/CHIPEncoding.h>
+#include <lib/support/logging/CHIPLogging.h>
 
 namespace chip {
 namespace app {
@@ -38,6 +39,13 @@ namespace {{asUpperCamelCase label}} {
 Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, {{accessorGetterType this}} value)
 {
     {{~#if (isString type)}}
+    {{#if isNullable}}
+    if (value.IsNull()) {
+      ChipLogError(Zcl, "Null Nullable<Span> passed to {{asUpperCamelCase parent.label}}::{{asUpperCamelCase label}}::Get");
+      return Protocols::InteractionModel::Status::Failure;
+    }
+
+    {{/if}}
     {{~#*inline "lengthType"}}uint{{#if (isShortString type)}}8{{else}}16{{/if}}_t{{/inline}}
     uint8_t zclString[{{maxLength}} + {{>sizingBytes}}];
     Protocols::InteractionModel::Status status = emberAfReadAttribute(endpoint, {{>clusterId}}, Id, zclString, sizeof(zclString));
@@ -53,7 +61,7 @@ Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, {{accessorGet
       {{/if}}
     }
     {{#if isNullable}}
-    auto & span = value.SetNonNull();
+    auto & span = value.Value();
     {{/if}}
     {{~#*inline "value"}}{{#if isNullable}}span{{else}}value{{/if}}{{/inline}}
     VerifyOrReturnError({{>value}}.size() == {{maxLength}}, Protocols::InteractionModel::Status::InvalidDataType);
diff --git a/zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp b/zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp
index a3e74424d0768b..9decf50aaaff30 100644
--- a/zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp
+++ b/zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp
@@ -31,6 +31,7 @@
 #include <app/util/attribute-storage-null-handling.h>
 #include <app/util/odd-sized-integers.h>
 #include <lib/core/CHIPEncoding.h>
+#include <lib/support/logging/CHIPLogging.h>
 
 namespace chip {
 namespace app {
@@ -4216,6 +4217,12 @@ namespace LastNetworkID {
 
 Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nullable<chip::MutableByteSpan> & value)
 {
+    if (value.IsNull())
+    {
+        ChipLogError(Zcl, "Null Nullable<Span> passed to NetworkCommissioning::LastNetworkID::Get");
+        return Protocols::InteractionModel::Status::Failure;
+    }
+
     uint8_t zclString[32 + 1];
     Protocols::InteractionModel::Status status =
         emberAfReadAttribute(endpoint, Clusters::NetworkCommissioning::Id, Id, zclString, sizeof(zclString));
@@ -4226,7 +4233,7 @@ Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nu
         value.SetNull();
         return Protocols::InteractionModel::Status::Success;
     }
-    auto & span = value.SetNonNull();
+    auto & span = value.Value();
 
     VerifyOrReturnError(span.size() == 32, Protocols::InteractionModel::Status::InvalidDataType);
     memcpy(span.data(), &zclString[1], 32);
@@ -12233,6 +12240,12 @@ namespace AliroReaderVerificationKey {
 
 Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nullable<chip::MutableByteSpan> & value)
 {
+    if (value.IsNull())
+    {
+        ChipLogError(Zcl, "Null Nullable<Span> passed to DoorLock::AliroReaderVerificationKey::Get");
+        return Protocols::InteractionModel::Status::Failure;
+    }
+
     uint8_t zclString[65 + 1];
     Protocols::InteractionModel::Status status =
         emberAfReadAttribute(endpoint, Clusters::DoorLock::Id, Id, zclString, sizeof(zclString));
@@ -12243,7 +12256,7 @@ Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nu
         value.SetNull();
         return Protocols::InteractionModel::Status::Success;
     }
-    auto & span = value.SetNonNull();
+    auto & span = value.Value();
 
     VerifyOrReturnError(span.size() == 65, Protocols::InteractionModel::Status::InvalidDataType);
     memcpy(span.data(), &zclString[1], 65);
@@ -12283,6 +12296,12 @@ namespace AliroReaderGroupIdentifier {
 
 Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nullable<chip::MutableByteSpan> & value)
 {
+    if (value.IsNull())
+    {
+        ChipLogError(Zcl, "Null Nullable<Span> passed to DoorLock::AliroReaderGroupIdentifier::Get");
+        return Protocols::InteractionModel::Status::Failure;
+    }
+
     uint8_t zclString[16 + 1];
     Protocols::InteractionModel::Status status =
         emberAfReadAttribute(endpoint, Clusters::DoorLock::Id, Id, zclString, sizeof(zclString));
@@ -12293,7 +12312,7 @@ Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nu
         value.SetNull();
         return Protocols::InteractionModel::Status::Success;
     }
-    auto & span = value.SetNonNull();
+    auto & span = value.Value();
 
     VerifyOrReturnError(span.size() == 16, Protocols::InteractionModel::Status::InvalidDataType);
     memcpy(span.data(), &zclString[1], 16);
@@ -12365,6 +12384,12 @@ namespace AliroGroupResolvingKey {
 
 Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nullable<chip::MutableByteSpan> & value)
 {
+    if (value.IsNull())
+    {
+        ChipLogError(Zcl, "Null Nullable<Span> passed to DoorLock::AliroGroupResolvingKey::Get");
+        return Protocols::InteractionModel::Status::Failure;
+    }
+
     uint8_t zclString[16 + 1];
     Protocols::InteractionModel::Status status =
         emberAfReadAttribute(endpoint, Clusters::DoorLock::Id, Id, zclString, sizeof(zclString));
@@ -12375,7 +12400,7 @@ Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nu
         value.SetNull();
         return Protocols::InteractionModel::Status::Success;
     }
-    auto & span = value.SetNonNull();
+    auto & span = value.Value();
 
     VerifyOrReturnError(span.size() == 16, Protocols::InteractionModel::Status::InvalidDataType);
     memcpy(span.data(), &zclString[1], 16);
@@ -17112,6 +17137,12 @@ namespace ActivePresetHandle {
 
 Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nullable<chip::MutableByteSpan> & value)
 {
+    if (value.IsNull())
+    {
+        ChipLogError(Zcl, "Null Nullable<Span> passed to Thermostat::ActivePresetHandle::Get");
+        return Protocols::InteractionModel::Status::Failure;
+    }
+
     uint8_t zclString[16 + 1];
     Protocols::InteractionModel::Status status =
         emberAfReadAttribute(endpoint, Clusters::Thermostat::Id, Id, zclString, sizeof(zclString));
@@ -17122,7 +17153,7 @@ Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nu
         value.SetNull();
         return Protocols::InteractionModel::Status::Success;
     }
-    auto & span = value.SetNonNull();
+    auto & span = value.Value();
 
     VerifyOrReturnError(span.size() == 16, Protocols::InteractionModel::Status::InvalidDataType);
     memcpy(span.data(), &zclString[1], 16);
@@ -17162,6 +17193,12 @@ namespace ActiveScheduleHandle {
 
 Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nullable<chip::MutableByteSpan> & value)
 {
+    if (value.IsNull())
+    {
+        ChipLogError(Zcl, "Null Nullable<Span> passed to Thermostat::ActiveScheduleHandle::Get");
+        return Protocols::InteractionModel::Status::Failure;
+    }
+
     uint8_t zclString[16 + 1];
     Protocols::InteractionModel::Status status =
         emberAfReadAttribute(endpoint, Clusters::Thermostat::Id, Id, zclString, sizeof(zclString));
@@ -17172,7 +17209,7 @@ Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nu
         value.SetNull();
         return Protocols::InteractionModel::Status::Success;
     }
-    auto & span = value.SetNonNull();
+    auto & span = value.Value();
 
     VerifyOrReturnError(span.size() == 16, Protocols::InteractionModel::Status::InvalidDataType);
     memcpy(span.data(), &zclString[1], 16);
@@ -31764,6 +31801,12 @@ namespace NullableOctetString {
 
 Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nullable<chip::MutableByteSpan> & value)
 {
+    if (value.IsNull())
+    {
+        ChipLogError(Zcl, "Null Nullable<Span> passed to UnitTesting::NullableOctetString::Get");
+        return Protocols::InteractionModel::Status::Failure;
+    }
+
     uint8_t zclString[10 + 1];
     Protocols::InteractionModel::Status status =
         emberAfReadAttribute(endpoint, Clusters::UnitTesting::Id, Id, zclString, sizeof(zclString));
@@ -31774,7 +31817,7 @@ Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nu
         value.SetNull();
         return Protocols::InteractionModel::Status::Success;
     }
-    auto & span = value.SetNonNull();
+    auto & span = value.Value();
 
     VerifyOrReturnError(span.size() == 10, Protocols::InteractionModel::Status::InvalidDataType);
     memcpy(span.data(), &zclString[1], 10);
@@ -31814,6 +31857,12 @@ namespace NullableCharString {
 
 Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nullable<chip::MutableCharSpan> & value)
 {
+    if (value.IsNull())
+    {
+        ChipLogError(Zcl, "Null Nullable<Span> passed to UnitTesting::NullableCharString::Get");
+        return Protocols::InteractionModel::Status::Failure;
+    }
+
     uint8_t zclString[10 + 1];
     Protocols::InteractionModel::Status status =
         emberAfReadAttribute(endpoint, Clusters::UnitTesting::Id, Id, zclString, sizeof(zclString));
@@ -31824,7 +31873,7 @@ Protocols::InteractionModel::Status Get(chip::EndpointId endpoint, DataModel::Nu
         value.SetNull();
         return Protocols::InteractionModel::Status::Success;
     }
-    auto & span = value.SetNonNull();
+    auto & span = value.Value();
 
     VerifyOrReturnError(span.size() == 10, Protocols::InteractionModel::Status::InvalidDataType);
     memcpy(span.data(), &zclString[1], 10);