public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [edk2-redfish-client][PATCH] RedfishClientPkg/RedfishFeatureUtilityLib: validate string array
@ 2023-11-08  7:24 Nickle Wang via groups.io
  2023-11-09 15:10 ` Igor Kulchytskyy via groups.io
  0 siblings, 1 reply; 3+ messages in thread
From: Nickle Wang via groups.io @ 2023-11-08  7:24 UTC (permalink / raw)
  To: devel; +Cc: Abner Chang, Igor Kulchytskyy, Nick Ramirez

- Add function to validate Redfish request for string array
type. There is case that user request invalid string array
and feature driver can not find corresponding HII option.
- Fix typo.

Signed-off-by: Nickle Wang <nicklew@nvidia.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nick Ramirez <nramirez@nvidia.com>
---
 .../Library/RedfishFeatureUtilityLib.h        |  56 +++++--
 .../RedfishFeatureUtilityLib.c                | 156 +++++++++++++-----
 2 files changed, 154 insertions(+), 58 deletions(-)

diff --git a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
index e2f728b2..440c4b68 100644
--- a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
+++ b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
@@ -22,7 +22,7 @@
 
   Read redfish resource by given resource URI.
 
-  @param[in]  Service       Redfish srvice instacne to make query.
+  @param[in]  Service       Redfish service instance to make query.
   @param[in]  ResourceUri   Target resource URI.
   @param[out] Response      HTTP response from redfish service.
 
@@ -47,7 +47,7 @@ GetResourceByUri (
   @param[out] ArraySignatureClose       String to the close of array signature.
 
   @retval     EFI_SUCCESS            Index is found.
-  @retval     EFI_NOT_FOUND          The non-array configure language string is retured.
+  @retval     EFI_NOT_FOUND          The non-array configure language string is returned.
   @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang is wrong.
   @retval     Others                 Errors occur.
 
@@ -118,10 +118,10 @@ GetNumberOfRedpathNodes (
 
   @param[in]  NodeString             The node string to parse.
   @param[in]  Index                  Index of the node.
-  @param[out] EndOfNodePtr           Pointer to receive the poitner to
+  @param[out] EndOfNodePtr           Pointer to receive the pointer to
                                      the last character of node string.
 
-  @retval     EFI_STRING             the begining of the node string.
+  @retval     EFI_STRING             the beginning of the node string.
 
 **/
 EFI_STRING
@@ -140,7 +140,7 @@ GetRedpathNodeByIndex (
   @param[out] Index                 The array index number.
 
   @retval     EFI_SUCCESS            Index is found.
-  @retval     EFI_NOT_FOUND          The non-array configure language string is retured.
+  @retval     EFI_NOT_FOUND          The non-array configure language string is returned.
   @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang is wrong.
   @retval     Others                 Errors occur.
 
@@ -188,7 +188,7 @@ DestroyConfiglanguageList (
 
   @param[in]  DestConfigLang        Pointer to the node's configure language string.
                                     The memory pointed by ConfigLang must be allocated
-                                    through memory allocation interface. Becasue we will replace
+                                    through memory allocation interface. Because we will replace
                                     the pointer in this function.
   @param[in]  MaxtLengthConfigLang  The maximum length of ConfigLang.
   @param[in]  ConfigLangInstance    Pointer to Collection member instance.
@@ -244,7 +244,7 @@ ApplyFeatureSettingsStringType (
 
 /**
 
-  Apply property value to UEFI HII database in numric type.
+  Apply property value to UEFI HII database in numeric type.
 
   @param[in]  Schema        Property schema.
   @param[in]  Version       Property schema version.
@@ -356,7 +356,7 @@ ApplyFeatureSettingsNumericArrayType (
   @param[in]  Schema        Property schema.
   @param[in]  Version       Property schema version.
   @param[in]  ConfigureLang Configure language refers to this property.
-  @param[in]  ArrayHead     Head of Redfich CS boolean array value.
+  @param[in]  ArrayHead     Head of Redfish CS boolean array value.
 
   @retval     EFI_SUCCESS     New value is applied successfully.
   @retval     Others          Errors occur.
@@ -421,7 +421,7 @@ CreatePayloadToPatchResource (
   @param[in]    ConfigLang        ConfigLang to save
   @param[in]    Uri               Redfish Uri to save
 
-  @retval  EFI_INVALID_PARAMETR   SystemId is NULL or EMPTY
+  @retval  EFI_INVALID_PARAMETER   SystemId is NULL or EMPTY
   @retval  EFI_SUCCESS            Redfish uri is saved
 
 **/
@@ -433,7 +433,7 @@ RedfisSetRedfishUri (
 
 /**
 
-  Get the property name by given Configure Langauge.
+  Get the property name by given Configure Language.
 
   @param[in]  ResourceUri              URI of root of resource.
   @param[in]  ConfigureLang            Configure Language string.
@@ -576,7 +576,7 @@ GetOdataId (
 
 /**
 
-  Return config language from given URI and prperty name. It's call responsibility to release returned buffer.
+  Return config language from given URI and property name. It's call responsibility to release returned buffer.
 
   @param[in] Uri            The URI to match
   @param[in] PropertyName   The property name of resource. This is optional.
@@ -790,7 +790,7 @@ MatchPropertyWithJsonContext (
 
 /**
 
-  Create string array and append to arry node in Redfish JSON convert format.
+  Create string array and append to array node in Redfish JSON convert format.
 
   @param[in,out]  Head          The head of string array.
   @param[in]      StringArray   Input string array.
@@ -809,7 +809,7 @@ AddRedfishCharArray (
 
 /**
 
-  Create numeric array and append to arry node in Redfish JSON convert format.
+  Create numeric array and append to array node in Redfish JSON convert format.
 
   @param[in,out]  Head           The head of string array.
   @param[in]      NumericArray   Input numeric array.
@@ -828,7 +828,7 @@ AddRedfishNumericArray (
 
 /**
 
-  Create boolean array and append to arry node in Redfish JSON convert format.
+  Create boolean array and append to array node in Redfish JSON convert format.
 
   @param[in,out]  Head           The head of string array.
   @param[in]      BooleanArray   Input boolean array.
@@ -866,12 +866,34 @@ CompareRedfishStringArrayValues (
   IN UINTN                 ArraySize
   );
 
+/**
+
+  Check and see if value in Redfish string array can be found in HII
+  configuration string array. This is to see if there is any invalid
+  values from Redfish.
+
+  @param[in]  Head          The head of string array.
+  @param[in]  StringArray   Input string array.
+  @param[in]  ArraySize     The size of StringArray.
+
+  @retval     TRUE          All string in Redfish array are as same as string
+                            in HII configuration array.
+              FALSE         These two array are not identical.
+
+**/
+BOOLEAN
+ValidateRedfishStringArrayValues (
+  IN RedfishCS_char_Array  *Head,
+  IN CHAR8                 **StringArray,
+  IN UINTN                 ArraySize
+  );
+
 /**
 
   Check and see if value in Redfish numeric array are all the same as the one
   from HII configuration.
 
-  @param[in]  Head          The head of Redfish CS numeraic array.
+  @param[in]  Head          The head of Redfish CS numeric array.
   @param[in]  NumericArray  Input numeric array.
   @param[in]  ArraySize     The size of NumericArray.
 
@@ -914,9 +936,9 @@ CompareRedfishBooleanArrayValues (
   This is just a simple check.
 
   @param[in]  RedfishVagueKeyValuePtr     The vague key value sets on Redfish service.
-  @param[in]  RedfishVagueKeyValueNumber  The numebr of vague key value sets
+  @param[in]  RedfishVagueKeyValueNumber  The number of vague key value sets
   @param[in]  ConfigVagueKeyValuePtr      The vague configuration on platform.
-  @param[in]  ConfigVagueKeyValueNumber   The numebr of vague key value sets
+  @param[in]  ConfigVagueKeyValueNumber   The number of vague key value sets
 
   @retval     TRUE          All values are the same.
               FALSE         There is some difference.
diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
index 13e29902..a3ca1006 100644
--- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
+++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
@@ -772,61 +772,69 @@ ApplyFeatureSettingsStringArrayType (
   }
 
   //
-  // If there is no change in array, do nothing
+  // Check and see if element in request string array can be found in HII string array.
   //
-  if (!CompareRedfishStringArrayValues (ArrayHead, RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
-    //
-    // Apply settings from redfish
-    //
-    DEBUG ((DEBUG_MANAGEABILITY, "%a: %a.%a apply %s for array\n", __func__, Schema, Version, ConfigureLang));
-    FreeArrayTypeRedfishValue (&RedfishValue);
-
+  if (ValidateRedfishStringArrayValues (ArrayHead, RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
     //
-    // Convert array from RedfishCS_char_Array to EDKII_REDFISH_VALUE
+    // If there is no change in array, do nothing
     //
-    RedfishValue.ArrayCount = 0;
-    Buffer                  = ArrayHead;
-    while (Buffer != NULL) {
-      RedfishValue.ArrayCount += 1;
-      Buffer                   = Buffer->Next;
-    }
+    if (!CompareRedfishStringArrayValues (ArrayHead, RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
+      //
+      // Apply settings from redfish
+      //
+      DEBUG ((DEBUG_MANAGEABILITY, "%a: %a.%a apply %s for array\n", __func__, Schema, Version, ConfigureLang));
+      FreeArrayTypeRedfishValue (&RedfishValue);
 
-    //
-    // Allocate pool for new values
-    //
-    RedfishValue.Value.StringArray = AllocatePool (RedfishValue.ArrayCount *sizeof (CHAR8 *));
-    if (RedfishValue.Value.StringArray == NULL) {
-      ASSERT (FALSE);
-      return EFI_OUT_OF_RESOURCES;
-    }
+      //
+      // Convert array from RedfishCS_char_Array to EDKII_REDFISH_VALUE
+      //
+      RedfishValue.ArrayCount = 0;
+      Buffer                  = ArrayHead;
+      while (Buffer != NULL) {
+        RedfishValue.ArrayCount += 1;
+        Buffer                   = Buffer->Next;
+      }
 
-    Buffer = ArrayHead;
-    Index  = 0;
-    while (Buffer != NULL) {
-      RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize (Buffer->ArrayValue), Buffer->ArrayValue);
-      if (RedfishValue.Value.StringArray[Index] == NULL) {
+      //
+      // Allocate pool for new values
+      //
+      RedfishValue.Value.StringArray = AllocatePool (RedfishValue.ArrayCount *sizeof (CHAR8 *));
+      if (RedfishValue.Value.StringArray == NULL) {
         ASSERT (FALSE);
-        FreePool (RedfishValue.Value.StringArray);
         return EFI_OUT_OF_RESOURCES;
       }
 
-      Buffer = Buffer->Next;
-      Index++;
-    }
+      Buffer = ArrayHead;
+      Index  = 0;
+      while (Buffer != NULL) {
+        RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize (Buffer->ArrayValue), Buffer->ArrayValue);
+        if (RedfishValue.Value.StringArray[Index] == NULL) {
+          ASSERT (FALSE);
+          FreePool (RedfishValue.Value.StringArray);
+          return EFI_OUT_OF_RESOURCES;
+        }
 
-    ASSERT (Index <= RedfishValue.ArrayCount);
+        Buffer = Buffer->Next;
+        Index++;
+      }
 
-    Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang, RedfishValue);
-    if (!EFI_ERROR (Status)) {
-      //
-      // Configuration changed. Enable system reboot flag.
-      //
-      REDFISH_ENABLE_SYSTEM_REBOOT ();
+      ASSERT (Index <= RedfishValue.ArrayCount);
+
+      Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang, RedfishValue);
+      if (!EFI_ERROR (Status)) {
+        //
+        // Configuration changed. Enable system reboot flag.
+        //
+        REDFISH_ENABLE_SYSTEM_REBOOT ();
+      } else {
+        DEBUG ((DEBUG_ERROR, "%a: apply %s array failed: %r\n", __func__, ConfigureLang, Status));
+      }
     } else {
-      DEBUG ((DEBUG_ERROR, "%a: apply %s array failed: %r\n", __func__, ConfigureLang, Status));
+      DEBUG ((DEBUG_ERROR, "%a: %a.%a %s array value has no change\n", __func__, Schema, Version, ConfigureLang));
     }
   } else {
-    DEBUG ((DEBUG_ERROR, "%a: %a.%a %s array value has no change\n", __func__, Schema, Version, ConfigureLang));
+    DEBUG ((DEBUG_ERROR, "%a: %a.%a %s array value has invalid element, skip!\n", __func__, Schema, Version, ConfigureLang));
+    Status = EFI_DEVICE_ERROR;
   }
 
   for (Index = 0; Index < RedfishValue.ArrayCount; Index++) {
@@ -3432,6 +3440,72 @@ CompareRedfishStringArrayValues (
   return TRUE;
 }
 
+/**
+
+  Check and see if value in Redfish string array can be found in HII
+  configuration string array. This is to see if there is any invalid
+  values from Redfish.
+
+  @param[in]  Head          The head of string array.
+  @param[in]  StringArray   Input string array.
+  @param[in]  ArraySize     The size of StringArray.
+
+  @retval     TRUE          All string in Redfish array are as same as string
+                            in HII configuration array.
+              FALSE         These two array are not identical.
+
+**/
+BOOLEAN
+ValidateRedfishStringArrayValues (
+  IN RedfishCS_char_Array  *Head,
+  IN CHAR8                 **StringArray,
+  IN UINTN                 ArraySize
+  )
+{
+  UINTN                 Index;
+  UINTN                 InputArrayCount;
+  RedfishCS_char_Array  *CharArrayBuffer;
+
+  if ((Head == NULL) || (StringArray == NULL) || (ArraySize == 0)) {
+    return FALSE;
+  }
+
+  //
+  // Check to see if string from Redfish can be found in string array
+  // returned by HII or not. If not, the input from Redfish is invalid.
+  //
+  CharArrayBuffer = Head;
+  Index           = 0;
+  InputArrayCount = 0;
+  while (CharArrayBuffer != NULL) {
+    for (Index = 0; Index < ArraySize; Index++) {
+      if (AsciiStrCmp (StringArray[Index], CharArrayBuffer->ArrayValue) == 0) {
+        break;
+      }
+    }
+
+    if (Index == ArraySize) {
+      DEBUG ((DEBUG_ERROR, "%a: input string: %a is not found in HII string list\n", __func__, CharArrayBuffer->ArrayValue));
+      return FALSE;
+    }
+
+    InputArrayCount += 1;
+    CharArrayBuffer  = CharArrayBuffer->Next;
+  }
+
+  //
+  // Check to see if the number of string from Redfish equals to the
+  // number of string returned by HII. HII only accepts the same
+  // number of string array due to the design or HII ordered list.
+  //
+  if (InputArrayCount != ArraySize) {
+    DEBUG ((DEBUG_ERROR, "%a: input string size: %d is not the same as HII string list size: %d\n", __func__, InputArrayCount, ArraySize));
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
 /**
 
   Check and see if value in Redfish numeric array are all the same as the one
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110901): https://edk2.groups.io/g/devel/message/110901
Mute This Topic: https://groups.io/mt/102459779/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [edk2-redfish-client][PATCH] RedfishClientPkg/RedfishFeatureUtilityLib: validate string array
  2023-11-08  7:24 [edk2-devel] [edk2-redfish-client][PATCH] RedfishClientPkg/RedfishFeatureUtilityLib: validate string array Nickle Wang via groups.io
@ 2023-11-09 15:10 ` Igor Kulchytskyy via groups.io
  2023-12-04  9:45   ` Nickle Wang via groups.io
  0 siblings, 1 reply; 3+ messages in thread
From: Igor Kulchytskyy via groups.io @ 2023-11-09 15:10 UTC (permalink / raw)
  To: Nickle Wang, devel@edk2.groups.io; +Cc: Abner Chang, Nick Ramirez

Hi Nickle,
I noticed one typo. See below the text.

And I have a question regarding newly introduced ValidateRedfishStringArrayValues function.
You use that function to check if value in Redfish string array can be found in HII configuration string array.
And you still call CompareRedfishStringArrayValues function. Both iterate through ArrayHead and RedfishValue.Value.StringArray.
Maybe their functionality can be combined in ValidateRedfishStringArrayValues? Some additional an out parameter from that function can indicate If there is no change in array.
What do you think?
Thank you,
Igor


-----Original Message-----
From: Nickle Wang <nicklew@nvidia.com>
Sent: Wednesday, November 8, 2023 2:25 AM
To: devel@edk2.groups.io
Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@nvidia.com>
Subject: [EXTERNAL] [edk2-redfish-client][PATCH] RedfishClientPkg/RedfishFeatureUtilityLib: validate string array


**CAUTION: The e-mail below is from an external source. Please exercise caution before opening attachments, clicking links, or following guidance.**

- Add function to validate Redfish request for string array
type. There is case that user request invalid string array
and feature driver can not find corresponding HII option.
- Fix typo.

Signed-off-by: Nickle Wang <nicklew@nvidia.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nick Ramirez <nramirez@nvidia.com>
---
 .../Library/RedfishFeatureUtilityLib.h        |  56 +++++--
 .../RedfishFeatureUtilityLib.c                | 156 +++++++++++++-----
 2 files changed, 154 insertions(+), 58 deletions(-)

diff --git a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
index e2f728b2..440c4b68 100644
--- a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
+++ b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
@@ -22,7 +22,7 @@

   Read redfish resource by given resource URI.

-  @param[in]  Service       Redfish srvice instacne to make query.
+  @param[in]  Service       Redfish service instance to make query.
   @param[in]  ResourceUri   Target resource URI.
   @param[out] Response      HTTP response from redfish service.

@@ -47,7 +47,7 @@ GetResourceByUri (
   @param[out] ArraySignatureClose       String to the close of array signature.

   @retval     EFI_SUCCESS            Index is found.
-  @retval     EFI_NOT_FOUND          The non-array configure language string is retured.
+  @retval     EFI_NOT_FOUND          The non-array configure language string is returned.
   @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang is wrong.
   @retval     Others                 Errors occur.

@@ -118,10 +118,10 @@ GetNumberOfRedpathNodes (

   @param[in]  NodeString             The node string to parse.
   @param[in]  Index                  Index of the node.
-  @param[out] EndOfNodePtr           Pointer to receive the poitner to
+  @param[out] EndOfNodePtr           Pointer to receive the pointer to
                                      the last character of node string.

-  @retval     EFI_STRING             the begining of the node string.
+  @retval     EFI_STRING             the beginning of the node string.

 **/
 EFI_STRING
@@ -140,7 +140,7 @@ GetRedpathNodeByIndex (
   @param[out] Index                 The array index number.

   @retval     EFI_SUCCESS            Index is found.
-  @retval     EFI_NOT_FOUND          The non-array configure language string is retured.
+  @retval     EFI_NOT_FOUND          The non-array configure language string is returned.
   @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang is wrong.
   @retval     Others                 Errors occur.

@@ -188,7 +188,7 @@ DestroyConfiglanguageList (

   @param[in]  DestConfigLang        Pointer to the node's configure language string.
                                     The memory pointed by ConfigLang must be allocated
-                                    through memory allocation interface. Becasue we will replace
+                                    through memory allocation interface. Because we will replace
                                     the pointer in this function.
   @param[in]  MaxtLengthConfigLang  The maximum length of ConfigLang.
   @param[in]  ConfigLangInstance    Pointer to Collection member instance.
@@ -244,7 +244,7 @@ ApplyFeatureSettingsStringType (

 /**

-  Apply property value to UEFI HII database in numric type.
+  Apply property value to UEFI HII database in numeric type.

   @param[in]  Schema        Property schema.
   @param[in]  Version       Property schema version.
@@ -356,7 +356,7 @@ ApplyFeatureSettingsNumericArrayType (
   @param[in]  Schema        Property schema.
   @param[in]  Version       Property schema version.
   @param[in]  ConfigureLang Configure language refers to this property.
-  @param[in]  ArrayHead     Head of Redfich CS boolean array value.
+  @param[in]  ArrayHead     Head of Redfish CS boolean array value.

   @retval     EFI_SUCCESS     New value is applied successfully.
   @retval     Others          Errors occur.
@@ -421,7 +421,7 @@ CreatePayloadToPatchResource (
   @param[in]    ConfigLang        ConfigLang to save
   @param[in]    Uri               Redfish Uri to save

-  @retval  EFI_INVALID_PARAMETR   SystemId is NULL or EMPTY
+  @retval  EFI_INVALID_PARAMETER   SystemId is NULL or EMPTY
   @retval  EFI_SUCCESS            Redfish uri is saved

 **/
@@ -433,7 +433,7 @@ RedfisSetRedfishUri (

 /**

-  Get the property name by given Configure Langauge.
+  Get the property name by given Configure Language.

   @param[in]  ResourceUri              URI of root of resource.
   @param[in]  ConfigureLang            Configure Language string.
@@ -576,7 +576,7 @@ GetOdataId (

 /**

-  Return config language from given URI and prperty name. It's call responsibility to release returned buffer.
+  Return config language from given URI and property name. It's call responsibility to release returned buffer.

   @param[in] Uri            The URI to match
   @param[in] PropertyName   The property name of resource. This is optional.
@@ -790,7 +790,7 @@ MatchPropertyWithJsonContext (

 /**

-  Create string array and append to arry node in Redfish JSON convert format.
+  Create string array and append to array node in Redfish JSON convert format.

   @param[in,out]  Head          The head of string array.
   @param[in]      StringArray   Input string array.
@@ -809,7 +809,7 @@ AddRedfishCharArray (

 /**

-  Create numeric array and append to arry node in Redfish JSON convert format.
+  Create numeric array and append to array node in Redfish JSON convert format.

   @param[in,out]  Head           The head of string array.
   @param[in]      NumericArray   Input numeric array.
@@ -828,7 +828,7 @@ AddRedfishNumericArray (

 /**

-  Create boolean array and append to arry node in Redfish JSON convert format.
+  Create boolean array and append to array node in Redfish JSON convert format.

   @param[in,out]  Head           The head of string array.
   @param[in]      BooleanArray   Input boolean array.
@@ -866,12 +866,34 @@ CompareRedfishStringArrayValues (
   IN UINTN                 ArraySize
   );

+/**
+
+  Check and see if value in Redfish string array can be found in HII
+  configuration string array. This is to see if there is any invalid
+  values from Redfish.
+
+  @param[in]  Head          The head of string array.
+  @param[in]  StringArray   Input string array.
+  @param[in]  ArraySize     The size of StringArray.
+
+  @retval     TRUE          All string in Redfish array are as same as string

Igor: I think here we should have a plural of "string"
+  @retval     TRUE          All strings in Redfish array are as same as strings


+                            in HII configuration array.
+              FALSE         These two array are not identical.
+
+**/
+BOOLEAN
+ValidateRedfishStringArrayValues (
+  IN RedfishCS_char_Array  *Head,
+  IN CHAR8                 **StringArray,
+  IN UINTN                 ArraySize
+  );
+
 /**

   Check and see if value in Redfish numeric array are all the same as the one
   from HII configuration.

-  @param[in]  Head          The head of Redfish CS numeraic array.
+  @param[in]  Head          The head of Redfish CS numeric array.
   @param[in]  NumericArray  Input numeric array.
   @param[in]  ArraySize     The size of NumericArray.

@@ -914,9 +936,9 @@ CompareRedfishBooleanArrayValues (
   This is just a simple check.

   @param[in]  RedfishVagueKeyValuePtr     The vague key value sets on Redfish service.
-  @param[in]  RedfishVagueKeyValueNumber  The numebr of vague key value sets
+  @param[in]  RedfishVagueKeyValueNumber  The number of vague key value sets
   @param[in]  ConfigVagueKeyValuePtr      The vague configuration on platform.
-  @param[in]  ConfigVagueKeyValueNumber   The numebr of vague key value sets
+  @param[in]  ConfigVagueKeyValueNumber   The number of vague key value sets

   @retval     TRUE          All values are the same.
               FALSE         There is some difference.
diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
index 13e29902..a3ca1006 100644
--- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
+++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
@@ -772,61 +772,69 @@ ApplyFeatureSettingsStringArrayType (
   }

   //
-  // If there is no change in array, do nothing
+  // Check and see if element in request string array can be found in HII string array.
   //
-  if (!CompareRedfishStringArrayValues (ArrayHead, RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
-    //
-    // Apply settings from redfish
-    //
-    DEBUG ((DEBUG_MANAGEABILITY, "%a: %a.%a apply %s for array\n", __func__, Schema, Version, ConfigureLang));
-    FreeArrayTypeRedfishValue (&RedfishValue);
-
+  if (ValidateRedfishStringArrayValues (ArrayHead, RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
     //
-    // Convert array from RedfishCS_char_Array to EDKII_REDFISH_VALUE
+    // If there is no change in array, do nothing
     //
-    RedfishValue.ArrayCount = 0;
-    Buffer                  = ArrayHead;
-    while (Buffer != NULL) {
-      RedfishValue.ArrayCount += 1;
-      Buffer                   = Buffer->Next;
-    }
+    if (!CompareRedfishStringArrayValues (ArrayHead, RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
+      //
+      // Apply settings from redfish
+      //
+      DEBUG ((DEBUG_MANAGEABILITY, "%a: %a.%a apply %s for array\n", __func__, Schema, Version, ConfigureLang));
+      FreeArrayTypeRedfishValue (&RedfishValue);

-    //
-    // Allocate pool for new values
-    //
-    RedfishValue.Value.StringArray = AllocatePool (RedfishValue.ArrayCount *sizeof (CHAR8 *));
-    if (RedfishValue.Value.StringArray == NULL) {
-      ASSERT (FALSE);
-      return EFI_OUT_OF_RESOURCES;
-    }
+      //
+      // Convert array from RedfishCS_char_Array to EDKII_REDFISH_VALUE
+      //
+      RedfishValue.ArrayCount = 0;
+      Buffer                  = ArrayHead;
+      while (Buffer != NULL) {
+        RedfishValue.ArrayCount += 1;
+        Buffer                   = Buffer->Next;
+      }

-    Buffer = ArrayHead;
-    Index  = 0;
-    while (Buffer != NULL) {
-      RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize (Buffer->ArrayValue), Buffer->ArrayValue);
-      if (RedfishValue.Value.StringArray[Index] == NULL) {
+      //
+      // Allocate pool for new values
+      //
+      RedfishValue.Value.StringArray = AllocatePool (RedfishValue.ArrayCount *sizeof (CHAR8 *));
+      if (RedfishValue.Value.StringArray == NULL) {
         ASSERT (FALSE);
-        FreePool (RedfishValue.Value.StringArray);
         return EFI_OUT_OF_RESOURCES;
       }

-      Buffer = Buffer->Next;
-      Index++;
-    }
+      Buffer = ArrayHead;
+      Index  = 0;
+      while (Buffer != NULL) {
+        RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize (Buffer->ArrayValue), Buffer->ArrayValue);
+        if (RedfishValue.Value.StringArray[Index] == NULL) {
+          ASSERT (FALSE);
+          FreePool (RedfishValue.Value.StringArray);
+          return EFI_OUT_OF_RESOURCES;
+        }

-    ASSERT (Index <= RedfishValue.ArrayCount);
+        Buffer = Buffer->Next;
+        Index++;
+      }

-    Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang, RedfishValue);
-    if (!EFI_ERROR (Status)) {
-      //
-      // Configuration changed. Enable system reboot flag.
-      //
-      REDFISH_ENABLE_SYSTEM_REBOOT ();
+      ASSERT (Index <= RedfishValue.ArrayCount);
+
+      Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang, RedfishValue);
+      if (!EFI_ERROR (Status)) {
+        //
+        // Configuration changed. Enable system reboot flag.
+        //
+        REDFISH_ENABLE_SYSTEM_REBOOT ();
+      } else {
+        DEBUG ((DEBUG_ERROR, "%a: apply %s array failed: %r\n", __func__, ConfigureLang, Status));
+      }
     } else {
-      DEBUG ((DEBUG_ERROR, "%a: apply %s array failed: %r\n", __func__, ConfigureLang, Status));
+      DEBUG ((DEBUG_ERROR, "%a: %a.%a %s array value has no change\n", __func__, Schema, Version, ConfigureLang));
     }
   } else {
-    DEBUG ((DEBUG_ERROR, "%a: %a.%a %s array value has no change\n", __func__, Schema, Version, ConfigureLang));
+    DEBUG ((DEBUG_ERROR, "%a: %a.%a %s array value has invalid element, skip!\n", __func__, Schema, Version, ConfigureLang));
+    Status = EFI_DEVICE_ERROR;
   }

   for (Index = 0; Index < RedfishValue.ArrayCount; Index++) {
@@ -3432,6 +3440,72 @@ CompareRedfishStringArrayValues (
   return TRUE;
 }

+/**
+
+  Check and see if value in Redfish string array can be found in HII
+  configuration string array. This is to see if there is any invalid
+  values from Redfish.
+
+  @param[in]  Head          The head of string array.
+  @param[in]  StringArray   Input string array.
+  @param[in]  ArraySize     The size of StringArray.
+
+  @retval     TRUE          All string in Redfish array are as same as string
+                            in HII configuration array.
+              FALSE         These two array are not identical.
+
+**/
+BOOLEAN
+ValidateRedfishStringArrayValues (
+  IN RedfishCS_char_Array  *Head,
+  IN CHAR8                 **StringArray,
+  IN UINTN                 ArraySize
+  )
+{
+  UINTN                 Index;
+  UINTN                 InputArrayCount;
+  RedfishCS_char_Array  *CharArrayBuffer;
+
+  if ((Head == NULL) || (StringArray == NULL) || (ArraySize == 0)) {
+    return FALSE;
+  }
+
+  //
+  // Check to see if string from Redfish can be found in string array
+  // returned by HII or not. If not, the input from Redfish is invalid.
+  //
+  CharArrayBuffer = Head;
+  Index           = 0;
+  InputArrayCount = 0;
+  while (CharArrayBuffer != NULL) {
+    for (Index = 0; Index < ArraySize; Index++) {
+      if (AsciiStrCmp (StringArray[Index], CharArrayBuffer->ArrayValue) == 0) {
+        break;
+      }
+    }
+
+    if (Index == ArraySize) {
+      DEBUG ((DEBUG_ERROR, "%a: input string: %a is not found in HII string list\n", __func__, CharArrayBuffer->ArrayValue));
+      return FALSE;
+    }
+
+    InputArrayCount += 1;
+    CharArrayBuffer  = CharArrayBuffer->Next;
+  }
+
+  //
+  // Check to see if the number of string from Redfish equals to the
+  // number of string returned by HII. HII only accepts the same
+  // number of string array due to the design or HII ordered list.
+  //
+  if (InputArrayCount != ArraySize) {
+    DEBUG ((DEBUG_ERROR, "%a: input string size: %d is not the same as HII string list size: %d\n", __func__, InputArrayCount, ArraySize));
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
 /**

   Check and see if value in Redfish numeric array are all the same as the one
--
2.17.1

-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110989): https://edk2.groups.io/g/devel/message/110989
Mute This Topic: https://groups.io/mt/102459779/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [edk2-redfish-client][PATCH] RedfishClientPkg/RedfishFeatureUtilityLib: validate string array
  2023-11-09 15:10 ` Igor Kulchytskyy via groups.io
@ 2023-12-04  9:45   ` Nickle Wang via groups.io
  0 siblings, 0 replies; 3+ messages in thread
From: Nickle Wang via groups.io @ 2023-12-04  9:45 UTC (permalink / raw)
  To: Igor Kulchytskyy, devel@edk2.groups.io; +Cc: Abner Chang, Nick Ramirez

Hi Igor,

Sorry for my delay response. Having three checks in ValidateRedfishStringArrayValues is complicate to me so I took some time to verify it on my system.

> Maybe their functionality can be combined in ValidateRedfishStringArrayValues?
> Some additional an out parameter from that function can indicate If there is no
> change in array.

This is good idea! I update ValidateRedfishStringArrayValues in version 2 patch file here: https://edk2.groups.io/g/devel/message/112025  PR: https://github.com/tianocore/edk2-redfish-client/pull/58 Please help me to check them if you have time.

Thanks,
Nickle

> -----Original Message-----
> From: Igor Kulchytskyy <igork@ami.com>
> Sent: Thursday, November 9, 2023 11:11 PM
> To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io
> Cc: Abner Chang <abner.chang@amd.com>; Nick Ramirez
> <nramirez@nvidia.com>
> Subject: RE: [EXTERNAL] [edk2-redfish-client][PATCH]
> RedfishClientPkg/RedfishFeatureUtilityLib: validate string array
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Nickle,
> I noticed one typo. See below the text.
> 
> And I have a question regarding newly introduced
> ValidateRedfishStringArrayValues function.
> You use that function to check if value in Redfish string array can be found in HII
> configuration string array.
> And you still call CompareRedfishStringArrayValues function. Both iterate through
> ArrayHead and RedfishValue.Value.StringArray.
> Maybe their functionality can be combined in ValidateRedfishStringArrayValues?
> Some additional an out parameter from that function can indicate If there is no
> change in array.
> What do you think?
> Thank you,
> Igor
> 
> 
> -----Original Message-----
> From: Nickle Wang <nicklew@nvidia.com>
> Sent: Wednesday, November 8, 2023 2:25 AM
> To: devel@edk2.groups.io
> Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy <igork@ami.com>;
> Nick Ramirez <nramirez@nvidia.com>
> Subject: [EXTERNAL] [edk2-redfish-client][PATCH]
> RedfishClientPkg/RedfishFeatureUtilityLib: validate string array
> 
> 
> **CAUTION: The e-mail below is from an external source. Please exercise
> caution before opening attachments, clicking links, or following guidance.**
> 
> - Add function to validate Redfish request for string array type. There is case that
> user request invalid string array and feature driver can not find corresponding HII
> option.
> - Fix typo.
> 
> Signed-off-by: Nickle Wang <nicklew@nvidia.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Nick Ramirez <nramirez@nvidia.com>
> ---
>  .../Library/RedfishFeatureUtilityLib.h        |  56 +++++--
>  .../RedfishFeatureUtilityLib.c                | 156 +++++++++++++-----
>  2 files changed, 154 insertions(+), 58 deletions(-)
> 
> diff --git a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> index e2f728b2..440c4b68 100644
> --- a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> +++ b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> @@ -22,7 +22,7 @@
> 
>    Read redfish resource by given resource URI.
> 
> -  @param[in]  Service       Redfish srvice instacne to make query.
> +  @param[in]  Service       Redfish service instance to make query.
>    @param[in]  ResourceUri   Target resource URI.
>    @param[out] Response      HTTP response from redfish service.
> 
> @@ -47,7 +47,7 @@ GetResourceByUri (
>    @param[out] ArraySignatureClose       String to the close of array signature.
> 
>    @retval     EFI_SUCCESS            Index is found.
> -  @retval     EFI_NOT_FOUND          The non-array configure language string is
> retured.
> +  @retval     EFI_NOT_FOUND          The non-array configure language string is
> returned.
>    @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang is
> wrong.
>    @retval     Others                 Errors occur.
> 
> @@ -118,10 +118,10 @@ GetNumberOfRedpathNodes (
> 
>    @param[in]  NodeString             The node string to parse.
>    @param[in]  Index                  Index of the node.
> -  @param[out] EndOfNodePtr           Pointer to receive the poitner to
> +  @param[out] EndOfNodePtr           Pointer to receive the pointer to
>                                       the last character of node string.
> 
> -  @retval     EFI_STRING             the begining of the node string.
> +  @retval     EFI_STRING             the beginning of the node string.
> 
>  **/
>  EFI_STRING
> @@ -140,7 +140,7 @@ GetRedpathNodeByIndex (
>    @param[out] Index                 The array index number.
> 
>    @retval     EFI_SUCCESS            Index is found.
> -  @retval     EFI_NOT_FOUND          The non-array configure language string is
> retured.
> +  @retval     EFI_NOT_FOUND          The non-array configure language string is
> returned.
>    @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang is
> wrong.
>    @retval     Others                 Errors occur.
> 
> @@ -188,7 +188,7 @@ DestroyConfiglanguageList (
> 
>    @param[in]  DestConfigLang        Pointer to the node's configure language
> string.
>                                      The memory pointed by ConfigLang must be allocated
> -                                    through memory allocation interface. Becasue we will
> replace
> +                                    through memory allocation
> + interface. Because we will replace
>                                      the pointer in this function.
>    @param[in]  MaxtLengthConfigLang  The maximum length of ConfigLang.
>    @param[in]  ConfigLangInstance    Pointer to Collection member instance.
> @@ -244,7 +244,7 @@ ApplyFeatureSettingsStringType (
> 
>  /**
> 
> -  Apply property value to UEFI HII database in numric type.
> +  Apply property value to UEFI HII database in numeric type.
> 
>    @param[in]  Schema        Property schema.
>    @param[in]  Version       Property schema version.
> @@ -356,7 +356,7 @@ ApplyFeatureSettingsNumericArrayType (
>    @param[in]  Schema        Property schema.
>    @param[in]  Version       Property schema version.
>    @param[in]  ConfigureLang Configure language refers to this property.
> -  @param[in]  ArrayHead     Head of Redfich CS boolean array value.
> +  @param[in]  ArrayHead     Head of Redfish CS boolean array value.
> 
>    @retval     EFI_SUCCESS     New value is applied successfully.
>    @retval     Others          Errors occur.
> @@ -421,7 +421,7 @@ CreatePayloadToPatchResource (
>    @param[in]    ConfigLang        ConfigLang to save
>    @param[in]    Uri               Redfish Uri to save
> 
> -  @retval  EFI_INVALID_PARAMETR   SystemId is NULL or EMPTY
> +  @retval  EFI_INVALID_PARAMETER   SystemId is NULL or EMPTY
>    @retval  EFI_SUCCESS            Redfish uri is saved
> 
>  **/
> @@ -433,7 +433,7 @@ RedfisSetRedfishUri (
> 
>  /**
> 
> -  Get the property name by given Configure Langauge.
> +  Get the property name by given Configure Language.
> 
>    @param[in]  ResourceUri              URI of root of resource.
>    @param[in]  ConfigureLang            Configure Language string.
> @@ -576,7 +576,7 @@ GetOdataId (
> 
>  /**
> 
> -  Return config language from given URI and prperty name. It's call responsibility
> to release returned buffer.
> +  Return config language from given URI and property name. It's call
> responsibility to release returned buffer.
> 
>    @param[in] Uri            The URI to match
>    @param[in] PropertyName   The property name of resource. This is optional.
> @@ -790,7 +790,7 @@ MatchPropertyWithJsonContext (
> 
>  /**
> 
> -  Create string array and append to arry node in Redfish JSON convert format.
> +  Create string array and append to array node in Redfish JSON convert format.
> 
>    @param[in,out]  Head          The head of string array.
>    @param[in]      StringArray   Input string array.
> @@ -809,7 +809,7 @@ AddRedfishCharArray (
> 
>  /**
> 
> -  Create numeric array and append to arry node in Redfish JSON convert format.
> +  Create numeric array and append to array node in Redfish JSON convert
> format.
> 
>    @param[in,out]  Head           The head of string array.
>    @param[in]      NumericArray   Input numeric array.
> @@ -828,7 +828,7 @@ AddRedfishNumericArray (
> 
>  /**
> 
> -  Create boolean array and append to arry node in Redfish JSON convert format.
> +  Create boolean array and append to array node in Redfish JSON convert format.
> 
>    @param[in,out]  Head           The head of string array.
>    @param[in]      BooleanArray   Input boolean array.
> @@ -866,12 +866,34 @@ CompareRedfishStringArrayValues (
>    IN UINTN                 ArraySize
>    );
> 
> +/**
> +
> +  Check and see if value in Redfish string array can be found in HII
> + configuration string array. This is to see if there is any invalid
> + values from Redfish.
> +
> +  @param[in]  Head          The head of string array.
> +  @param[in]  StringArray   Input string array.
> +  @param[in]  ArraySize     The size of StringArray.
> +
> +  @retval     TRUE          All string in Redfish array are as same as string
> 
> Igor: I think here we should have a plural of "string"
> +  @retval     TRUE          All strings in Redfish array are as same as strings
> 
> 
> +                            in HII configuration array.
> +              FALSE         These two array are not identical.
> +
> +**/
> +BOOLEAN
> +ValidateRedfishStringArrayValues (
> +  IN RedfishCS_char_Array  *Head,
> +  IN CHAR8                 **StringArray,
> +  IN UINTN                 ArraySize
> +  );
> +
>  /**
> 
>    Check and see if value in Redfish numeric array are all the same as the one
>    from HII configuration.
> 
> -  @param[in]  Head          The head of Redfish CS numeraic array.
> +  @param[in]  Head          The head of Redfish CS numeric array.
>    @param[in]  NumericArray  Input numeric array.
>    @param[in]  ArraySize     The size of NumericArray.
> 
> @@ -914,9 +936,9 @@ CompareRedfishBooleanArrayValues (
>    This is just a simple check.
> 
>    @param[in]  RedfishVagueKeyValuePtr     The vague key value sets on Redfish
> service.
> -  @param[in]  RedfishVagueKeyValueNumber  The numebr of vague key value
> sets
> +  @param[in]  RedfishVagueKeyValueNumber  The number of vague key value
> + sets
>    @param[in]  ConfigVagueKeyValuePtr      The vague configuration on platform.
> -  @param[in]  ConfigVagueKeyValueNumber   The numebr of vague key value
> sets
> +  @param[in]  ConfigVagueKeyValueNumber   The number of vague key value
> sets
> 
>    @retval     TRUE          All values are the same.
>                FALSE         There is some difference.
> diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> index 13e29902..a3ca1006 100644
> ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> +++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUt
> +++ ilityLib.c
> @@ -772,61 +772,69 @@ ApplyFeatureSettingsStringArrayType (
>    }
> 
>    //
> -  // If there is no change in array, do nothing
> +  // Check and see if element in request string array can be found in HII string
> array.
>    //
> -  if (!CompareRedfishStringArrayValues (ArrayHead,
> RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
> -    //
> -    // Apply settings from redfish
> -    //
> -    DEBUG ((DEBUG_MANAGEABILITY, "%a: %a.%a apply %s for array\n",
> __func__, Schema, Version, ConfigureLang));
> -    FreeArrayTypeRedfishValue (&RedfishValue);
> -
> +  if (ValidateRedfishStringArrayValues (ArrayHead,
> + RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
>      //
> -    // Convert array from RedfishCS_char_Array to EDKII_REDFISH_VALUE
> +    // If there is no change in array, do nothing
>      //
> -    RedfishValue.ArrayCount = 0;
> -    Buffer                  = ArrayHead;
> -    while (Buffer != NULL) {
> -      RedfishValue.ArrayCount += 1;
> -      Buffer                   = Buffer->Next;
> -    }
> +    if (!CompareRedfishStringArrayValues (ArrayHead,
> RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
> +      //
> +      // Apply settings from redfish
> +      //
> +      DEBUG ((DEBUG_MANAGEABILITY, "%a: %a.%a apply %s for array\n",
> __func__, Schema, Version, ConfigureLang));
> +      FreeArrayTypeRedfishValue (&RedfishValue);
> 
> -    //
> -    // Allocate pool for new values
> -    //
> -    RedfishValue.Value.StringArray = AllocatePool (RedfishValue.ArrayCount
> *sizeof (CHAR8 *));
> -    if (RedfishValue.Value.StringArray == NULL) {
> -      ASSERT (FALSE);
> -      return EFI_OUT_OF_RESOURCES;
> -    }
> +      //
> +      // Convert array from RedfishCS_char_Array to EDKII_REDFISH_VALUE
> +      //
> +      RedfishValue.ArrayCount = 0;
> +      Buffer                  = ArrayHead;
> +      while (Buffer != NULL) {
> +        RedfishValue.ArrayCount += 1;
> +        Buffer                   = Buffer->Next;
> +      }
> 
> -    Buffer = ArrayHead;
> -    Index  = 0;
> -    while (Buffer != NULL) {
> -      RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize
> (Buffer->ArrayValue), Buffer->ArrayValue);
> -      if (RedfishValue.Value.StringArray[Index] == NULL) {
> +      //
> +      // Allocate pool for new values
> +      //
> +      RedfishValue.Value.StringArray = AllocatePool (RedfishValue.ArrayCount
> *sizeof (CHAR8 *));
> +      if (RedfishValue.Value.StringArray == NULL) {
>          ASSERT (FALSE);
> -        FreePool (RedfishValue.Value.StringArray);
>          return EFI_OUT_OF_RESOURCES;
>        }
> 
> -      Buffer = Buffer->Next;
> -      Index++;
> -    }
> +      Buffer = ArrayHead;
> +      Index  = 0;
> +      while (Buffer != NULL) {
> +        RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize
> (Buffer->ArrayValue), Buffer->ArrayValue);
> +        if (RedfishValue.Value.StringArray[Index] == NULL) {
> +          ASSERT (FALSE);
> +          FreePool (RedfishValue.Value.StringArray);
> +          return EFI_OUT_OF_RESOURCES;
> +        }
> 
> -    ASSERT (Index <= RedfishValue.ArrayCount);
> +        Buffer = Buffer->Next;
> +        Index++;
> +      }
> 
> -    Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang,
> RedfishValue);
> -    if (!EFI_ERROR (Status)) {
> -      //
> -      // Configuration changed. Enable system reboot flag.
> -      //
> -      REDFISH_ENABLE_SYSTEM_REBOOT ();
> +      ASSERT (Index <= RedfishValue.ArrayCount);
> +
> +      Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang,
> RedfishValue);
> +      if (!EFI_ERROR (Status)) {
> +        //
> +        // Configuration changed. Enable system reboot flag.
> +        //
> +        REDFISH_ENABLE_SYSTEM_REBOOT ();
> +      } else {
> +        DEBUG ((DEBUG_ERROR, "%a: apply %s array failed: %r\n", __func__,
> ConfigureLang, Status));
> +      }
>      } else {
> -      DEBUG ((DEBUG_ERROR, "%a: apply %s array failed: %r\n", __func__,
> ConfigureLang, Status));
> +      DEBUG ((DEBUG_ERROR, "%a: %a.%a %s array value has no change\n",
> + __func__, Schema, Version, ConfigureLang));
>      }
>    } else {
> -    DEBUG ((DEBUG_ERROR, "%a: %a.%a %s array value has no change\n",
> __func__, Schema, Version, ConfigureLang));
> +    DEBUG ((DEBUG_ERROR, "%a: %a.%a %s array value has invalid element,
> skip!\n", __func__, Schema, Version, ConfigureLang));
> +    Status = EFI_DEVICE_ERROR;
>    }
> 
>    for (Index = 0; Index < RedfishValue.ArrayCount; Index++) { @@ -3432,6
> +3440,72 @@ CompareRedfishStringArrayValues (
>    return TRUE;
>  }
> 
> +/**
> +
> +  Check and see if value in Redfish string array can be found in HII
> + configuration string array. This is to see if there is any invalid
> + values from Redfish.
> +
> +  @param[in]  Head          The head of string array.
> +  @param[in]  StringArray   Input string array.
> +  @param[in]  ArraySize     The size of StringArray.
> +
> +  @retval     TRUE          All string in Redfish array are as same as string
> +                            in HII configuration array.
> +              FALSE         These two array are not identical.
> +
> +**/
> +BOOLEAN
> +ValidateRedfishStringArrayValues (
> +  IN RedfishCS_char_Array  *Head,
> +  IN CHAR8                 **StringArray,
> +  IN UINTN                 ArraySize
> +  )
> +{
> +  UINTN                 Index;
> +  UINTN                 InputArrayCount;
> +  RedfishCS_char_Array  *CharArrayBuffer;
> +
> +  if ((Head == NULL) || (StringArray == NULL) || (ArraySize == 0)) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check to see if string from Redfish can be found in string array
> + // returned by HII or not. If not, the input from Redfish is invalid.
> +  //
> +  CharArrayBuffer = Head;
> +  Index           = 0;
> +  InputArrayCount = 0;
> +  while (CharArrayBuffer != NULL) {
> +    for (Index = 0; Index < ArraySize; Index++) {
> +      if (AsciiStrCmp (StringArray[Index], CharArrayBuffer->ArrayValue) == 0) {
> +        break;
> +      }
> +    }
> +
> +    if (Index == ArraySize) {
> +      DEBUG ((DEBUG_ERROR, "%a: input string: %a is not found in HII string
> list\n", __func__, CharArrayBuffer->ArrayValue));
> +      return FALSE;
> +    }
> +
> +    InputArrayCount += 1;
> +    CharArrayBuffer  = CharArrayBuffer->Next;  }
> +
> +  //
> +  // Check to see if the number of string from Redfish equals to the
> + // number of string returned by HII. HII only accepts the same  //
> + number of string array due to the design or HII ordered list.
> +  //
> +  if (InputArrayCount != ArraySize) {
> +    DEBUG ((DEBUG_ERROR, "%a: input string size: %d is not the same as HII
> string list size: %d\n", __func__, InputArrayCount, ArraySize));
> +    return FALSE;
> +  }
> +
> +  return TRUE;
> +}
> +
>  /**
> 
>    Check and see if value in Redfish numeric array are all the same as the one
> --
> 2.17.1
> 
> -The information contained in this message may be confidential and proprietary
> to American Megatrends (AMI). This communication is intended to be read only by
> the individual or entity to whom it is addressed or by their designee. If the reader
> of this message is not the intended recipient, you are on notice that any
> distribution of this message, in any form, is strictly prohibited. Please promptly
> notify the sender by reply e-mail or by telephone at 770-246-8600, and then
> delete or destroy all copies of the transmission.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112026): https://edk2.groups.io/g/devel/message/112026
Mute This Topic: https://groups.io/mt/102459779/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-12-04  9:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-08  7:24 [edk2-devel] [edk2-redfish-client][PATCH] RedfishClientPkg/RedfishFeatureUtilityLib: validate string array Nickle Wang via groups.io
2023-11-09 15:10 ` Igor Kulchytskyy via groups.io
2023-12-04  9:45   ` Nickle Wang via groups.io

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox