Removing myself from CC list. I don't work on anything relating to this code. It should just go to the bug triage queue.

(Especially if it's an iOS-10-only failure, this should probably go to iOS folks for triage first. Note I do not even have an iOS device.)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4bb66d2c936984bd48c541305d122a02a46ecc9a

commit 4bb66d2c936984bd48c541305d122a02a46ecc9a
Author: kkhorimoto <kkhorimoto@chromium.org>
Date: Fri Aug 19 02:11:33 2016

Disable CookieStoreTest.InvalidDomainTest on iOS10

These tests fail on iOS10, but these failures were not caught upstream
since bots aren't yet updated to run iOS10.

BUG= 639167 
TBR=jww@chromium.org
NOTREECHECKS=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2258603004
Cr-Commit-Position: refs/heads/master@{#413025}

[modify] https://crrev.com/4bb66d2c936984bd48c541305d122a02a46ecc9a/net/cookies/cookie_store_unittest.h

This should go through triage process, we do not have anyone on iOS team who understands this code.

It sounds like the test fails on iOS 10 only. +cc justincohen
eugenebut: any idea on who to dispatch this to?

Per comment #6. This should go through triage process, we do not have anyone on iOS team who understands this code.

davidben: Is there a networking triage queue? I'm doing iOS triage right now and it does not appear to have anyone on the iOS team who knows enough for me to send this to.

justincohen: have you seen this failure in your iOS 10 testing?

There is a networking triage queue, but I don't think anyone on the team does anything with iOS 10. The cookie bits end up synchronizing with the platform cookie store, so this sounds like a very iOS-specific thing.

Kurt, could you please attach a crash log.

The full stdio can be found here.  I've also copied the error messages for the disabled tests below.

https://uberchromegw.corp.google.com/i/internal.bling.tryserver/builders/smoke/builds/20048/steps/ios_net_unittests%20%28iPhone%205%20iOS%2010.0%29%20on%20Mac/logs/stdio

CookieStoreIOS/CookieStoreTest/0.InvalidDomainTest: 
../../net/cookies/cookie_store_unittest.h:607: Failure
Value of: this->SetCookie(cs, url_foobar, "a=1; domain=.yo.foo.bar.com; domain=")
  Actual: true
Expected: false
../../net/cookies/cookie_store_unittest.h:289: Failure
Value of: TokenizeCookieLine(line2)
  Actual: { "a=1" }
Expected: TokenizeCookieLine(line1)
Which is: {}

RoundTripTestCookieStore/CookieStoreTest/0.InvalidDomainTest: 
../../net/cookies/cookie_store_unittest.h:607: Failure
Value of: this->SetCookie(cs, url_foobar, "a=1; domain=.yo.foo.bar.com; domain=")
  Actual: true
Expected: false
../../net/cookies/cookie_store_unittest.h:289: Failure
Value of: TokenizeCookieLine(line2)
  Actual: { "a=1" }
Expected: TokenizeCookieLine(line1)
Which is: {}

Adding some iOS net folks.

I'll take a look

Is it reproducible on the simulator?
Do I need XCode 8 to build for iOS 10?

Thanks for looking into this. Yes this is reproducible with simulator and yes, you need Xcode 8 (at least beta 4) to build this for iOS10.

I've got it reproduced locally, after I properly synced my chromium checkout. The problem is introduced in https://codereview.chromium.org/2246613003 which added regression test for  issue 601786 

  // Regression test for  https://crbug.com/601786 
  EXPECT_FALSE(
      this->SetCookie(cs, url_foobar, "a=1; domain=.yo.foo.bar.com; domain="));

Removing this test makes CookieStoreTest/InvalidDomainTests pass on iOS10.

@jww - Is it expected to work on iOS? 
If not, should we #ifdef it out?
If yes, any ideas on how to fix it?

Yikes, this is an odd one (made odder by my lack of iOS knowledge :-/). Yes, I would expect these tests to run on iOS, and, in fact, they do run on iOS 9 I believe, based on the ios-simulator bot passing on https://codereview.chromium.org/2246613003. So I suppose something must have changed between iOS 9 and 10.

I know there are a few things in the iOS cookie store that rely on the underlying OS. For example, I needed to turn off several tests on iOS because the iOS cookie parser behaves differently than the one in the Chrome networking stack (see https://cs.chromium.org/chromium/src/net/cookies/cookie_store_unittest.h?l=468). I suppose it's possible that the iOS cookie parser modified something to change the behavior for domain parsing between 9 and 10?

I've debugged a little more.

The cookie line in this test:

  EXPECT_FALSE(
      this->SetCookie(cs, url_foobar, "a=1; domain=.yo.foo.bar.com; domain="));

Inside of CookieStoreIOS::SetCookieWithOptionsAsync

NSCookie is created with domain "foo.bar.com".
(lldb) p [cookie name]
(NSTaggedPointerString *) $1 = 0xa000000000000611 @"a"
(lldb) p [cookie value]
(NSTaggedPointerString *) $2 = 0xa000000000000311 @"1"
(lldb) p [cookie domain]
(__NSCFString *) $3 = 0x0000600000033f60 @"foo.bar.com"

Both has_explicit_domain and has_valid_domain are true:

      bool has_explicit_domain = HasExplicitDomain(cookie_line);
      bool has_valid_domain =
          net::cookie_util::GetCookieDomainWithString(
              url, domain_string, &dummy);

So the result is true, while we expect it to be false.

On iOS 9.3 the cookie is created with domain ".yo.foo.bar.com" 

(lldb) po cookie
<NSHTTPCookie version:0 name:"a" value:"1" expiresDate:(null) created:2016-08-25 14:57:39 +0000 sessionOnly:TRUE domain:".yo.foo.bar.com" path:"/" isSecure:FALSE>

and |has_valud_domain| is false, so result is true.

Any ideas?

That it would set the domain to foo.bar.com seems even more odd to me. Perhaps this is an iOS 10 bug then?

In any case, since we don't actually have control over cookie line parsing in iOS, I'm personally fine with skipping this test on iOS with a comment pointing to issue 638389, just as we do for EmptyKeyTest (https://cs.chromium.org/chromium/src/net/cookies/cookie_store_unittest.h?q=EmptyKeyTest&sq=package:chromium&l=472&dr=CSs).

Andrei and I have poked around a little more and found that tests succeeds on iOS 10 if we change cookie line to "a=1; domain=; domain=.yo.foo.bar.com", however it fails on iOS 9.3.

It appears that if cookie like doesn't have domain specified, then it is taken from URL on both versions, however on iOS 9 cookie line appears to be parsed right-to-left, and on iOS 10 cookie line appears to be parsed left-to-right.

If the last parsed domain= is empty, then cookie gets domain from the url and test fails.

It seems that we cannot expect particular behavior from iOS parser.
I 'll prepare the CL to disable this particular test on iOS.

Is this something that we should file a radar for?  I'm not following exactly what the failure case is on iOS10, but if you can simplify it down to a few lines I'm happy to file the radar too.

I don't think it is necessarily a bug, more like a change in undefined behavior, but I should be able to demonstrate it in few lines of code if we want to file a radar.

Thanks for the digging, mef@. Unfortunately, changing the test to have the empty domain= first is no good since having it last was the point of the test. Thus, your CL that you uploaded where it's just disabled on iOS seems like the best solution to me.

It is either the iOS bug (both in iOS 9 and iOS 10) or we set incorrect our test expectations incorrectly. The problem happens when the cookie string contains two domain attributes, e.g. "a=1; domain=sub1.site.com; domain=sub2.site.com"

The https://tools.ietf.org/html/rfc2965 spec doesn't say what to do in that case. iOS 9 gets the first domain and ignores the second. iOS 10 does the opposite - it ignores the first one and gets the second one.

If we assume that a cookie string with two domains is invalid then it is a bug in both versions of iOS.
If we assume that the behavior is undefined and the cookie string processor can pick either of them then our test is incorrect.

This one's kinda weird...We changed our behavior to match IE and Safari's behavior (And to diverge from FF's, which our code was originally aimed at matching), but then iOS/Safari changed their behavior to always use the first one...

justincohen@: Here is a snippet if you consider it radar-worthy:

  NSString* cookieLine = @"a=1; domain=example.org; domain=";
  NSURL* url = [NSURL URLWithString:@"http://foo.bar.com"];
  NSArray* cookies = [NSHTTPCookie cookiesWithResponseHeaderFields:@{
                                                                     @"Set-Cookie" : cookieLine
                                                                     } forURL:url];
  NSLog(@"url:%@", url);
  NSLog(@"cookie line:%@", cookieLine);
  NSLog(@"cookie:%@", cookies[0]);

Running on iOS 9:

2016-08-25 17:04:53.045 mef_test_3[80812:4850980] url:http://foo.bar.com
2016-08-25 17:04:53.045 mef_test_3[80812:4850980] cookie line:a=1; domain=example.org; domain=
2016-08-25 17:04:53.046 mef_test_3[80812:4850980] cookie:<NSHTTPCookie version:0 name:"a" value:"1" expiresDate:(null) created:2016-08-25 21:04:53 +0000 sessionOnly:TRUE domain:".example.org" path:"/" isSecure:FALSE>

Running on iOS 10:

2016-08-25 17:06:02.992 mef_test_3[81012:4853318] url:http://foo.bar.com
2016-08-25 17:06:02.992 mef_test_3[81012:4853318] cookie line:a=1; domain=example.org; domain=
2016-08-25 17:06:02.993 mef_test_3[81012:4853318] cookie:<NSHTTPCookie version:0 name:"a" value:"1" expiresDate:(null) created:2016-08-25 21:06:02 +0000 sessionOnly:TRUE domain:"foo.bar.com" partition:"none" path:"/" isSecure:FALSE>

Note how cookie domain is set to ".example.org" on iOS 9 but to "foo.bar.com" on iOS 10.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5881d4e63b7ba966f48cb5cbda5867e90d6a17b1

commit 5881d4e63b7ba966f48cb5cbda5867e90d6a17b1
Author: mef <mef@chromium.org>
Date: Thu Aug 25 21:15:53 2016

Reenable CookieStoreTest InvalidDomainTest on iOS.

BUG= 639167 

Review-Url: https://codereview.chromium.org/2275023004
Cr-Commit-Position: refs/heads/master@{#414540}

[modify] https://crrev.com/5881d4e63b7ba966f48cb5cbda5867e90d6a17b1/net/cookies/cookie_store_unittest.h

I've re-enabled the InvalidDomainTest without the problematic case on iOS.

Thanks for looking into this, Misha!