Fix parsing of max-age in SdchManager.

net/base/sdch_manager.cc

Index: net/base/sdch_manager.cc
diff --git a/net/base/sdch_manager.cc b/net/base/sdch_manager.cc
index 7f8245cc917e3507cef3b9bae14fad32233aa06f..699c8c5469f27e785521f67d99eec5a3be0cb6cb 100644
--- a/net/base/sdch_manager.cc
+++ b/net/base/sdch_manager.cc
@@ -376,10 +376,15 @@ SdchProblemCode SdchManager::AddSdchDictionary(
         if (value != "1.0")
           return SDCH_DICTIONARY_UNSUPPORTED_VERSION;
       } else if (name == "max-age") {
-        int64_t seconds;
-        // TODO(eroman): crbug.com/596541 -- should not accept a leading +.
-        base::StringToInt64(value, &seconds);
-        expiration = base::Time::Now() + base::TimeDelta::FromSeconds(seconds);
+        // max-age must be a non-negative number. If it is very large saturate

[eroman, 2016/04/11 19:36:37]

Randy, what is the expected behavior here?

I wrote some code as a proof of concept, but don't expect to actually leave it
as-written.

  * What is the normative reference for the SDCH spec? I found some stuff
online, but it didn't clarify parsing strictness, nor the full production for
max-age (other than it is delta-seconds, which it leaves undefined. The HTTP
spec defines delta-seconds as 1*DIGIT though, so it does not include negative
numbers).

  * At least one test relies on max-age allowing negative numbers. This seems
like a violation. The existing behavior has it successfully parse it and infer
an expired time.

  * What is the rationale for which things in this function hard fail, vs those
that do optimistic parsing? For instance the parsing of "port" below is
best-effort and if given a bad number simply pretends it doesn't exist. (I
previously ported this to use net::ParseUint32, but preserved its existing
semantics).

  * The default expiration if max-age is unspecified is Now() + 30days. The code
I wrote here as a proof of concept uses the default in the case too where
max-age was specified but was invalid. This seems wrong, and does cause a test
to fail. A better approach would be to either default to an expired date, or to
just fail the parsing outright.

What do you think?

+        // to 2^32 - 1.
+        uint32_t seconds = std::numeric_limits<uint32_t>::max();
+        ParseIntError parse_int_error;
+        if (ParseUint32(value, &seconds, &parse_int_error) ||
+            parse_int_error == ParseIntError::FAILED_OVERFLOW) {
+          expiration =
+              base::Time::Now() + base::TimeDelta::FromSeconds(seconds);
+        }
       } else if (name == "port") {
         int port;
         if (ParseInt32(value, ParseIntFormat::NON_NEGATIVE, &port))