Truncate file name longer than MAX_PATH(260) on D2D

ui/base/dragdrop/os_exchange_data_provider_win.cc

Index: ui/base/dragdrop/os_exchange_data_provider_win.cc
diff --git a/ui/base/dragdrop/os_exchange_data_provider_win.cc b/ui/base/dragdrop/os_exchange_data_provider_win.cc
index 3ecbefa33ae8cddf3af54c9ddd2258187e3f0922..e5799d88cc9f3a04b4ddea8fc3d3fbaf3f5fd66f 100644
--- a/ui/base/dragdrop/os_exchange_data_provider_win.cc
+++ b/ui/base/dragdrop/os_exchange_data_provider_win.cc
@@ -4,6 +4,9 @@
 
 #include "ui/base/dragdrop/os_exchange_data_provider_win.h"
 
+#include <algorithm>
+#include <vector>
+
 #include "base/file_path.h"
 #include "base/i18n/file_util_icu.h"
 #include "base/logging.h"
@@ -124,7 +127,7 @@ STDMETHODIMP FormatEtcEnumerator::Next(
     ULONG count, FORMATETC* elements_array, ULONG* elements_fetched) {
   // MSDN says |elements_fetched| is allowed to be NULL if count is 1.
   if (!elements_fetched)
-    DCHECK(count == 1);
+    DCHECK_EQ(count, 1ul);
 
   // This method copies count elements into |elements_array|.
   ULONG index = 0;
@@ -403,7 +406,7 @@ bool OSExchangeDataProviderWin::GetPickledData(CLIPFORMAT format,
   if (SUCCEEDED(source_object_->GetData(&format_etc, &medium))) {
     if (medium.tymed & TYMED_HGLOBAL) {
       base::win::ScopedHGlobal<char> c_data(medium.hGlobal);
-      DCHECK(c_data.Size() > 0);
+      DCHECK_GT(c_data.Size(), static_cast<size_t>(0));

[sky, 2011/09/08 17:25:46]

0u

[yosin_UTC9, 2011/09/09 04:25:41]

Done.

       // Need to subtract 1 as SetPickledData adds an extra byte to the end.
       *data = Pickle(c_data.get(), static_cast<int>(c_data.Size() - 1));
       success = true;
@@ -903,35 +906,56 @@ static STGMEDIUM* GetStorageForFileName(const FilePath& path) {
 
   STGMEDIUM* storage = new STGMEDIUM;
   storage->tymed = TYMED_HGLOBAL;
-  storage->hGlobal = drop_files;
+  storage->hGlobal = hdata;
   storage->pUnkForRelease = NULL;
   return storage;
 }
 
 static STGMEDIUM* GetStorageForFileDescriptor(
     const FilePath& path) {
-  string16 valid_file_name = path.value();
-  DCHECK(!valid_file_name.empty() && valid_file_name.size() + 1 <= MAX_PATH);
-  HANDLE handle = GlobalAlloc(GPTR, sizeof(FILEGROUPDESCRIPTOR));
-  FILEGROUPDESCRIPTOR* descriptor =
-      reinterpret_cast<FILEGROUPDESCRIPTOR*>(GlobalLock(handle));
+  string16 file_name = path.value();
+  DCHECK(!file_name.empty());
+  HANDLE hdata = GlobalAlloc(GPTR, sizeof(FILEGROUPDESCRIPTOR));

[sky, 2011/09/08 17:25:46]

Same thing about checking return value here.

[yosin_UTC9, 2011/09/09 04:25:41]

I'll do another CR.

+  base::win::ScopedHGlobal<FILEGROUPDESCRIPTOR> locked_mem(hdata);
 
+  FILEGROUPDESCRIPTOR* descriptor = locked_mem.get();
   descriptor->cItems = 1;
-  wcscpy_s(descriptor->fgd[0].cFileName,
-           valid_file_name.size() + 1,
-           valid_file_name.c_str());
   descriptor->fgd[0].dwFlags = FD_LINKUI;
-
-  GlobalUnlock(handle);
+  int file_name_len = file_name.size();
+  if (file_name_len + 1 < MAX_PATH) {
+    wcscpy_s(descriptor->fgd[0].cFileName,
+             MAX_PATH,
+             file_name.c_str());
+  } else {
+    string16 extension = path.Extension();

[sky, 2011/09/08 17:25:46]

Shouldn't we make sure we maintain the path component? For example, lets say
path is something like:

c:/temp/foo.an-absurdly-long-extension-that-is-almost-max_path

Shouldn't we make sure we honor the c:/temp part?

[yosin_UTC9, 2011/09/09 04:25:41]

Callers should take care of it. Actually, this function receives only file name
component at this time, even if filename parameter in Content-Disposition.

+    int extension_len = extension.size();
+    int max_name_len = MAX_PATH - extension_len - 1;
+    if (max_name_len >= 1) {
+      // We take at least one character from name.
+      int orig_name_len = file_name_len - extension_len;
+      int name_len = std::min(orig_name_len, max_name_len);
+      memcpy(descriptor->fgd[0].cFileName,
+             file_name.c_str(),
+             sizeof(wchar_t) * name_len);
+      wcscpy_s(descriptor->fgd[0].cFileName + name_len,
+               MAX_PATH - extension_len,
+               extension.c_str());
+    } else {
+      // We take MAX_PATH-1 characters from file_name.
+      wcsncpy_s(descriptor->fgd[0].cFileName,
+                MAX_PATH,
+                file_name.c_str(),
+                MAX_PATH - 1);
+    }
+  }
 
   STGMEDIUM* storage = new STGMEDIUM;
-  storage->hGlobal = handle;
+  storage->hGlobal = hdata;
   storage->tymed = TYMED_HGLOBAL;
   storage->pUnkForRelease = NULL;
   return storage;
 }
 
-
 ///////////////////////////////////////////////////////////////////////////////
 // OSExchangeData, public: