[cmake-developers] [PATCH] Fix some bugs in CPack's DragNDrop generator's handling of LICENSE files

[Andrey Mishchenko]

There were issues in the special-character-escaping and line-wrapping code
which caused DragNDrop packaging to fail mysteriously at a later step with
parsing errors in the sla.r file generated by the following code.

---
 Source/CPack/cmCPackDragNDropGenerator.cxx | 89
+++++++++++++++++++++++-------
 Source/CPack/cmCPackDragNDropGenerator.h   | 12 ++--
 2 files changed, 76 insertions(+), 25 deletions(-)

diff --git a/Source/CPack/cmCPackDragNDropGenerator.cxx
b/Source/CPack/cmCPackDragNDropGenerator.cxx
index 1a694ea..2413b7f 100644
--- a/Source/CPack/cmCPackDragNDropGenerator.cxx
+++ b/Source/CPack/cmCPackDragNDropGenerator.cxx
@@ -693,27 +693,48 @@ int cmCPackDragNDropGenerator::CreateDMG(const
std::string& src_dir,
       ofs << std::dec << std::nouppercase << std::setfill(' ');
       }

+    bool have_write_license_error = false;
+    std::string error;
+
     if(oldStyle)
       {
-      WriteLicense(ofs, 0, "", cpack_license_file);
+      if(!this->WriteLicense(ofs, 0, "", cpack_license_file, &error))
+        {
+        have_write_license_error = true;
+        }
       }
     else
       {
-      for(size_t i = 0; i < languages.size(); ++i)
+      for(size_t i = 0; i < languages.size() && !have_write_license_error;
++i)
         {
         if(singleLicense)
           {
-          WriteLicense(ofs, i + 5000, languages[i], cpack_license_file);
+          if(!this->WriteLicense(ofs, i + 5000, languages[i],
cpack_license_file, &error))
+            {
+            have_write_license_error = true;
+            }
           }
         else
           {
-          WriteLicense(ofs, i + 5000, languages[i]);
+          if(!this->WriteLicense(ofs, i + 5000, languages[i], "", &error))
+            {
+            have_write_license_error = true;
+            }
           }
         }
       }

     ofs.Close();

+    if(have_write_license_error)
+      {
+      cmCPackLogger(cmCPackLog::LOG_ERROR,
+        "Error writing license file to SLA." << std::endl
+        << error
+        << std::endl);
+      return 0;
+      }
+
     // convert to UDCO
     std::string temp_udco = this->GetOption("CPACK_TOPLEVEL_DIRECTORY");
     temp_udco += "/temp-udco.dmg";
@@ -724,7 +745,6 @@ int cmCPackDragNDropGenerator::CreateDMG(const
std::string& src_dir,
     udco_image_command << " -format UDCO";
     udco_image_command << " -ov -o \"" << temp_udco << "\"";

-    std::string error;
     if(!this->RunCommand(udco_image_command, &error))
       {
       cmCPackLogger(cmCPackLog::LOG_ERROR,
@@ -855,9 +875,10 @@
cmCPackDragNDropGenerator::GetComponentInstallDirNameSuffix(
   return GetComponentPackageFileName(package_file_name, componentName,
false);
 }

-void
+bool
 cmCPackDragNDropGenerator::WriteLicense(cmGeneratedFileStream&
outputStream,
-  int licenseNumber, std::string licenseLanguage, std::string licenseFile)
+  int licenseNumber, std::string licenseLanguage, std::string licenseFile,
+  std::string *error)
 {
   if(!licenseFile.empty() && !singleLicense)
     {
@@ -881,9 +902,12 @@
cmCPackDragNDropGenerator::WriteLicense(cmGeneratedFileStream& outputStream,
       std::getline(license_ifs, line);
       if(!line.empty())
         {
-        EscapeQuotes(line);
+        EscapeQuotesAndBackslashes(line);
         std::vector<std::string> lines;
-        BreakLongLine(line, lines);
+        if(!this->BreakLongLine(line, lines, error))
+          {
+          return false;
+          }
         for(size_t i = 0; i < lines.size(); ++i)
           {
           outputStream << "        \"" << lines[i] << "\"\n";
@@ -920,9 +944,12 @@
cmCPackDragNDropGenerator::WriteLicense(cmGeneratedFileStream& outputStream,
         std::getline(menu_ifs, line);
         if(!line.empty())
           {
-          EscapeQuotes(line);
+          EscapeQuotesAndBackslashes(line);
           std::vector<std::string> lines;
-          BreakLongLine(line, lines);
+          if(!this->BreakLongLine(line, lines, error))
+            {
+            return false;
+            }
           for(size_t i = 0; i < lines.size(); ++i)
             {
             std::string comma;
@@ -949,31 +976,53 @@
cmCPackDragNDropGenerator::WriteLicense(cmGeneratedFileStream& outputStream,
     outputStream << "};\n";
     outputStream << "\n";
     }
+
+  return true;
 }

-void
+bool
 cmCPackDragNDropGenerator::BreakLongLine(const std::string& line,
-  std::vector<std::string>& lines)
+  std::vector<std::string>& lines, std::string *error)
 {
   const size_t max_line_length = 512;
   for(size_t i = 0; i < line.size(); i += max_line_length)
     {
-    int line_length = max_line_length;
-    if(i + max_line_length > line.size())
+    size_t line_length = max_line_length;
+    if(i + line_length > line.size())
       {
       line_length = line.size() - i;
       }
+    else while(line_length > 0 && line[i + line_length - 1] != ' ')
+      {
+      line_length = line_length - 1;
+      }
+
+    if(line_length == 0)
+      {
+      *error = "Please make sure there are no words "
+               "(or character sequences not broken up by spaces or
newlines) "
+               "in your license file which are more than 512 characters
long.";
+      return false;
+      }
     lines.push_back(line.substr(i, line_length));
     }
+  return true;
 }

 void
-cmCPackDragNDropGenerator::EscapeQuotes(std::string& line)
+cmCPackDragNDropGenerator::EscapeQuotesAndBackslashes(std::string& line)
 {
-  std::string::size_type pos = line.find('\"');
-  while(pos != std::string::npos)
+  std::string::size_type backslash_pos = line.find('\\');
+  while(backslash_pos != std::string::npos)
+    {
+    line.replace(backslash_pos, 1, "\\\\");
+    backslash_pos = line.find('\\', backslash_pos + 2);
+    }
+
+  std::string::size_type quote_pos = line.find('\"');
+  while(quote_pos != std::string::npos)
     {
-    line.replace(pos, 1, "\\\"");
-    pos = line.find('\"', pos + 2);
+    line.replace(quote_pos, 1, "\\\"");
+    quote_pos = line.find('\"', quote_pos + 2);
     }
 }
diff --git a/Source/CPack/cmCPackDragNDropGenerator.h
b/Source/CPack/cmCPackDragNDropGenerator.h
index b5e5ffe..604cdf5 100644
--- a/Source/CPack/cmCPackDragNDropGenerator.h
+++ b/Source/CPack/cmCPackDragNDropGenerator.h
@@ -50,11 +50,13 @@ private:
   std::string slaDirectory;
   bool singleLicense;

-  void WriteLicense(cmGeneratedFileStream& outputStream, int licenseNumber,
-    std::string licenseLanguage, std::string licenseFile = "");
-  void BreakLongLine(const std::string& line,
-    std::vector<std::string>& lines);
-  void EscapeQuotes(std::string& line);
+  bool WriteLicense(cmGeneratedFileStream& outputStream, int licenseNumber,
+    std::string licenseLanguage, std::string licenseFile,
+    std::string *error);
+  bool BreakLongLine(const std::string& line,
+    std::vector<std::string>& lines,
+    std::string *error);
+  void EscapeQuotesAndBackslashes(std::string& line);
 };

 #endif
-- 
2.5.4 (Apple Git-61)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20160102/890ecdf3/attachment.html>