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/third_party/icu/icu_utf.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;
 
 namespace {
 
 typedef std::map<content::DownloadId, base::FilePath> ReservationMap;
 
+// The lower bound for file name truncation. If the truncation results in a name
+// shorter than this limit, we give up automatic truncation and prompt the user.
+static const size_t kTruncatedNameLengthLowerbound = 5;
+
+// The length of the suffix string we append for an intermediate file name.
+// In the file name truncation, we keep the margin to append the suffix.
+// TODO(kinaba): remove the margin. The user should be able to set maximum
+// possible filename.
+static const size_t kIntermediateNameSuffixLength = sizeof(".crdownload") - 1;
+
 // Map of download path reservations. Each reserved path is associated with a
 // DownloadId. This object is destroyed in |Revoke()| when there are no more
 // reservations.
 //
 // It is not an error, although undesirable, to have multiple DownloadIds that
 // are mapped to the same path. This can happen if a reservation is created that
 // is supposed to overwrite an existing reservation.
 ReservationMap* g_reservation_map = NULL;
 
 // Observes a DownloadItem for changes to its target path and state. Updates or
@@ skipping 44 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;
 }
 
+// Truncates path->BaseName() to satisfy the condition .value().size() <= limit.

[asanka, 2013/02/19 17:59:15]

Nit: path->BaseName().value().size() <= limit.

|limit| only applies to the BaseName().

[kinaba, 2013/02/20 13:57:41]

Done.

+// - It keeps the extension as is. Only truncates the body part.

[asanka, 2013/02/19 17:59:15]

Nit: "base filename" would, I think, be clearer than "body part."

[kinaba, 2013/02/20 13:57:41]

Done.

+// - It secures the body part length to be >= kTruncatedNameLengthLowerbound.
+// If it was unable to shorten the name, returns false.
+bool TruncateFileName(base::FilePath* path, size_t limit) {
+  base::FilePath basename(path->BaseName());
+  // It is already short enough.
+  if (basename.value().size() <= limit)
+    return true;
+
+  base::FilePath dir(path->DirName());
+  base::FilePath::StringType ext(basename.Extension());
+  base::FilePath::StringType name(basename.RemoveExtension().value());
+
+  // Impossible to satisfy the limit.
+  if (limit < kTruncatedNameLengthLowerbound + ext.size())
+    return false;
+  limit -= ext.size();
+
+  // Encoding specific truncation logic.
+  base::FilePath::StringType truncated;
+#if defined(OS_CHROMEOS) || defined(OS_MACOSX)
+  // UTF-8.
+  TruncateUTF8ToByteSize(name, limit, &truncated);
+#elif defined(OS_WIN)
+  // UTF-16.
+  DCHECK(name.size() > limit);
+  truncated = name.substr(
+      0, limit > 0 && CBU16_IS_TRAIL(name[limit]) ? limit - 1 : limit);

[asanka, 2013/02/19 17:59:15]

Nit: limit > 0 is always true at this point.

[kinaba, 2013/02/20 13:57:41]

Done.

+#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.
+#endif
+
+  if (truncated.size() < kTruncatedNameLengthLowerbound)
+    return false;
+  *path = dir.Append(truncated + ext);
+  return true;
+}
+
 // 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.
+// - Truncates the suggested name if it exceeds the filesystem's limit.
 // - Uniquifies |suggested_path| if |should_uniquify_path| is true.
 // - 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 base::FilePath& suggested_path,
     const base::FilePath& default_download_path,
     bool should_uniquify,
     const DownloadPathReservationTracker::ReservedPathCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   DCHECK(download_id.IsValid());
   DCHECK(suggested_path.IsAbsolute());
 
   // Create a reservation map if one doesn't exist. It will be automatically
   // deleted when all the reservations are revoked.
   if (g_reservation_map == NULL)
     g_reservation_map = new ReservationMap;
 
   ReservationMap& reservations = *g_reservation_map;
   DCHECK(!ContainsKey(reservations, download_id));
 
   base::FilePath target_path(suggested_path.NormalizePathSeparators());
   bool is_path_writeable = true;
   bool has_conflicts = false;
+  bool name_too_long = false;
 
   // Create the default download path if it doesn't already exist and is where
   // we are going to create the downloaded file. |target_path| might point
   // elsewhere if this was a programmatic download.
   if (!default_download_path.empty() &&
       default_download_path == target_path.DirName() &&
       !file_util::DirectoryExists(default_download_path)) {
     file_util::CreateDirectory(default_download_path);
   }
 
   // Check writability of the suggested path. If we can't write to it, default
   // to the user's "My Documents" directory. We'll prompt them in this case.
   base::FilePath dir = target_path.DirName();
   base::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);
   }
 
-  if (is_path_writeable && should_uniquify && IsPathInUse(target_path)) {
-    has_conflicts = true;
-    for (int uniquifier = 1;
-         uniquifier <= DownloadPathReservationTracker::kMaxUniqueFiles;
-         ++uniquifier) {
-      base::FilePath path_to_check(target_path.InsertBeforeExtensionASCII(
-          StringPrintf(" (%d)", uniquifier)));
-      if (!IsPathInUse(path_to_check)) {
-        target_path = path_to_check;
-        has_conflicts = false;
-        break;
+  if (is_path_writeable) {
+    // Check the limit of file name length if it could be obtained. When the
+    // suggested name exceeds the limit, truncate or prompt the user.
+    int max_length = file_util::GetMaximumPathComponentLength(dir);
+    if (max_length != -1) {
+      int limit = max_length - kIntermediateNameSuffixLength;
+      if (limit <= 0 || !TruncateFileName(&target_path, limit))
+        name_too_long = true;
+    }
+
+    // Uniquify the name, if it already exists.
+    if (!name_too_long && should_uniquify && IsPathInUse(target_path)) {
+      has_conflicts = true;
+      for (int uniquifier = 1;
+           uniquifier <= DownloadPathReservationTracker::kMaxUniqueFiles;
+           ++uniquifier) {
+        // Append uniquifier. If necessary, truncate the name.
+        std::string suffix(StringPrintf(" (%d)", uniquifier));

[asanka, 2013/02/19 17:59:15]

Nit: Use base::StringPrintf() and include "base/stringprintf.h"

[kinaba, 2013/02/20 13:57:41]

Done.

+        base::FilePath path_to_check(target_path);
+        int limit = max_length - kIntermediateNameSuffixLength - suffix.size();
+        if (limit <= 0 || !TruncateFileName(&path_to_check, limit))
+          break;

[asanka, 2013/02/19 17:59:15]

Nit: We could get here if |max_length| is -1 and there is a path conflict. If
that case was handled explicitly then we wouldn't be doing arithmetic on a
signal value and the code would be easier to follow.

[kinaba, 2013/02/20 13:57:41]

Good catch. Thanks.
Explicitly dispatched the |max_length| = -1 case.

+        path_to_check = path_to_check.InsertBeforeExtensionASCII(suffix);
+
+        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 base::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 base::FilePath& path) {
   return IsPathInUse(path);
 }