Truncate the download file name if it exceeds the filesystem limit.

chrome/browser/download/download_path_reservation_tracker.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/download/download_path_reservation_tracker.h"
 
 #include <map>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/stl_util.h"
+#include "base/string_util.h"
+#include "base/sys_info.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/common/chrome_paths.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/download_id.h"
 #include "content/public/browser/download_item.h"
 
 using content::BrowserThread;
 using content::DownloadId;
 using content::DownloadItem;
 
@@ skipping 58 matching lines @@
   if (IsPathReserved(path))
     return true;
 
   // If the path exists in the file system, then the path is in use.
   if (file_util::PathExists(path))
     return true;
 
   return false;
 }
 
+// Truncate the given name so that its becomes .size() <= limit. Returns an
+// empty string if it is impossible due to unknown file-name encoding.
+FilePath::StringType TruncateFileName(const FilePath::StringType& name,
+                                      size_t limit) {
+  FilePath::StringType truncated;
+#if defined(OS_CHROMEOS) || defined(OS_MACOSX)
+  TruncateUTF8ToByteSize(name, limit, &truncated);
+#elif defined(OS_WIN)
+  // TODO(kinaba): Implement for Windows, where StringType == string16.
+#else
+  // We cannot generally assume that the file name encoding is in UTF-8 (see
+  // the comment for FilePath::AsUTF8Unsafe), hence no safe way to truncate.

[Randy Smith (Not in Mondays), 2013/02/07 19:12:03]

I'm inclined to think that we'll end up with a better user experience if we
power through and do it anyway, but that might be worth checking with people
wiser in the ways of character encodings than I.

+#endif
+  return truncated;
+}
+
 // Called on the FILE thread to reserve a download path. This method:
 // - Creates directory |default_download_path| if it doesn't exist.
 // - Verifies that the parent directory of |suggested_path| exists and is
 //   writeable.
 // - Uniquifies |suggested_path| if |should_uniquify_path| is true.

[Randy Smith (Not in Mondays), 2013/02/07 19:12:03]

Should mention truncation in this comment as well.

[kinaba, 2013/02/08 10:46:57]

Done.

 // - Schedules |callback| on the UI thread with the reserved path and a flag
 //   indicating whether the returned path has been successfully verified.
 void CreateReservation(
     DownloadId download_id,
     const FilePath& suggested_path,
     const FilePath& default_download_path,
     bool should_uniquify,
     const DownloadPathReservationTracker::ReservedPathCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   DCHECK(download_id.IsValid());
@@ skipping 24 matching lines @@
   // to the user's "My Documents" directory. We'll prompt them in this case.
   FilePath dir = target_path.DirName();
   FilePath filename = target_path.BaseName();
   if (!file_util::PathIsWritable(dir)) {
     DVLOG(1) << "Unable to write to directory \"" << dir.value() << "\"";
     is_path_writeable = false;
     PathService::Get(chrome::DIR_USER_DOCUMENTS, &dir);
     target_path = dir.Append(filename);
   }
 
+  // Check the limit of file name length if it could be obtained. When the
+  // suggested name exceeds the limit, truncate or prompt the user.
+  bool name_too_long = false;
+  if (is_path_writeable) {
+    int max_length = base::SysInfo::GetMaximumPathComponentLength(dir);
+    if (max_length > 0) {
+      // The extension should not be truncated, so we split it first.
+      FilePath::StringType ext = filename.Extension();
+      FilePath::StringType name = filename.RemoveExtension().value();
+
+      // Reserve several characters we may append later.
+      const int kExtraChars = sizeof(" (100).crdownload") - 1;

[Randy Smith (Not in Mondays), 2013/02/07 19:12:03]

I'd rather find some way to export the amount of space we need to reserve for
the intermediate name from the system that's creating the intermediate name
(chrome_download_manager_delegate.h).  But that's a nit that we can resolve in
later reviews.

+      int limit = max_length - ext.size() - kExtraChars;
+      if (static_cast<int>(name.size()) > limit) {
+        name_too_long = true;
+        if (limit > 0) {

[Randy Smith (Not in Mondays), 2013/02/07 19:12:03]

I'd be inclined to reserve a few more characters here, though I'll admit I'm not
sure how many; five?  It should be pulled out into a separate constant, as I
could imagine people might want to play with the value for maintainability.

[kinaba, 2013/02/08 10:46:57]

Pulled out the constant (tentatively 5).
IMHO we can be more aggressive like, say, 100, since all modern filesystem has
max_length>=255 (ecryptfs's 143 is notably low) but not quite sure either...

+          // Truncate the name to fit in |limit|.
+          FilePath::StringType truncated = TruncateFileName(name, limit);
+          if (!truncated.empty()) {
+            target_path = dir.Append(truncated + ext);
+            name_too_long = false;
+          }
+        }
+      }
+    }
+  }
+
   if (is_path_writeable && should_uniquify && IsPathInUse(target_path)) {
     has_conflicts = true;
     for (int uniquifier = 1;
          uniquifier <= DownloadPathReservationTracker::kMaxUniqueFiles;
          ++uniquifier) {
       FilePath path_to_check(target_path.InsertBeforeExtensionASCII(
           StringPrintf(" (%d)", uniquifier)));
       if (!IsPathInUse(path_to_check)) {
         target_path = path_to_check;
         has_conflicts = false;
         break;
       }
     }
   }
   reservations[download_id] = target_path;
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(callback, target_path, (is_path_writeable && !has_conflicts)));
+  bool verified = (is_path_writeable && !has_conflicts && !name_too_long);
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(callback, target_path, verified));
 }
 
 // Called on the FILE thread to update the path of the reservation associated
 // with |download_id| to |new_path|.
 void UpdateReservation(DownloadId download_id, const FilePath& new_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   DCHECK(g_reservation_map != NULL);
   ReservationMap::iterator iter = g_reservation_map->find(download_id);
   if (iter != g_reservation_map->end()) {
     iter->second = new_path;
@@ skipping 97 matching lines @@
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&CreateReservation, download_item.GetGlobalId(),
                  target_path, default_path, uniquify_path, callback));
 }
 
 // static
 bool DownloadPathReservationTracker::IsPathInUseForTesting(
     const FilePath& path) {
   return IsPathInUse(path);
 }