Truncate file name longer than MAX_PATH(260) on D2D

* Truncate file name longer than MAX_PATH(260).
* Change for cpplint.py

BUG=93601
TEST=Manual on Windows


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=100373

[sky, 2011-09-06 15:55:05]

http://codereview.chromium.org/7778008/diff/2001/chrome/browser/ui/views/tab_...
File chrome/browser/ui/views/tab_contents/tab_contents_drag_win.cc (right):

http://codereview.chromium.org/7778008/diff/2001/chrome/browser/ui/views/tab_...
chrome/browser/ui/views/tab_contents/tab_contents_drag_win.cc:234:
file_name.value().find('%') != std::string::npos) {
If the file name contains a '%' and you enter this branch of the if statement
don't you throw away the file name altogether?

http://codereview.chromium.org/7778008/diff/2001/ui/base/dragdrop/os_exchange...
File ui/base/dragdrop/os_exchange_data_provider_win.cc (right):

http://codereview.chromium.org/7778008/diff/2001/ui/base/dragdrop/os_exchange...
ui/base/dragdrop/os_exchange_data_provider_win.cc:926: size_t max_name_len =
MAX_PATH - extension_len - 1;
Is it possible that extension_len is >= MAX_PATH - 1, which would cause overflow
for size_t?

[yosin_UTC9, 2011-09-08 11:19:20]

* Handle too long file extension
* Move decoding URL encoded file name to another CR (WIP)
** BUG=95795

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

http://codereview.chromium.org/7778008/diff/6001/ui/base/dragdrop/os_exchange...
File ui/base/dragdrop/os_exchange_data_provider_win.cc (right):

http://codereview.chromium.org/7778008/diff/6001/ui/base/dragdrop/os_exchange...
ui/base/dragdrop/os_exchange_data_provider_win.cc:409: DCHECK_GT(c_data.Size(),
static_cast<size_t>(0));
0u

http://codereview.chromium.org/7778008/diff/6001/ui/base/dragdrop/os_exchange...
ui/base/dragdrop/os_exchange_data_provider_win.cc:895: HANDLE hdata =
GlobalAlloc(GMEM_MOVEABLE, kTotalBytes);
Shouldn't we check the return value?

http://codereview.chromium.org/7778008/diff/6001/ui/base/dragdrop/os_exchange...
ui/base/dragdrop/os_exchange_data_provider_win.cc:918: HANDLE hdata =
GlobalAlloc(GPTR, sizeof(FILEGROUPDESCRIPTOR));
Same thing about checking return value here.

http://codereview.chromium.org/7778008/diff/6001/ui/base/dragdrop/os_exchange...
ui/base/dragdrop/os_exchange_data_provider_win.cc:930: string16 extension =
path.Extension();
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]

Hi Sky,
Could you review again?

I would like to keep this CR as small as fixing buffer overflow.

I'll work for re-factoring this file later with
 o Using non-URL encoding filename
 o Using default filename for empty filename
   e.g. http://foo.com/image/12345/
 o Using truncated filename too long file name
   e.g. I can't drop file to desktop if filename + directory path exceeds
MAX_PATH

Thanks in advance.
-yosi

http://codereview.chromium.org/7778008/diff/6001/ui/base/dragdrop/os_exchange...
File ui/base/dragdrop/os_exchange_data_provider_win.cc (right):

http://codereview.chromium.org/7778008/diff/6001/ui/base/dragdrop/os_exchange...
ui/base/dragdrop/os_exchange_data_provider_win.cc:409: DCHECK_GT(c_data.Size(),
static_cast<size_t>(0));
On 2011/09/08 17:25:46, sky wrote:
> 0u

Done.

http://codereview.chromium.org/7778008/diff/6001/ui/base/dragdrop/os_exchange...
ui/base/dragdrop/os_exchange_data_provider_win.cc:895: HANDLE hdata =
GlobalAlloc(GMEM_MOVEABLE, kTotalBytes);
On 2011/09/08 17:25:46, sky wrote:
> Shouldn't we check the return value?
Yes, we should check. Although, I would like to do this in another CR. Checking
is easy but we also need to change upstream to handling failure case. It isn't
simple change and many places (at least four places in this file.)

I would like to focus this CR to fix buffer overflow.

http://codereview.chromium.org/7778008/diff/6001/ui/base/dragdrop/os_exchange...
ui/base/dragdrop/os_exchange_data_provider_win.cc:918: HANDLE hdata =
GlobalAlloc(GPTR, sizeof(FILEGROUPDESCRIPTOR));
On 2011/09/08 17:25:46, sky wrote:
> Same thing about checking return value here.
I'll do another CR.

http://codereview.chromium.org/7778008/diff/6001/ui/base/dragdrop/os_exchange...
ui/base/dragdrop/os_exchange_data_provider_win.cc:930: string16 extension =
path.Extension();
On 2011/09/08 17:25:46, sky wrote:
> 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?

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

[sky, 2011-09-09 04:31:42]

LGTM

[commit-bot: I haz the power, 2011-09-09 08:57:27]

Change committed as 100373