Include trailing '\' when iterating over path in reparse points search.

Skip root dir when iterating over path in reparse points search.

It turns out that ::GetFileAttributes(...) Windows API for paths in form of "<drive>:" returns attribute of binary file work dir instead of "<drive>:\"
dir.

BUG=340625
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=249667

[cpu_(ooo_6.6-7.5), 2014-02-03 18:21:21]

The diff is busted (not visible). Please reload the change again.

[rvargas (doing something else), 2014-02-03 23:09:23]

Do you mind opening a bug? It helps to keep track of the impact.

https://codereview.chromium.org/150093006/diff/1/sandbox/win/src/win_utils.cc
File sandbox/win/src/win_utils.cc (left):

https://codereview.chromium.org/150093006/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:117: 
Looks like we should instead break the loop when we reach the root directory
(sounds like a bad idea to rely on the result of that last call, even if it is
properly formatted).

[mharanczyk, 2014-02-04 11:18:19]

Reported issue and reuploaded patch set, looks like it was done properly now,
sorry for not noticing it earlier.

Regarding different loop end condition: you cannot deduce that you reached root
dir from attributes, so what exactly is your idea? Check if path length is 2 and
is a letter and a colon?

[cpu_(ooo_6.6-7.5), 2014-02-04 22:12:11]

Removing myself from review. Ricardo is enough.

[rvargas (doing something else), 2014-02-04 22:29:47]

On 2014/02/04 11:18:19, mharanczyk wrote:
> Reported issue and reuploaded patch set, looks like it was done properly now,
> sorry for not noticing it earlier.
> 
> Regarding different loop end condition: you cannot deduce that you reached
root
> dir from attributes, so what exactly is your idea? Check if path length is 2
and
> is a letter and a colon?

Correct. This function expects a path in the form \??\l:\foo\bar and uses l:\foo
when calling GetFileAttributes. From the point of view of reaching this point, a
previous policy match on the path is required, so at the very least l:\ has to
match an explicit policy rule (aka, there is no concern of that being a reparse
point).

... and GetFileAttributes of a root directory is not exactly standard in that
the result is (FS) implementation dependent so there is not much point in doing
that.

I was thinking on doing "while (last_pos > 2)" and just comment that we skip the
root. Not even checking for letter and colon.

[mharanczyk, 2014-02-06 06:49:22]

I don't know if you get automatic email if I push something to update patch, so
just a note that I modified it yesterday as you suggested.

[rvargas (doing something else), 2014-02-06 18:12:07]

No, I don't get automatic emails on updates (plus, in general, it is not easy to
know when something is "ready for review").

LGTM, thanks.

https://codereview.chromium.org/150093006/diff/210001/sandbox/win/src/win_uti...
File sandbox/win/src/win_utils.cc (right):

https://codereview.chromium.org/150093006/diff/210001/sandbox/win/src/win_uti...
sandbox/win/src/win_utils.cc:117: } while (last_pos > 2);  // Skip root dir
Tiny nit: period at the end.

[mharanczyk, 2014-02-07 06:20:09]

The CQ bit was checked by mharanczyk@opera.com

[commit-bot: I haz the power, 2014-02-07 06:20:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mharanczyk@opera.com/150093006/270001

[commit-bot: I haz the power, 2014-02-07 09:50:50]

Change committed as 249667