Sandbox: Add support for file system policies that use implied device paths.

Sandbox: Add support for file system policies that use implied device paths.

A policy rule of the form \HarddiskVolume0\Foo\bar allows sandboxed code
to use \\.\HarddiskVolume0\Foo\bar directly.

BUG=111155
TEST=sbox_integration_tests

Committed: https://crrev.com/0e49d029d5a1a25d971880b9e44d67ac70b31a80
Cr-Commit-Position: refs/heads/master@{#323295}

[rvargas (doing something else), 2015-02-11 00:34:21]

rvargas@chromium.org changed reviewers:
+ cpu@chromium.org

[cpu_(ooo_6.6-7.5), 2015-02-11 00:42:58]

: error LNK2019: unresolved external symbol __imp__CharUpperW@4 referenced in
function "public: static int __cdecl base::FilePath::CompareIgnoreCase

[rvargas (doing something else), 2015-02-11 01:08:12]

On 2015/02/11 00:42:58, cpu wrote:
> : error LNK2019: unresolved external symbol __imp__CharUpperW@4 referenced in
> function "public: static int __cdecl base::FilePath::CompareIgnoreCase

yeah, I saw that. For the time being I'm assuming it's not this patch (I'm not
changing base)

[cpu_(ooo_6.6-7.5), 2015-02-13 19:27:37]

I need a bit more time to run the changes in my head.

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

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:43: base::string16 lower_path =
base::StringToLowerASCII(path);
without this helper do we still need to include string_util ?

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

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:53: bool IsDevicePath(const base::string16& path,
base::string16* trimmed_path ) {
name: TrimDevicePath() ?

and above TrimNtPath() ?

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:71: 
I think the thing below is
http://en.cppreference.com/w/cpp/string/byte/isalpha

[rvargas (doing something else), 2015-02-13 19:43:05]

Take your time.

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

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:53: bool IsDevicePath(const base::string16& path,
base::string16* trimmed_path ) {
On 2015/02/13 19:27:37, cpu wrote:
> name: TrimDevicePath() ?
> 
> and above TrimNtPath() ?

Not so sure. I want the "main" function to be the implied by the return value
("is this a foo kind of path?"). The first version had the out argument as
optional but all users were passing a value so I removed that feature.

I don't think it is worth splitting this in two functions: check + trim.

See line 227

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:71: 
On 2015/02/13 19:27:37, cpu wrote:
> I think the thing below is
> http://en.cppreference.com/w/cpp/string/byte/isalpha

That is locale dependent and a drive letter is not.

[cpu_(ooo_6.6-7.5), 2015-02-17 21:59:46]

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

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:53: bool IsDevicePath(const base::string16& path,
base::string16* trimmed_path ) {
On 2015/02/13 19:43:05, rvargas wrote:
> On 2015/02/13 19:27:37, cpu wrote:
> > name: TrimDevicePath() ?
> > 
> > and above TrimNtPath() ?
> 
> Not so sure. I want the "main" function to be the implied by the return value
> ("is this a foo kind of path?"). The first version had the out argument as
> optional but all users were passing a value so I removed that feature.
> 
> I don't think it is worth splitting this in two functions: check + trim.
> 
> See line 227

Acknowledged.

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:71: 
On 2015/02/13 19:43:05, rvargas wrote:
> On 2015/02/13 19:27:37, cpu wrote:
> > I think the thing below is
> > http://en.cppreference.com/w/cpp/string/byte/isalpha
> 
> That is locale dependent and a drive letter is not.

Acknowledged.

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:179: path.rfind(L'\\') == kNTDotPrefixLen - 1) {
I get nervous when I see && == and - in a line with no ( ), I hope this is not a
operator precedence quiz.

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:204: return true;
two objects are the same if one of them is a pipe. 

Uhh..  

(yeah I know it is not new code)

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:263: }
if I might suggest, I think we need a helper here or elsewhere of the form

bool icompare(const std::string& s1, const std::string& s2, size_t len) {
   return 0 == _wcsnicmp(s1.c_str(), s2.c_str(), len);
}

possibly with ( ..., size_t start, size_t len)

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:273: return false;
can lines 272 and 273 be moved above 269 ?

[rvargas (doing something else), 2015-02-17 22:59:29]

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

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:179: path.rfind(L'\\') == kNTDotPrefixLen - 1) {
On 2015/02/17 21:59:45, cpu wrote:
> I get nervous when I see && == and - in a line with no ( ), I hope this is not
a
> operator precedence quiz.

Done.

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:204: return true;
On 2015/02/17 21:59:46, cpu wrote:
> two objects are the same if one of them is a pipe. 
> 
> Uhh..  
> 
> (yeah I know it is not new code)

Open to suggestions... Of course, the reasoning was that under PIPE no same
person would have redirections

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:263: }
On 2015/02/17 21:59:45, cpu wrote:
> if I might suggest, I think we need a helper here or elsewhere of the form
> 
> bool icompare(const std::string& s1, const std::string& s2, size_t len) {
>    return 0 == _wcsnicmp(s1.c_str(), s2.c_str(), len);
> }
> 
> possibly with ( ..., size_t start, size_t len)

Not sure the result is better, but done.

https://codereview.chromium.org/909373004/diff/1/sandbox/win/src/win_utils.cc...
sandbox/win/src/win_utils.cc:273: return false;
On 2015/02/17 21:59:46, cpu wrote:
> can lines 272 and 273 be moved above 269 ?

Done.

[cpu_(ooo_6.6-7.5), 2015-02-17 23:55:27]

https://codereview.chromium.org/909373004/diff/20001/sandbox/win/src/win_util...
File sandbox/win/src/win_utils.cc (right):

https://codereview.chromium.org/909373004/diff/20001/sandbox/win/src/win_util...
sandbox/win/src/win_utils.cc:39: bool EqualPath(base::string16 first,
base::string16 second) {
by const ref? here and below?

[cpu_(ooo_6.6-7.5), 2015-02-18 16:04:06]

other than that lgtm

[rvargas (doing something else), 2015-02-18 20:39:53]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2015-02-18 20:41:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/909373004/40001

[commit-bot: I haz the power, 2015-02-18 22:04:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-18 22:04:04]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[rvargas (doing something else), 2015-03-20 23:23:53]

Please take a look at the latest version. (PS4)

[cpu_(ooo_6.6-7.5), 2015-03-24 19:14:28]

lgtm

[rvargas (doing something else), 2015-03-24 19:43:15]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2015-03-24 19:43:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/909373004/80001

[commit-bot: I haz the power, 2015-03-25 00:01:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-25 00:01:19]

Try jobs failed on following builders:
  win8_chromium_rel on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[rvargas (doing something else), 2015-03-30 19:49:06]

... And another change (hopefully the last one)

[cpu_(ooo_6.6-7.5), 2015-04-01 17:32:27]

remove the version helpers include, then lgtm

[rvargas (doing something else), 2015-04-01 17:37:26]

The CQ bit was checked by rvargas@chromium.org

[rvargas (doing something else), 2015-04-01 17:37:26]

The patchset sent to the CQ was uploaded after l-g-t-m from cpu@chromium.org
Link to the patchset: https://codereview.chromium.org/909373004/#ps140001
(title: "Remove stale header")

[commit-bot: I haz the power, 2015-04-01 17:38:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/909373004/140001

[commit-bot: I haz the power, 2015-04-01 19:20:12]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-04-01 19:21:14]

Patchset 8 (id:??) landed as
https://crrev.com/0e49d029d5a1a25d971880b9e44d67ac70b31a80
Cr-Commit-Position: refs/heads/master@{#323295}