[PATCH 0/2] Fix two bugs regarding shell redirection

[PATCH 0/2] Fix two bugs regarding shell redirection

[Ruiyu Ni]

Ruiyu Ni (2):
  ShellPkg/redirection: Insert \xFEFF after converting ASCII to Unicode
  ShellPkg/redirection: Insert \xFEFF for ENV variable redirection

 ShellPkg/Application/Shell/FileHandleWrappers.c | 80 ++++++++++++++++++-------
 1 file changed, 58 insertions(+), 22 deletions(-)

-- 
2.16.1.windows.1

[PATCH 1/2] ShellPkg/redirection: Insert \xFEFF after converting ASCII to Unicode

[Ruiyu Ni]

When "<a" is used to redirect ASCII file to an application, Shell
core reads the ASCII file and converts the ASCII to Unicode as the
input source of the application.
But per Shell spec, the input source should have \xFEFF to indicate
it's a Unicode stream.
The patch adds the missing \xFEFF.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
---
 ShellPkg/Application/Shell/FileHandleWrappers.c | 46 +++++++++++++++++--------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/ShellPkg/Application/Shell/FileHandleWrappers.c b/ShellPkg/Application/Shell/FileHandleWrappers.c
index 8c62eb5862..655854b25d 100644
--- a/ShellPkg/Application/Shell/FileHandleWrappers.c
+++ b/ShellPkg/Application/Shell/FileHandleWrappers.c
@@ -1924,42 +1924,58 @@ FileInterfaceFileRead(
   OUT VOID                    *Buffer
   )
 {
+  EFI_STATUS  Status;
+  UINT64      Position;
   CHAR8       *AsciiStrBuffer;
   CHAR16      *UscStrBuffer;
   UINTN       Size;
-  UINTN       CharNum;
-  EFI_STATUS  Status;
   if (((EFI_FILE_PROTOCOL_FILE*)This)->Unicode) {
     //
     // Unicode
+    // There might be different file tag for the Unicode file. We cannot unconditionally insert the \xFEFF.
+    // So we choose to leave the file content as is.
     //
     return (((EFI_FILE_PROTOCOL_FILE*)This)->Orig->Read(((EFI_FILE_PROTOCOL_FILE*)This)->Orig, BufferSize, Buffer));
   } else {
     //
     // Ascii
     //
-    Size  = (*BufferSize) / sizeof(CHAR16);
-    AsciiStrBuffer = AllocateZeroPool(Size + sizeof(CHAR8));
+    *BufferSize = *BufferSize / sizeof (CHAR16) * sizeof (CHAR16);
+    if (*BufferSize == 0) {
+      return EFI_SUCCESS;
+    }
+    Status = ((EFI_FILE_PROTOCOL_FILE*)This)->Orig->GetPosition (((EFI_FILE_PROTOCOL_FILE*)This)->Orig, &Position);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+    if (Position == 0) {
+      //
+      // First two bytes in Buffer is for the Unicode file tag.
+      //
+      *(CHAR16 *)Buffer = gUnicodeFileTag;
+      Buffer = (CHAR16 *)Buffer + 1;
+      Size   = *BufferSize / sizeof (CHAR16) - 1;
+    } else {
+      Size   = *BufferSize / sizeof (CHAR16);
+    }
+    AsciiStrBuffer = AllocateZeroPool (Size + 1);
     if (AsciiStrBuffer == NULL) {
       return EFI_OUT_OF_RESOURCES;
     }
-    UscStrBuffer = AllocateZeroPool(*BufferSize + sizeof(CHAR16));
+    UscStrBuffer = AllocateZeroPool ((Size + 1) * sizeof(CHAR16));
     if (UscStrBuffer== NULL) {
       SHELL_FREE_NON_NULL(AsciiStrBuffer);
       return EFI_OUT_OF_RESOURCES;
     }
-    Status = (((EFI_FILE_PROTOCOL_FILE*)This)->Orig->Read(((EFI_FILE_PROTOCOL_FILE*)This)->Orig, &Size, AsciiStrBuffer));
+    Status = ((EFI_FILE_PROTOCOL_FILE*)This)->Orig->Read (((EFI_FILE_PROTOCOL_FILE*)This)->Orig, &Size, AsciiStrBuffer);
     if (!EFI_ERROR(Status)) {
-      CharNum = UnicodeSPrint(UscStrBuffer, *BufferSize + sizeof(CHAR16), L"%a", AsciiStrBuffer);
-      if (CharNum == Size) {
-        CopyMem (Buffer, UscStrBuffer, *BufferSize);
-      } else {
-        Status = EFI_UNSUPPORTED;
-      }
+      AsciiStrToUnicodeStrS (AsciiStrBuffer, UscStrBuffer, Size + 1);
+      *BufferSize = Size * sizeof (CHAR16);
+      CopyMem (Buffer, UscStrBuffer, *BufferSize);
     }
-    SHELL_FREE_NON_NULL(AsciiStrBuffer);
-    SHELL_FREE_NON_NULL(UscStrBuffer);
-    return (Status);
+    SHELL_FREE_NON_NULL (AsciiStrBuffer);
+    SHELL_FREE_NON_NULL (UscStrBuffer);
+    return Status;
   }
 }
 
-- 
2.16.1.windows.1

[PATCH 2/2] ShellPkg/redirection: Insert \xFEFF for ENV variable redirection

[Ruiyu Ni]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1080

Per Shell spec 2.2 chapter 3.4.4.2, Unicode file tag should be
inserted in the output from the input redirected variable, to ensure
it looks like a UCS-2 encode file.

The patch fixes this issue.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
---
 ShellPkg/Application/Shell/FileHandleWrappers.c | 34 ++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/ShellPkg/Application/Shell/FileHandleWrappers.c b/ShellPkg/Application/Shell/FileHandleWrappers.c
index 655854b25d..bcce055321 100644
--- a/ShellPkg/Application/Shell/FileHandleWrappers.c
+++ b/ShellPkg/Application/Shell/FileHandleWrappers.c
@@ -1148,15 +1148,35 @@ FileInterfaceEnvDelete(
 EFI_STATUS
 EFIAPI
 FileInterfaceEnvRead(
-  IN EFI_FILE_PROTOCOL *This,
-  IN OUT UINTN *BufferSize,
-  OUT VOID *Buffer
+  IN     EFI_FILE_PROTOCOL *This,
+  IN OUT UINTN             *BufferSize,
+  OUT    VOID              *Buffer
   )
 {
-  return (SHELL_GET_ENVIRONMENT_VARIABLE(
-    ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
-    BufferSize,
-    Buffer));
+  EFI_STATUS     Status;
+
+  *BufferSize = *BufferSize / sizeof (CHAR16) * sizeof (CHAR16);
+  if (*BufferSize != 0) {
+    //
+    // Make sure the first unicode character is \xFEFF
+    //
+    *(CHAR16 *)Buffer = gUnicodeFileTag;
+    Buffer            = (CHAR16 *)Buffer + 1;
+    *BufferSize      -= sizeof (gUnicodeFileTag);
+  }
+
+  Status = SHELL_GET_ENVIRONMENT_VARIABLE (
+             ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+             BufferSize,
+             Buffer
+             );
+  if (!EFI_ERROR (Status) || (Status == EFI_BUFFER_TOO_SMALL)) {
+    //
+    // BufferSize is valid and needs update when Status is Success or BufferTooSmall.
+    //
+    *BufferSize += sizeof (gUnicodeFileTag);
+  }
+  return Status;
 }
 
 /**
-- 
2.16.1.windows.1

Re: [PATCH 0/2] Fix two bugs regarding shell redirection

[Carsey, Jaben]

Both look good.
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ruiyu Ni
> Sent: Wednesday, August 08, 2018 8:45 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/2] Fix two bugs regarding shell redirection
> Importance: High
> 
> Ruiyu Ni (2):
>   ShellPkg/redirection: Insert \xFEFF after converting ASCII to Unicode
>   ShellPkg/redirection: Insert \xFEFF for ENV variable redirection
> 
>  ShellPkg/Application/Shell/FileHandleWrappers.c | 80
> ++++++++++++++++++-------
>  1 file changed, 58 insertions(+), 22 deletions(-)
> 
> --
> 2.16.1.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel