Apphost: Handle long paths while reading bundles (dotnet/core-setup#5948)
authorSwaroop Sridhar <Swaroop.Sridhar@microsoft.com>
Sat, 20 Apr 2019 00:21:45 +0000 (17:21 -0700)
committerGitHub <noreply@github.com>
Sat, 20 Apr 2019 00:21:45 +0000 (17:21 -0700)
Handle path-lengths larger than 127 bytes in:
- Bundle ID (also a component of the extraction path)
- relative-path (path of a file within the bundle)

The above strings are encoded as length prefixed strings by System.IO.BinaryWriter.Write()
The length itself is encoded as a series of bytes where
  - 7-bits contain length bits
  - 8th bit indicates whether there are more bytes to follow.

For a path, there can be a maximum of two bytes (max 4096)
This commit handles the case where a path length is long enough to be need two bytes.

Also added a test case.

Fixes dotnet/core-setup#5932

Commit migrated from https://github.com/dotnet/core-setup/commit/9889b387cff5086f09ef174c5e73c278d3dba208

src/installer/corehost/cli/apphost/bundle/bundle_runner.cpp
src/installer/corehost/cli/apphost/bundle/bundle_runner.h
src/installer/corehost/cli/apphost/bundle/file_entry.cpp
src/installer/corehost/cli/apphost/bundle/file_entry.h
src/installer/corehost/cli/apphost/bundle/manifest.cpp
src/installer/corehost/cli/apphost/bundle/manifest.h
src/installer/test/Assets/TestProjects/StandaloneAppWithSubDirs/Program.cs
src/installer/test/Assets/TestProjects/StandaloneAppWithSubDirs/Sentence/Noun/Pronoun/Another/word
src/installer/test/Assets/TestProjects/StandaloneAppWithSubDirs/Sentence/This is a really, really, really, really, really, really, really, really, really, really, really, really, really, really long file name for punctuation/word [new file with mode: 0644]
src/installer/test/Assets/TestProjects/StandaloneAppWithSubDirs/StandaloneAppWithSubDirs.csproj

index 7c26d3f..3b6f8b3 100644 (file)
@@ -39,6 +39,44 @@ void bundle_runner_t::read(void* buf, size_t size, FILE* stream)
     }
 }
 
+// Handle the relatively uncommon scenario where the bundle ID or 
+// the relative-path of a file within the bundle is longer than 127 bytes
+size_t bundle_runner_t::get_path_length(int8_t first_byte, FILE* stream)
+{
+    size_t length = 0;
+
+    // If the high bit is set, it means there are more bytes to read.
+    if ((first_byte & 0x80) == 0)
+    {
+         length = first_byte;
+    }
+    else
+    {
+        int8_t second_byte = 0;
+        read(&second_byte, 1, stream);
+
+        if (second_byte & 0x80)
+        {
+            // There can be no more than two bytes in path_length
+            trace::error(_X("Failure processing application bundle; possible file corruption."));
+            trace::error(_X("Path length encoding read beyond two bytes"));
+
+            throw StatusCode::BundleExtractionFailure;
+        }
+
+        length = (second_byte << 7) | (first_byte & 0x7f);
+    }
+
+    if (length <= 0 || length > PATH_MAX)
+    {
+        trace::error(_X("Failure processing application bundle; possible file corruption."));
+        trace::error(_X("Path length is zero or too long"));
+        throw StatusCode::BundleExtractionFailure;
+    }
+
+    return length;
+}
+
 // Read a non-null terminated fixed length UTF8 string from a byte-stream
 // and transform it to pal::string_t
 void bundle_runner_t::read_string(pal::string_t &str, size_t size, FILE* stream)
@@ -218,12 +256,12 @@ FILE* bundle_runner_t::create_extraction_file(const pal::string_t& relative_path
 // Extract one file from the bundle to disk.
 void bundle_runner_t::extract_file(file_entry_t *entry)
 {
-    FILE* file = create_extraction_file(entry->relative_path);
+    FILE* file = create_extraction_file(entry->relative_path());
     const size_t buffer_size = 8 * 1024; // Copy the file in 8KB chunks
     uint8_t buffer[buffer_size];
-    int64_t file_size = entry->data.size;
+    int64_t file_size = entry->size();
 
-    seek(m_bundle_stream, entry->data.offset, SEEK_SET);
+    seek(m_bundle_stream, entry->offset(), SEEK_SET);
     do {
         int64_t copy_size = (file_size <= buffer_size) ? file_size : buffer_size;
         read(buffer, copy_size, m_bundle_stream);
index 3821227..2ca63e2 100644 (file)
@@ -32,6 +32,7 @@ namespace bundle
 
         static void read(void* buf, size_t size, FILE* stream);
         static void write(const void* buf, size_t size, FILE* stream);
+        static size_t get_path_length(int8_t first_byte, FILE* stream);
         static void read_string(pal::string_t& str, size_t size, FILE* stream);
 
     private:
index 437f1eb..ca08ee6 100644 (file)
@@ -12,9 +12,8 @@ using namespace bundle;
 
 bool file_entry_t::is_valid()
 {
-    return data.offset > 0 && data.size > 0 &&
-        (file_type_t)data.type < file_type_t::__last &&
-        data.path_length > 0 && data.path_length <= PATH_MAX;
+    return m_data.offset > 0 && m_data.size > 0 &&
+        (file_type_t)m_data.type < file_type_t::__last;
 }
 
 file_entry_t* file_entry_t::read(FILE* stream)
@@ -22,7 +21,7 @@ file_entry_t* file_entry_t::read(FILE* stream)
     file_entry_t* entry = new file_entry_t();
 
     // First read the fixed-sized portion of file-entry
-    bundle_runner_t::read(&entry->data, sizeof(entry->data), stream);
+    bundle_runner_t::read(&entry->m_data, sizeof(entry->m_data), stream);
     if (!entry->is_valid())
     {
         trace::error(_X("Failure processing application bundle; possible file corruption."));
@@ -30,9 +29,12 @@ file_entry_t* file_entry_t::read(FILE* stream)
         throw StatusCode::BundleExtractionFailure;
     }
 
+    size_t path_length =
+        bundle_runner_t::get_path_length(entry->m_data.path_length_byte_1, stream);
+
     // Read the relative-path, given its length 
-    pal::string_t& path = entry->relative_path;
-    bundle_runner_t::read_string(path, entry->data.path_length, stream);
+    pal::string_t& path = entry->m_relative_path;
+    bundle_runner_t::read_string(path, path_length, stream);
 
     // Fixup the relative-path to have current platform's directory separator.
     if (bundle_dir_separator != DIR_SEPARATOR)
index 3d9520d..15900c2 100644 (file)
@@ -26,7 +26,7 @@ namespace bundle
 
     class file_entry_t
     {
-    public:
+    private:
 
         // The inner structure represents the fields that can be 
         // read contiguously for every file_entry. 
@@ -36,16 +36,23 @@ namespace bundle
             int64_t offset;
             int64_t size;
             file_type_t type;
-            int8_t path_length;
-        } data;
+            int8_t path_length_byte_1;
+        } m_data;
 #pragma pack(pop)
-        pal::string_t relative_path; // Path of an embedded file, relative to the extraction directory.
 
+        pal::string_t m_relative_path; // Path of an embedded file, relative to the extraction directory.
+
+    public:
         file_entry_t()
-            :data(), relative_path()
+            :m_data(), m_relative_path()
         {
         }
 
+        const pal::string_t& relative_path() { return m_relative_path; }
+        int64_t offset() { return m_data.offset; }
+        int64_t size() { return m_data.size; }
+        file_type_t type() { return m_data.type; }
+
         static file_entry_t* read(FILE* stream);
 
     private:
index 3c38a4a..d8465d1 100644 (file)
@@ -14,9 +14,7 @@ bool manifest_header_t::is_valid()
 {
     return m_data.major_version == m_current_major_version &&
            m_data.minor_version == m_current_minor_version &&
-           m_data.num_embedded_files > 0 &&
-           m_data.bundle_id_length > 0 && 
-           m_data.bundle_id_length < PATH_MAX;
+           m_data.num_embedded_files > 0;
 }
 
 manifest_header_t* manifest_header_t::read(FILE* stream)
@@ -32,10 +30,13 @@ manifest_header_t* manifest_header_t::read(FILE* stream)
 
         throw StatusCode::BundleExtractionFailure;
     }
+
+    // bundle_id is a component of the extraction path
+    size_t bundle_id_length = 
+        bundle_runner_t::get_path_length(header->m_data.bundle_id_length_byte_1, stream);
      
     // Next read the bundle-ID string, given its length
-    bundle_runner_t::read_string(header->m_bundle_id, 
-                                 header->m_data.bundle_id_length, stream);
+    bundle_runner_t::read_string(header->m_bundle_id, bundle_id_length, stream);
 
     return header;
 }
index 56c8b83..7af95d2 100644 (file)
@@ -40,7 +40,7 @@ namespace bundle
             uint32_t major_version;
             uint32_t minor_version;
             int32_t num_embedded_files;
-            int8_t bundle_id_length;
+            int8_t bundle_id_length_byte_1;
         } m_data;
 #pragma pack(pop)
         pal::string_t m_bundle_id;
index 8f2ec6b..31a48ab 100644 (file)
@@ -29,8 +29,9 @@ namespace StandaloneAppWithSubDirs
                 Part("Noun", "Adjective") +
                 Part("Noun") +
                 Part("Conjunction") +
-                Part("Noun", "Pronoun", "Another");
-
+                Part("Noun", "Pronoun", "Another") + 
+                Part("This is a really, really, really, really, really, really, really, really, really, really, really, really, really, really long file name for punctuation");
+                      
             // This should print "Wow! We now say hello to the big world and you."
             Console.WriteLine(message);
         }
diff --git a/src/installer/test/Assets/TestProjects/StandaloneAppWithSubDirs/Sentence/This is a really, really, really, really, really, really, really, really, really, really, really, really, really, really long file name for punctuation/word b/src/installer/test/Assets/TestProjects/StandaloneAppWithSubDirs/Sentence/This is a really, really, really, really, really, really, really, really, really, really, really, really, really, really long file name for punctuation/word
new file mode 100644 (file)
index 0000000..945c9b4
--- /dev/null
@@ -0,0 +1 @@
+.
\ No newline at end of file
index 8fc2412..123cbf8 100644 (file)
@@ -7,7 +7,6 @@
     <RuntimeFrameworkVersion>$(MNAVersion)</RuntimeFrameworkVersion>
   </PropertyGroup>
 
-
   <Target Name="CopyFiles" AfterTargets="Publish">
     <ItemGroup>
       <FilesToCopy Include="$(MSBuildThisFileDirectory)\Sentence\**\*.*" />